fix(resolution): never resolve a production call into a test file - #1663
Open
ferrine wants to merge 1 commit into
Open
fix(resolution): never resolve a production call into a test file#1663ferrine wants to merge 1 commit into
ferrine wants to merge 1 commit into
Conversation
Tests depend on the code they exercise; the code never depends on its
tests. A production symbol resolving INTO a test file is therefore always
wrong, and it happens constantly, because the names reaching the
name-matching fallback are exactly the ones with no real definition to bind
to. A React component calling `t(...)` from a `const { t } = useTranslation()`
destructure has no local `t` node and no exported one either — the hook
comes from a package — so the only same-named symbols anywhere are helpers
inside test files, and path proximity picks one. Measured on a mixed
Elixir/TypeScript/Python repository: 2,820 calls across 137 production page
components all landed on a single i18n stub defined in one test file, making
it the second most-called symbol in the codebase.
The gate is unconditional, including when it empties the candidate set. A
fallback that kept a lone test-file match would leave the whole defect in
place, since the fabricated edges are precisely the ones with no production
alternative; and production cannot depend on a test file in any case, as it
would not build.
Test files are identified by the naming their RUNNER enforces, never by
directory. This deletes edges, so a wrong identification is silent, and
which directories hold a project's tests is the project's own decision:
`spec/` is an OpenAPI document in one repo and RSpec in another, `testing/`
is often a shipped utility library. A filename is not a preference — pytest
collects `test_*.py` and nothing else, `go test` requires `_test.go`, vitest
and jest default to `*.test.*`. On the repository above, the runner-enforced
names accounted for every impossible edge but one; directory patterns would
have added the risk without the reward.
The bare CamelCase suffix (`OrderTest.java`) is scoped to the JVM/.NET/Swift
extensions whose runners collect on it. Applied to JavaScript it misreads
any identifier ending in "Test" — `useTests.ts` is an ordinary hook, and
vitest would never collect it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013Bsi9EH64kMisnik4E1gf7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Branch:
ferrine:fix/no-production-to-test-calls→mainSize: 3 files, +267/−5 · Opt-out:
CODEGRAPH_TEST_TREE_GATE=0Why
Tests depend on the code they exercise; the code never depends on its tests. So
a production symbol resolving into a test file is always wrong — and it happens
constantly, because the names that reach the name-matching fallback are exactly
the ones with no real definition to bind to.
The clearest case, from a real React codebase: components call
t(...)fromconst { t } = useTranslation().tis destructured from a hook, so there is nolocal node; it is never exported; and
useTranslationis re-exported straightfrom
react-i18next, so the owner is outside the repo entirely. The only symbolsnamed
tanywhere were four test-file locals — and 2,820 calls across 137production page components all resolved to one i18n stub defined in a single
test, making it the second most-called symbol in the codebase. Across the repo
this shape accounted for ~4,100 edges over 67 targets (
say,step,monotonic,field, …), and the symbols they landed on reported callers andimpact that were almost entirely fictional.
What changed
Test-file definitions are withheld from references that come from production
code. A reference from a test is left alone — tests calling test helpers is the
normal direction, and shared fixtures legitimately live in the test tree.
Applied at three strategies, because the defect reaches all of them: exact-name,
fuzzy, and — the one that needed it most — the receiver-name overlap fallback,
which scores a receiver name against a class name, so a short receiver
(
rep.say()) matches a test's stand-in class (_Rep) on one shared word. Stand-ins are by definition named after the collaborator they replace.
The gate is unconditional, including when it empties the candidate set. A
"keep it if nothing else matches" fallback would leave the entire defect in
place, since the fabricated edges are precisely the ones with no production
alternative — and production cannot depend on a test file in any case, as it
would not build.
Identification: runner-enforced filenames only, never directories
This deletes edges, so a wrong identification is silent — and which directories
hold a project's tests is the project's own decision.
spec/is an OpenAPIdocument in one repo and RSpec in another;
testing/is often a shipped utilitylibrary. Guessing at directories would remove real edges from projects that
merely name a folder unluckily.
A filename is not a preference. pytest collects
test_*.pyand*_test.pyandnothing else,
go testrequires_test.go, vitest and jest default to*.test.*/*.spec.*. A file named that way is a test, in every project, bythe runner's own rule.
I measured the split before choosing: of the impossible production→test edges on
that repo, runner-enforced filenames accounted for all but one. Directory
patterns would have added the risk without the reward, so they are not used.
The bare CamelCase suffix (
OrderTest.java) is scoped to.java/.kt/.kts/ .scala/.cs/.swift, whose runners collect on it. Applied to JavaScript itmisreads any identifier ending in "Test" —
useTests.tsis an ordinary hook andvitest would never collect it. That one was caught by your existing
Cross-language type/import gate (RN name collisions)test, whose fixture usesuseTests.ts.Result
Production→test call edges dropped from ~4,100 to 20. The three highest-degree
symbols that remain are all legitimate, checked individually: two are called
exclusively from test files (the gate correctly leaves those alone), and the
third is a real utility with hundreds of genuine production callers.