Skip to content

test(warp): neutralize python import fixtures to house naming - #246

Merged
flyingrobots merged 1 commit into
mainfrom
fix/warp-python-import-fixtures
Aug 16, 2026
Merged

test(warp): neutralize python import fixtures to house naming#246
flyingrobots merged 1 commit into
mainfrom
fix/warp-python-import-fixtures

Conversation

@flyingrobots

@flyingrobots flyingrobots commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Why

Three test files built their Python fixture trees as coqui/matcher/... and coqui/dev_profiler/... — the package layout of a private downstream repo, named in a public test suite. Nothing in the resolvers keys off the fixture name, so removing it costs no coverage.

Surfaced while working in that downstream repo; it is a naming-hygiene fix, not a bug fix.

What changed

Renamed to Graft's own fixture vocabulary (alpha/beta siblings, already used elsewhere in these suites):

before after
coqui/matcher app/alpha
coqui/dev_profiler app/beta

Why app/ and not pkg/. pkg/ is Graft's dominant fixture root (53× pkg/sources.py, 51× pkg/caller.py), but it is already in heavy use inside two of these three files — 67 references in qualified-reference-resolver.test.ts and 2 in structural-review-cold-warp.test.ts. Renaming into it risked a namespace collision within a single test's knownFiles set. app/ appears in none of the three.

Two things that made this not a blind find-and-replace

1. matcher also names unrelated Go fixtures. qualified-reference-resolver.test.ts has 11 references to matcher/sources/pending.go, matcher/sources/a.go etc. from Go resolver tests that have nothing to do with this. The rename is therefore scoped to compound prefixes only — coqui/matcher, coqui.matcher, "coqui", "matcher" — never the bare token. The Go fixtures are untouched, confirmed by diff inspection.

2. One assertion pinned a sort order that the rename inverted. This assertion compares against a .sort()ed actual:

expect(...map((edge) => edge.to).sort()).toEqual([
  "sym:app/alpha/sources.py:pending_ids",
  "sym:app/beta/cli.py:profile",
]);

The literal had been written in the order that sort produced for the old names — coqui/dev_profiler sorts before coqui/matcher, whereas app/alpha sorts before app/beta. The two entries are swapped to match. It is still exact array equality including order, so the assertion is exactly as strong as before. This was caught by the suite going red, not by inspection.

Semantics preserved

Package depth is unchanged — one root segment, one subpackage — so the relative import under test climbs identically:

from ..beta.cli import profile      # was: from ..dev_profiler.cli import profile

The diff is 37 insertions / 37 deletions: a pure rename, no lines added or removed.

Verification

  • pnpm test — 258 test files, 2056 tests, all passing
  • pnpm lint — clean
  • pnpm typecheck — clean
  • git grep -in coqui -- test src bin — no matches

No behaviour change, so no CHANGELOG entry.

Summary by CodeRabbit

  • Tests
    • Updated Python code analysis test fixtures to use the current app.alpha and app.beta package paths.
    • Continued validating relative imports, qualified references, caller detection, indexed references, and shadowed imports.

The python import-resolver, qualified-reference-resolver, and cold-WARP
structural-review tests built their fixture trees as `coqui/matcher/...` and
`coqui/dev_profiler/...` — the package layout of a private downstream repo,
named in a public test suite. Nothing in the resolvers keys off the name, so
this costs no coverage.

Renamed to Graft's own fixture vocabulary (`pkg/`, `alpha`/`beta` siblings),
using `app/` as the root because `pkg/` is already in heavy use in two of
these files (67 and 2 references) and reusing it risked namespace collision
inside a single test:

  coqui/matcher      -> app/alpha
  coqui/dev_profiler -> app/beta

Scoped to compound path prefixes (`coqui/matcher`, `coqui.matcher`,
`"coqui", "matcher"`) rather than bare tokens, because `matcher` also names
unrelated **Go** fixtures in qualified-reference-resolver.test.ts
(`matcher/sources/pending.go`, 11 references) that must not move. Those are
untouched; the diff is 37 insertions / 37 deletions, a pure rename.

Package depth is preserved (one root segment, one subpackage), so the relative
import under test still resolves the same way: `from ..beta.cli import profile`
climbs exactly as `from ..dev_profiler.cli import profile` did.

One assertion needed its expected array reordered. It compares against a
`.sort()`ed actual, and the literal had been written in the order that sort
produced for the old names — `coqui/dev_profiler` sorts before `coqui/matcher`,
while `app/alpha` sorts before `app/beta`. The expectation is still exact array
equality including order, so the assertion is exactly as strong as before.

Verified: 258 test files / 2056 tests pass; pnpm lint and pnpm typecheck clean;
no `coqui` token remains under test/, src/, or bin/.

No behaviour change, so no CHANGELOG entry.
@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7107cc9c-648b-4566-9ae6-839f46ea6677

📥 Commits

Reviewing files that changed from the base of the PR and between 68959cd and 8afe23b.

📒 Files selected for processing (3)
  • test/unit/mcp/structural-review-cold-warp.test.ts
  • test/unit/warp/python-import-resolver.test.ts
  • test/unit/warp/qualified-reference-resolver.test.ts

Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The tests replace coqui package fixtures with app.alpha and app.beta paths. Assertions continue to cover relative imports, indexed references, qualified module members, qualified callers, and shadowed imports.

Changes

Python fixture path updates

Layer / File(s) Summary
Resolver test fixture updates
test/unit/warp/python-import-resolver.test.ts
Import resolver fixtures, graph nodes, lookup paths, and expected symbols now use app.alpha and app.beta paths.
Caller and shadowing fixture updates
test/unit/mcp/structural-review-cold-warp.test.ts, test/unit/warp/qualified-reference-resolver.test.ts
Caller and module fixtures now use app.alpha paths. Existing qualified-caller and shadowing assertions remain in place.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 8afe2

This PR only renames test fixture paths and updates the order-sensitive expectation; it does not change production behavior, and no actionable merge-blocking risk remains.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies a WARP test fixture naming change and matches the pull request’s main objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/warp-python-import-fixtures

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@flyingrobots
flyingrobots merged commit 0c41d6f into main Aug 16, 2026
3 checks passed
@flyingrobots
flyingrobots deleted the fix/warp-python-import-fixtures branch August 16, 2026 08:25
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.

1 participant