Skip to content

fix(retro): count polyglot test files and exclude vendored dirs#2013

Open
genisis0x wants to merge 2 commits into
garrytan:mainfrom
genisis0x:fix/1999-retro-test-glob-polyglot
Open

fix(retro): count polyglot test files and exclude vendored dirs#2013
genisis0x wants to merge 2 commits into
garrytan:mainfrom
genisis0x:fix/1999-retro-test-glob-polyglot

Conversation

@genisis0x

Copy link
Copy Markdown
Contributor

Problem

/retro Step 1 command 10 counts repository test files with a JS/TS-only glob and excludes only node_modules:

find . -name '*.test.*' -o -name '*.spec.*' -o -name '*_test.*' -o -name '*_spec.*' 2>/dev/null | grep -v node_modules | wc -l

Python's dominant convention is test_*.py, which none of those patterns match, so on a Python repo the test_health.total_test_files metric never reflects the real count. Step 13 persists it and Step 12 trends it, so the test-health trend line tracks noise. As #1999 notes, a naive widening over-corrects, because virtualenv / vendored dependency trees (.venv, venv, site-packages, vendor) carry their own test suites and only node_modules was pruned.

Fix

  • Add the common non-JS conventions: test_*.py and *Test.java. The existing *_test.* / *_spec.* already cover Go (*_test.go), Ruby (*_test.rb / *_spec.rb), and Python's *_test.py, so the genuine gaps are Python's test_ prefix and Java's separator-less FooTest.java.
  • Prune vendored and virtualenv directories (node_modules, .venv, venv, site-packages, vendor, .git) inside find instead of post-filtering only node_modules, so dependency test suites no longer inflate the count.

Verification

On a polyglot fixture (5 real project test files across ts/js/go/py/java, plus dependency test files under .venv/ and node_modules/):

glob count
old (JS-only, node_modules filter) 4 (misses test_*.py + *Test.java, counts a .venv test)
new 5 (exact project count)

bun test test/gen-skill-docs.test.ts test/skill-validation.test.ts — 734 pass, 0 fail. SKILL.md regenerated from the template.

The same glob shape also lives in the shared scripts/resolvers/testing.ts (the /ship before/after counts); left out of scope here to keep this focused on the /retro trend metric in #1999.

Closes #1999

Step 1 command 10 counted test files with a JS/TS-only glob
(*.test.*, *.spec.*, *_test.*, *_spec.*) and excluded only
node_modules. On a Python repo this misses test_*.py entirely, so the
persisted total_test_files metric lands on stray matches instead of the
real count, and the Step 12 trend line measures noise. A naive widening
over-counts because virtualenv and vendored dependency trees (.venv,
venv, site-packages, vendor) carry their own test suites.

Add the common non-JS conventions (test_*.py, *Test.java; *_test.* /
*_spec.* already cover go/rb/_test.py) and prune vendored + virtualenv
directories so the count reflects project test files only.

Closes garrytan#1999
@trunk-io

trunk-io Bot commented Jun 15, 2026

Copy link
Copy Markdown

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@jbetala7

Copy link
Copy Markdown
Contributor

Heads-up on an overlap with #2037 (opened 06-18). Both PRs rewrite the same Step-1 test-counting glob — command 10 at retro/SKILL.md:1000 and the mirrored line in retro/SKILL.md.tmpl:

find . -name '*.test.*' -o -name '*.spec.*' -o -name '*_test.*' -o -name '*_spec.*' 2>/dev/null | grep -v node_modules | wc -l

— replacing the JS/TS-only glob + node_modules-only filter with a polyglot pattern and vendored-dir exclusion. Because both touch the identical lines in both SKILL.md and SKILL.md.tmpl, whichever merges second will conflict.

This PR is the narrower, issue-linked fix (Closes #1999, command 10 only). #2037 is effectively a superset: it also fixes the per-commit test glob in commands 12/13 (retro/SKILL.md:1009, still grep -E '\.(test|spec)\.' on main, which this PR doesn't touch) and adds a Step 1.5 anti-fabrication guard + regression test.

Not flagging either as wrong — just so the authors/maintainer can sequence them: if #2037 lands it subsumes this change (worth carrying Closes #1999 and crediting this PR); if this lands first, #2037 only needs to drop the now-duplicate command-10 hunk and keep its command-12/13 + plausibility-guard value.

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.

/retro test-file count uses a JS-only glob, undercounts Python repos (and overcounts if naively fixed)

2 participants