tests(ticdc): stop relying on git root for mq compose paths#12607
tests(ticdc): stop relying on git root for mq compose paths#12607wlwilliamx wants to merge 2 commits intopingcap:masterfrom
Conversation
ti-chi-bot
bot
added
release-note-none
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the external dependency go-findroot with a custom internal implementation, ResolveRepoPath, to locate the repository root. This change allows the framework to discover test assets without relying on git metadata, improving support for symlinked checkouts and git worktrees. Feedback was provided to improve the robustness of the directory traversal logic in the new utility to ensure it correctly evaluates all directories up to the filesystem root.
| for dir != "." && dir != string(filepath.Separator) { | ||
| if isRepoRoot(dir) { | ||
| return filepath.Join(dir, normalizedRel), nil | ||
| } | ||
| parent := filepath.Dir(dir) | ||
| if parent == dir { | ||
| break | ||
| } | ||
| dir = parent | ||
| } |
There was a problem hiding this comment.
The current loop condition dir != "." && dir != string(filepath.Separator) prevents the search from checking the filesystem root (e.g., / on Unix) or the current directory if base is relative. Since filepath.Dir returns the same path when it reaches the root of the filesystem or the root of a relative path (like .), it is more robust to use parent == dir as the termination condition. This ensures that every directory in the hierarchy, including the root, is evaluated by isRepoRoot.
| for dir != "." && dir != string(filepath.Separator) { | |
| if isRepoRoot(dir) { | |
| return filepath.Join(dir, normalizedRel), nil | |
| } | |
| parent := filepath.Dir(dir) | |
| if parent == dir { | |
| break | |
| } | |
| dir = parent | |
| } | |
| for { | |
| if isRepoRoot(dir) { | |
| return filepath.Join(dir, normalizedRel), nil | |
| } | |
| parent := filepath.Dir(dir) | |
| if parent == dir { | |
| break | |
| } | |
| dir = parent | |
| } |
|
@wlwilliamx: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
ti-chi-bot
bot
added
needs-1-more-lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lidezhu, wk989898 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
3 participants
What problem does this PR solve?
Issue Number: close #12606
What is changed and how it works?
This removes the mq protocol test framework dependency on git metadata when it resolves repo-local docker-compose assets.
framework.ResolveRepoPath, which searches upward from the framework source tree and current working directorygo-findrootdependencyCheck List
Tests
Manual test:
go test ./tests/mq_protocol_tests/framework/... -run TestResolveRepoPath -count=1Questions
Will it cause performance regression or break compatibility?
No. This is a test-only path resolution change.
Do you need to update user documentation, design documentation or monitoring documentation?
No.
Release note