Repository navigation
fix(test): anchor Jest failure detection on the leading status marker - #4505
Open
houtinghang wants to merge 1 commit into
Open
houtinghang wants to merge 1 commit into
houtinghang wants to merge 1 commit into
Conversation
`rtk test npm test` reported a fully passing suite as `[FAIL] FAILURES:` whenever a test *name* contained the substring "FAIL", because the Jest branch of extract_test_summary counted every line containing "FAIL" as a failure. Tests about failure handling are commonly named that way, and `npm test` often runs `node --test`, whose spec reporter prints the test name right after the `✔` glyph. Only the runner's own leading marker says a line is a failure: Jest opens a failed suite with a `FAIL <path>` header and marks a failed test with `✕`; node --test uses `✖`. Match on those at the start of the trimmed line instead of anywhere in it, so a passing test is free to carry "FAIL" in its own name. Fixes rtk-ai#3741
📊 Automated PR Analysis
SummaryFixes a Jest/node --test failure-detection bug in src/core/runner.rs where any test name containing the substring "FAIL" was misclassified as a failure. The fix anchors failure detection to leading status markers (✕, ✖, or a "FAIL " suite header) instead of a bare substring match, and adds regression tests covering green and failing runs. Review Checklist
Linked issues: #3741 Analyzed automatically by wshm · This is an automated analysis, not a human review. |
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.
Summary
rtk test npm testlabels a fully green suite[FAIL] FAILURES:as soon as any test name contains the substringFAIL(#3741). The Jest branch ofextract_test_summarydidline.contains("FAIL"), so a passing✔ contains uppercase word FAIL in the nameline was collected as a failure, while the same block'sOUTPUTtail still showedℹ fail 0.This is the follow-up KuSh asked for in #3753 after #988 moved the runner into
src/core/runner.rs("anchoring the Jest match … would be welcome"). It takes the smaller of the two suggested routes: no newTestEcosystemvariant, just an anchored match.✕— Jest failed test✖— node --test failed test (the reporter behind a barenpm testin the issue)FAIL— Jest failed-suite header (FAIL src/x.test.jsafter ANSI stripping,FAIL src/x.test.jswithout colour)Tests:/Test Suites:summary handling is unchanged.extract_test_summarytests: a green node --test run and a green Jest run whose test names containFAILstay green; a JestFAILheader, a✕line and a node✖line are still reported, and the passing✓sibling is not quoted as a failure.Closes #3741
Tests
cargo fmt --all --checkcargo clippy --all-targetscargo test(full suite). One integration test,unwritable_hooks_dir_never_breaks_the_hook, fails in my WSL environment with and without this change because the tests run as root there and root ignores the chmod the test relies on; unrelated to this PR.developbefore the change and pass after it.node --testvianpm test) through the built binary:rtk test npm testno longer prints[FAIL] FAILURES:.AI disclosure
Written with the help of Claude (Anthropic); I reviewed the change and the test results above before opening this PR.
🤖 Generated with Claude Code