chore: rewrite test progress watcher in JavaScript - #278
Merged
Merged
Conversation
Per review on #277: the bash version embedded a Python heredoc, adding a third scripting language when this repo already has first-class JS support. Rewritten as node, matching the existing scripts/*.js conventions (shebang, JSDoc header, CommonJS) and wired up as `yarn watch:tests`. Parses the JUnit XML attributes with a regex rather than adding a parser dependency -- fast-xml-parser resolves here but only transitively, so depending on it could break silently if the dep tree shifts. Only a few root-element attributes are needed. Verified against the bash version on the same results directory: identical output. Also checked the missing-directory path (exits 1), repeated polling, and failure aggregation (failures + errors). One bug caught while comparing: the first JS attempt read the leading `<?xml ...?>` declaration instead of the `<testsuite` element and reported 0 classes where bash reported 2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z
Per review: rather than working around the missing dependency with a regex, declare it. fast-xml-parser was already resolving here, but only transitively via @react-native-community/cli -- depending on that would have worked today and could break silently if the dep tree shifted. Now an explicit devDependency (it is only used by a dev script, never shipped), and readSuite parses properly instead of string-matching attributes off the root element. Verified output is unchanged against the same results directory, that failures and errors still aggregate, and that a half-written file is skipped rather than crashing the poll -- Gradle writes these while the build runs, so partial reads are normal rather than exceptional. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z
Code Coverage Report
|
📊 Code Coverage Summary
|
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
1 participant
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.
Follow-up to the review comment on #277: the test-progress watcher was bash wrapping a Python heredoc, which added a third scripting language to a repo that already has first-class JS support.
Changes
scripts/watch_test_progress.jsscripts/*.jsconventions (shebang, JSDoc header, CommonJS)scripts/watch_test_progress.shpackage.jsonyarn watch:tests, plusfast-xml-parseras a devDependencydocs/build/wsl_unison_environment.mdOn the dependency
fast-xml-parseralready resolved here, but only transitively via@react-native-community/cli. Depending on that would have worked today and broken silently if the dep tree shifted, so it is now declared explicitly.devDependenciessince it is only used by a dev script and never ships.What it does
Gradle's
Testtask prints nothing per-test, so a long run looks identical to a hung one. ThetestLoggingblock added in #277 covers new runs; this covers a build that is already in flight, by reading the JUnit XML as each class lands:Verification
Compared against the bash version on the same results directory — identical output. That comparison was worth doing: the first JS attempt read the leading
<?xml ...?>declaration instead of the<testsuiteelement and reportedclasses=0where bash reportedclasses=2.Also checked:
FAILURES=3)Not part of the build or CI, so the meaningful check here is that
yarn.lockresolves cleanly on a fresh install.🤖 Generated with Claude Code
https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z