Skip to content

Fix monorepo subdirectory path handling when ITR is enabled#34

Open
anmarchenko wants to merge 1 commit intomainfrom
anmarchenko/fix_monorepo_subdirectory_tests
Open

Fix monorepo subdirectory path handling when ITR is enabled#34
anmarchenko wants to merge 1 commit intomainfrom
anmarchenko/fix_monorepo_subdirectory_tests

Conversation

@anmarchenko
Copy link
Member

Summary

Fixes #33

  • When running ddtest from a monorepo subdirectory (cd core && ddtest run), full test discovery (ITR enabled) returns repo-root-relative paths like core/spec/models/order_spec.rb. Workers run from the subdirectory CWD, so they resolve core/core/spec/... which does not exist.
  • Added conservative path normalization in PrepareTestOptimization that 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.
  • Fast discovery path is unchanged (it already returns CWD-relative paths from glob).

Test plan

  • 8 unit tests for the normalization helper (prefix stripping, nested subdirs, no-op cases)
  • Regression test reproducing the exact issue Monorepo: workers fail with LoadError when ITR is enabled and CWD is a subdirectory #33 scenario (subdir + root-relative paths)
  • Non-regression tests for repo-root execution, fast-discovery path, and prefix mismatch safety
  • End-to-end Plan test verifying test-files.txt and runner-0 contain CWD-valid paths
  • make test passes (all packages)
  • make lint passes (0 issues)

Made with Cursor

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>
@anmarchenko anmarchenko marked this pull request as ready for review February 12, 2026 18:19
@anmarchenko anmarchenko requested a review from a team as a code owner February 12, 2026 18:19
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +96 to +100
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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Monorepo: workers fail with LoadError when ITR is enabled and CWD is a subdirectory

1 participant