Skip to content

include_tests is a no-op for root-level tests/ directories and *.spec.ts files #1593

Description

@asemraza

Summary

include_tests silently does nothing for repositories whose test files live in a root-level tests/ directory, or whose tests are named *.spec.ts. The flag is documented, defaults to false, and the response looks correct. Test code is simply present in every result with no indication.

Found on v0.10.3 while verifying #1542.

Cause

is_test_file in src/mcp/mcp.c matches substrings against a repository-relative path:

return strstr(path, "/test") != NULL || strstr(path, "test_") != NULL ||
       strstr(path, "_test.") != NULL || strstr(path, "/tests/") != NULL ||
       strstr(path, "/spec/") != NULL || strstr(path, ".test.") != NULL;

Paths are stored repository-relative, which I confirmed on a 7,722-node graph: only 116 nodes contain /Users/, and all of them are in a real directory named app/Filament/Clusters/Users. An absolute-path store would match every node.

So tests/Unit/Foo.php matches nothing. /tests/ needs a leading slash the path never has. A nested app/Modules/Billing/tests/Foo.php does match, which is why this is easy to miss. Separately, .spec.ts matches nothing either: the list has /spec/ for directories and .test. for files, but no .spec..

Controlled reproduction

Same repository, same node type (Module), same tool version, both flagged is_test: true by the indexer. The only difference is the filename:

File Filtered with include_tests: false
src/lib/jetclean/navigation.test.ts yes, 18 vs 19 callers
e2e/login.spec.ts no, appears as a caller

Node type is therefore not the discriminator. On a PHP repository with tests in tests/, include_tests: true and include_tests: false return byte-identical results, test nodes included in both.

Why it is worth more than a filter fix

The filter runs at output time, so the test nodes are still resolution candidates. On PHP the effect is not extra rows but wrong edges. Tracing OtherCharge::generateOtherChargeInvoice returned:

tests.Unit.ClientResetPasswordControllerTest.ClientResetPasswordTestUser.save   1  heuristic 0.75
tests.Unit.CreditCardStatusServiceTest.CreditCardStatusRelation.first           2  heuristic 0.55

The method calls $othercaharge->save() and an Eloquent ->first(). Both resolve into vendor/, which is not indexed, so name-based fallback picked unrelated helper classes that happen to define those names, inside test files. Excluding tests/ at index time removed both edges and introduced no replacements, which suggests the fallback should also prefer non-test candidates, or decline rather than guess when the only candidates are test-local.

You already have php_dynamic_unresolved at 0.10 confidence for exactly this situation; here the heuristic guess won instead.

Suggested fix

Normalise or anchor the path check rather than extending the substring list, so tests/ at the repository root is treated the same as /tests/ nested, and add .spec. alongside .test.. A regression test with a root-level tests/ fixture would cover the common case, since the current fixtures appear to be nested.

Scope

Root-level test directories are the convention in Laravel, PHPUnit, Python and Rust. *.spec.ts is the convention in Playwright, Jasmine and Angular. Both currently fall through.

Metadata

Metadata

Assignees

No one assigned

    Labels

    parsing/qualityGraph extraction bugs, false positives, missing edges

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions