Repository navigation
Conversation
nextest annotates `passed` in place (`2 passed (1 slow), 1 failed, 1 skipped`). The summary regex expected `failed` to follow `passed` directly, so a slow test zeroed the failed/skipped counts and the green branch dropped failure blocks. Allow an optional parenthesised annotation after `passed`. Fixes rtk-ai#4420
📊 Automated PR Analysis
SummaryFixes a regex in the cargo nextest output filter so that when a test is slow, nextest's inline '(N slow)' annotation after 'passed' no longer breaks parsing of the subsequent failed/skipped counts. Previously this caused failing/skipped runs with a slow test to be misreported as a clean all-passed summary. Adds two new tests covering the slow-annotation case with failures and an all-pass case. Review Checklist
Linked issues: #4420 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
nextest puts its in-place annotation right after
passed(2 passed (1 slow), 1 failed, 1 skipped). The summary regex expectedfailedandskippedto come straight afterpassed, so once a test went slow both counts parsed as 0. Withfailed == 0the filter took its green branch and also dropped the collected failure blocks, so a failing run showed up ascargo nextest: 2 passed (1 binary, 2.503s).The fix adds an optional non-capturing
(?:\s+\([^)]*\))?group right afterpassed. It matches(1 slow)and any other parenthesised annotation nextest puts there, such as a combined(1 slow, 1 flaky). Capture-group numbering doesn't change, so the rest of the parser is untouched.Overlap with #3577: that PR adds
(N slow)rendering by capturing\((\d+) slow\)at the end of the regex, afterskipped. That position doesn't match what nextest actually prints, so its slow capture would still miss real output. This PR only fixes correctness and doesn't render slow counts. If #3577 lands afterwards, its slow capture should move into the group afterpassedthat this PR adds.Fixes #4420
Test plan
test_filter_cargo_nextest_slow_annotation_keeps_failed_and_skippeduses the real 0.9.143 lineSummary [ 2.503s] 3 tests run: 2 passed (1 slow), 1 failed, 1 skippedinside a realistic run with a FAIL block and a SLOW line. It asserts2 passed, 1 failed, 1 skippedand that the failure header and stderr are kept. It failed before the fix withcargo nextest: 2 passed (1 binary, 2.503s), which is the exact output from the issue.test_filter_cargo_nextest_slow_annotation_all_passcovers an all-green run with(1 slow), which still renders the compact line.test_filter_cargo_nextest_with_failures(2 passed, 2 failed, 1 skipped, no annotation) still passes as the non-slow non-regression.cargo fmt --all --checkcargo clippy --all-targets(no warnings)cargo test