Fix monorepo subdirectory path handling when ITR is enabled#34
Fix monorepo subdirectory path handling when ITR is enabled#34anmarchenko wants to merge 1 commit intomainfrom
Conversation
When running ddtest from a monorepo subdirectory (e.g., `cd core && ddtest run`), full test discovery returns repo-root-relative paths like `core/spec/models/order_spec.rb`. These paths were passed unchanged to workers running from the `core/` CWD, causing them to resolve `core/core/spec/...` which does not exist. Add conservative path normalization that detects the CWD's position relative to the git root and strips the matching prefix from repo-root-relative paths before storing them. The normalization is fail-safe: paths are returned unchanged if git root is unavailable, the prefix doesn't match, or CWD is already the repo root. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49256468eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if strings.HasPrefix(normalizedPath, prefixWithSlash) { | ||
| stripped := strings.TrimPrefix(normalizedPath, prefixWithSlash) | ||
| slog.Debug("Normalized test file path for subdirectory execution", | ||
| "original", path, "normalized", stripped, "subdirPrefix", subdirPrefix) | ||
| return stripped |
There was a problem hiding this comment.
Avoid stripping valid CWD-relative paths by prefix alone
This branch strips any relative path that starts with subdirPrefix + "/" even when the path is already valid from the current working directory, so it can rewrite the wrong files in nested layouts (for example, running in /repo/core with a real file at core/spec/foo_spec.rb under that directory). In that case core/spec/foo_spec.rb is transformed to spec/foo_spec.rb, and workers will execute/look up a different path than discovery returned. Please add an unambiguous guard (such as existence checks on original vs stripped paths) before rewriting.
Useful? React with 👍 / 👎.
Summary
Fixes #33
ddtestfrom a monorepo subdirectory (cd core && ddtest run), full test discovery (ITR enabled) returns repo-root-relative paths likecore/spec/models/order_spec.rb. Workers run from the subdirectory CWD, so they resolvecore/core/spec/...which does not exist.PrepareTestOptimizationthat detects the CWD's position relative to the git root and strips the matching prefix from repo-root-relative paths. Fail-safe: no-op if git root is unavailable, prefix doesn't match, or CWD is already repo root.Test plan
test-files.txtandrunner-0contain CWD-valid pathsmake testpasses (all packages)make lintpasses (0 issues)Made with Cursor