Skip to content

Commit

Permalink
(Fix): Parses correct timestamp from test suites (#215)
Browse files Browse the repository at this point in the history
Fixes issues in #209, and verified on the _actual_ bundle junits we were seeing in https://github.com/trunk-io/trunk/actions/runs/12267338838/job/34227320055?pr=19962

![Screenshot 2024-12-11 at 1 27 13 AM](https://github.com/user-attachments/assets/000f6bc1-070d-42a7-9be9-c62a027fca9e)


- `timestamp` _is not_ an actual part of the well-accepted JUnit spec for [**test cases**](https://github.com/nextest-rs/quick-junit/blob/main/src/report.rs#L309-L312)--only some tools support it, including our C++ test reporters, which I had been using for testing before
- `timestamp` _is_ a more common part of the JUnit spec for [**test suites**](https://github.com/nextest-rs/quick-junit/blob/main/src/report.rs#L167-L168), including [example](https://github.com/testmoapp/junitxml?tab=readme-ov-file#complete-junit-xml-example) and in the [Jest reporter](https://github.com/jest-community/jest-junit/blob/master/README.md) we use
- Using the suite as a fallback should cover our bases and allow us to properly fallback for the sake of quarantining and dogfooding this

- It's worth noting that everywhere I could find, `timestamp` refers to the start time. In theory if you intentionally ran 3 test runs in parallel (rather than jest retries' serial) you might get different results depending on finishing order. In theory, this is acceptable behavior since such a test would be considered flaky (eventually) anyway, but that leaves potential for follow-up work if/when we have a clear use case.
  • Loading branch information
TylerJang27 authored Dec 11, 2024
1 parent d90f1ab commit 2fdf1d9
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 2 deletions.
33 changes: 31 additions & 2 deletions cli/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,17 @@ fn convert_case_to_test(
org_slug: &str,
parent_name: &String,
case: &quick_junit::TestCase,
suite: &quick_junit::TestSuite,
) -> Test {
let name = String::from(case.name.as_str());
let xml_string_to_string = |s: &quick_junit::XmlString| String::from(s.as_str());
let class_name = case.classname.as_ref().map(xml_string_to_string);
let file = case.extra.get("file").map(xml_string_to_string);
let id: Option<String> = case.extra.get("id").map(xml_string_to_string);
let timestamp = case
.timestamp
.or(suite.timestamp)
.map(|t| t.timestamp_millis());
Test::new(
name,
parent_name.clone(),
Expand All @@ -150,7 +155,7 @@ fn convert_case_to_test(
id,
org_slug,
repo,
case.timestamp.map(|t| t.timestamp_millis()),
timestamp,
)
}

Expand Down Expand Up @@ -184,7 +189,7 @@ pub async fn extract_failed_tests(
for suite in &report.test_suites {
let parent_name = String::from(suite.name.as_str());
for case in &suite.test_cases {
let test = convert_case_to_test(repo, org_slug, &parent_name, case);
let test = convert_case_to_test(repo, org_slug, &parent_name, case, suite);
match &case.status {
TestCaseStatus::Skipped { .. } => {
continue;
Expand Down Expand Up @@ -315,8 +320,10 @@ mod tests {

/// Contains 1 failure at 1:00
const JUNIT0_FAIL: &str = "test_fixtures/junit0_fail.xml";
const JUNIT0_FAIL_SUITE: &str = "test_fixtures/junit0_fail_suite_timestamp.xml";
// Contains 1 pass at 2:00
const JUNIT0_PASS: &str = "test_fixtures/junit0_pass.xml";
const JUNIT0_PASS_SUITE: &str = "test_fixtures/junit0_pass_suite_timestamp.xml";
// Contains 1 failure at 3:00 and 1 failure at 5:00
const JUNIT1_FAIL: &str = "test_fixtures/junit1_fail.xml";
// Contains 2 passes at 4:00
Expand Down Expand Up @@ -346,6 +353,28 @@ mod tests {
assert!(retried_failures.is_empty());
}

#[tokio::test(start_paused = true)]
async fn test_extract_retry_suite_failed_tests() {
let file_sets = vec![FileSet {
file_set_type: FileSetType::Junit,
files: vec![
BundledFile {
original_path: get_test_file_path(JUNIT0_FAIL_SUITE),
..BundledFile::default()
},
BundledFile {
original_path: get_test_file_path(JUNIT0_PASS_SUITE),
..BundledFile::default()
},
],
glob: String::from("**/*.xml"),
}];

let retried_failures =
extract_failed_tests(&BundleRepo::default(), ORG_SLUG, &file_sets).await;
assert!(retried_failures.is_empty());
}

#[tokio::test(start_paused = true)]
async fn test_extract_multi_failed_tests() {
let file_sets = vec![FileSet {
Expand Down
8 changes: 8 additions & 0 deletions cli/test_fixtures/junit0_fail_suite_timestamp.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<testsuites tests="1" failures="1" disabled="0" errors="0" time="0.001" name="AllTests">
<testsuite name="HelloTest" tests="1" failures="1" disabled="0" skipped="0" errors="0" time="0.001" timestamp="2024-12-10T01:00:00.000">
<testcase name="Hello" file="trunk/hello_world/cc/hello_test.cc" line="9" status="run" result="completed" time="0.001" classname="HelloTest">
<failure message="Failure at 1:00"/>
</testcase>
</testsuite>
</testsuites>
6 changes: 6 additions & 0 deletions cli/test_fixtures/junit0_pass_suite_timestamp.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<testsuites tests="1" failures="0" disabled="0" errors="0" time="0." name="AllTests">
<testsuite name="HelloTest" tests="1" failures="0" disabled="0" skipped="0" errors="0" time="0." timestamp="2024-12-10T02:00:00.000">
<testcase name="Hello" file="trunk/hello_world/cc/hello_test.cc" line="9" status="run" result="completed" time="0." classname="HelloTest" />
</testsuite>
</testsuites>

0 comments on commit 2fdf1d9

Please sign in to comment.