From 1817040d6392189174dd138002f9796bfa533ca4 Mon Sep 17 00:00:00 2001 From: "Clippy (Triage)" Date: Wed, 15 Jul 2026 18:51:58 +0000 Subject: [PATCH 1/2] fix(context): keep reruns emitted before the closing / MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Playwright's junit reporter (with includeRetries) emits a test that failed every attempt as a single whose / attempts appear BEFORE the closing /. The parser starts each testcase as Success, so those reruns are parsed into flaky_runs while the status is still Success. set_test_case_status then replaced the whole status enum with a fresh NonSuccess (empty reruns), discarding every earlier attempt — so a fail-all-retries test collapsed to a single run and pass-on-retry never surfaced for the failing case. Carry the accumulated reruns onto the terminal status (mem::take the flaky_runs and add_reruns them to the new failure/error) so each attempt surfaces as its own run. The pass-on-retry (flaky) path was unaffected because it has no closing to clobber the runs. Adds a regression test using Playwright's real element order (system-out -> rerunFailure... -> failure); the existing failing-rerun test only passed because it ordered before . --- context/src/junit/parser.rs | 70 +++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/context/src/junit/parser.rs b/context/src/junit/parser.rs index e0e0c2b5..965c26d9 100644 --- a/context/src/junit/parser.rs +++ b/context/src/junit/parser.rs @@ -782,6 +782,15 @@ impl JunitParser { if !matches!(test_case.status, TestCaseStatus::Success { .. }) { return; // Only set status once } + // Playwright's junit reporter (with includeRetries) emits the / + // attempts BEFORE the closing /. Those reruns were + // parsed while the status was still Success, so quick-junit filed them under + // flaky_runs; move them onto the terminal status so a test that fails every retry + // keeps its earlier attempts instead of collapsing to a single run. + let prior_reruns = match &mut test_case.status { + TestCaseStatus::Success { flaky_runs } => mem::take(flaky_runs), + _ => Vec::new(), + }; let tag = e.name(); let mut test_case_status = if tag.as_ref() == TAG_TEST_CASE_STATUS_SKIPPED { TestCaseStatus::skipped() @@ -802,6 +811,7 @@ impl JunitParser { test_case_status.set_type(r#type); } + test_case_status.add_reruns(prior_reruns); test_case.status = test_case_status; } else { self.issues.push(JunitParseIssue::Invalid( @@ -2122,4 +2132,64 @@ failures: ] ); } + + #[test] + #[allow(deprecated)] + fn test_into_test_case_runs_emits_run_per_failing_rerun_playwright_order() { + // Playwright's junit reporter (with includeRetries) emits, for a test that failed every + // attempt, the testcase's and the attempts BEFORE the closing + // . Each attempt must still surface as its own run; regression test for reruns + // being dropped when the trailing replaced the accumulated status. + let mut junit_parser = JunitParser::new(); + let file_contents = r#" + + + + + + + + + + + + + + + + + + "#; + junit_parser + .parse(BufReader::new(file_contents.as_bytes())) + .unwrap(); + + let test_case_runs = junit_parser.into_test_case_runs(IntoTestCaseRunsOptions { + org_slug: "org", + repo: &RepoUrlParts { + host: "host".into(), + owner: "owner".into(), + name: "name".into(), + }, + codeowners: None, + quarantined_test_ids: &[], + variant: "", + test_runner_config: None, + }); + + // Both rerun attempts plus the final failure surface as their own runs. + assert_eq!(test_case_runs.len(), 3); + assert!( + test_case_runs + .iter() + .all(|run| run.status == TestCaseRunStatus::Failure as i32) + ); + // The per-retry output is preserved rather than discarded with the reruns. + let messages: Vec<&str> = test_case_runs + .iter() + .map(|run| run.status_output_message.as_str()) + .collect(); + assert!(messages.contains(&"retry 1 failure")); + assert!(messages.contains(&"retry 2 failure")); + } } From 030e5083cbff02986921607a6464c1693f1f6b9a Mon Sep 17 00:00:00 2001 From: "Clippy (Triage)" Date: Wed, 15 Jul 2026 18:55:07 +0000 Subject: [PATCH 2/2] test(context): make the fail-all-retries regression test faithful to Playwright The original #1142 test ordered before , and real Playwright output is the reverse. Rework the regression test to mirror the actual reporter output for a test that failed every attempt: reruns emitted BEFORE the terminal element, a mix of (assertion) and (timeout), and a terminal (attempt 0 timed out) rather than . Asserts all four attempts surface as their own runs with each attempt's message preserved. Verified it fails without the fix. --- context/src/junit/parser.rs | 56 ++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/context/src/junit/parser.rs b/context/src/junit/parser.rs index 965c26d9..6eb93e79 100644 --- a/context/src/junit/parser.rs +++ b/context/src/junit/parser.rs @@ -2136,26 +2136,32 @@ failures: #[test] #[allow(deprecated)] fn test_into_test_case_runs_emits_run_per_failing_rerun_playwright_order() { - // Playwright's junit reporter (with includeRetries) emits, for a test that failed every - // attempt, the testcase's and the attempts BEFORE the closing - // . Each attempt must still surface as its own run; regression test for reruns - // being dropped when the trailing replaced the accumulated status. + // Faithful to Playwright's junit reporter (includeRetries) for a test that failed every + // attempt: the / attempts and the testcase are + // emitted BEFORE the closing terminal element, the reruns are a MIX of rerunFailure + // (assertion) and rerunError (timeout), and the terminal element is (attempt 0 + // timed out), not . Regression test for those attempts being dropped when the + // trailing terminal element replaced the accumulated status. let mut junit_parser = JunitParser::new(); let file_contents = r#" - - - - - - + + + + + + - - - + + + + + + + - + page.waitForTimeout: Test timeout of 10000ms exceeded. @@ -2177,19 +2183,29 @@ failures: test_runner_config: None, }); - // Both rerun attempts plus the final failure surface as their own runs. - assert_eq!(test_case_runs.len(), 3); + // All four attempts (3 reruns + the terminal error) surface as their own runs; the proto + // has no dedicated error status, so every attempt is recorded as a failure. + assert_eq!(test_case_runs.len(), 4); assert!( test_case_runs .iter() .all(|run| run.status == TestCaseRunStatus::Failure as i32) ); - // The per-retry output is preserved rather than discarded with the reruns. + // Each attempt's own message survives — including the rerunError and the two rerunFailure + // assertions that were previously discarded. let messages: Vec<&str> = test_case_runs .iter() - .map(|run| run.status_output_message.as_str()) + .filter_map(|run| run.test_output.as_ref()) + .map(|out| out.message.as_str()) .collect(); - assert!(messages.contains(&"retry 1 failure")); - assert!(messages.contains(&"retry 2 failure")); + assert!( + messages + .contains(&"/^Pacific\\b/ hour 11 from \"11:00:03 A.M.\" should be even") + ); + assert!( + messages + .contains(&"/^Pacific\\b/ hour 11 from \"11:00:23 A.M.\" should be even") + ); + assert!(messages.contains(&"Test timeout of 10000ms exceeded.")); } }