perf(runner): count the header's tests without re-sourcing every file - #1357
Merged
Conversation
Printing "Running N tests" sourced the whole suite a second time and ran every data provider a second time, in a double subshell per file, before the run began: 1.72s over this repo's 241 files. The provider scan already walks every line of every file with awk, and is already memoized so the runner reuses it. Have that same pass also emit the file's column-0 function definitions, and count the names that survive get_functions_to_run. 124 of 241 files are counted this way and the pass drops to 1.51s. The other 117 keep sourcing, because a static scan would be wrong about them, in both directions. A provider's row count is only knowable by running it. eval, a nested source and an indented definition can define a test the scan cannot see, which would undercount. A heredoc or a multi-line string can contain a `function test_x()` that is text rather than a definition, which would overcount -- acceptance tests write their fixtures exactly that way. Each of those marks the file dynamic; the scan does not try to parse where a string ends, because it also builds the provider map and a line-skipping bug there would mis-wire real tests. The gate is the differential the issue asked for, kept as a test: for every file in tests/, under three filter settings, the static count must equal the sourcing count. It has to run in a shell that has sourced no test file, since the sourcing path asks `compgen -A function` and would otherwise also count the test functions of whichever file is running. That gate immediately found a latent bug. get_functions_to_run checked for duplicates with a substring match, so a name another selected name starts with -- `test_a` against `test_ab` -- read as a duplicate, and the function returns 1, which made the file select nothing. `compgen` output is sorted, so a prefix always arrived first and nothing had reached it; definition order does. Closes #1347 Claude-Session: https://claude.ai/code/session_01EXYWTGLjf7qM8Ru3GakDRm
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.
🤔 Background
Related #1347
Printing "Running N tests" sourced the whole suite a second time and re-ran every data provider, in a double subshell per file, before the run began — 1.72s over this repo's 241 files.
💡 Changes
get_functions_to_run. 124 of 241 files skip the second source; the counting pass drops to 1.51s.eval/nestedsource/indented definitions hide tests, and a heredoc or multi-line string containsfunction test_x()as text — which is how acceptance tests write fixtures. Each marks the file dynamic.tests/, three filter settings, static count must equal sourcing count. It runs in a shell that has sourced no test file, orcompgen -A functionwould also see the running file's own tests.test_aalongsidetest_abread as a duplicate and the file selected nothing.compgenis sorted so a prefix always came first; definition order does not.https://claude.ai/code/session_01EXYWTGLjf7qM8Ru3GakDRm