Part 2 - use `xcrun xcresulttool get test-results tests` for parsing xcresults #300

dfrankland · 2025-01-15T07:26:33Z

Relies on #299

Uses JSON schema provided by xcrun xcresulttool help get test-results tests to generate Rust types and parse JSON output. This also helps parse things like retries (now shows up as a reruns in JUnit XML) or test bundles without test suites (or maybe it's the opposite?). There are a few drawbacks though:

No file paths to tests
Potentially less precise test durations
No test plan duration without extra shell out
Requires XCode 16 or newer

/trunk skip-check

Trunk Check doesn't work with codegen very well

trunk-io · 2025-01-15T07:26:37Z

Merging to main in this repository is managed by Trunk.

To merge this pull request, check the box to the left or comment /trunk merge below.

trunk-staging-io · 2025-01-15T16:37:09Z

_{View Full Report ↗︎ ⋅ Docs}

dfrankland · 2025-01-15T15:59:19Z

cli/src/context.rs

-            &org_url_slug,
+            org_url_slug.clone(),


This reference was being passed all the way down only to be cloned. That makes it difficult to reason about the overhead these functions have. It's better to push clones upwards in the call stack to make it clearer how much data we're cloning.

dfrankland · 2025-01-15T16:00:47Z

cli/src/context.rs

-        let xcresult = XCResult::new(xcresult_path, repo, org_url_slug);
-        let junits = xcresult?
-            .generate_junits()
-            .map_err(|e| anyhow::anyhow!("Failed to generate junit files from xcresult: {}", e))?;
+        let xcresult = XCResult::new(xcresult_path, org_url_slug, repo.repo_full_name())?;
+        let junits = xcresult.generate_junits();
+        if junits.is_empty() {
+            return Err(anyhow::anyhow!(
+                "Failed to generate junit files from xcresult."
+            ));
+        }


The new way of parsing things uses a schema, so we don't do any extra parsing after the JSON from xcrun xcresulttool is retrieved.

dfrankland · 2025-01-15T16:02:20Z

xcresult/Cargo.toml

 indexmap = "2.6.0"
 lazy_static = "1.5.0"
-log = "0.4.22"


No need for this now. Also, crates like this shouldn't log like a CLI, they should function as a library which returns specific errors. We should eventually switch to using something like thiserror to improve error handling ergonomics.

dfrankland · 2025-01-15T16:02:41Z

xcresult/Cargo.toml

 quick-junit = "0.5.0"
 regex = "1.11.0"
+serde = { version = "1.0.215", default-features = false }


Needed by the code from JSON schema codegen via typify

dfrankland · 2025-01-15T16:07:08Z

xcresult/create-json-schema.py

+        ["xcrun", "xcresulttool", "help", "get", "test-results", "tests"],
+        capture_output=True,
+        text=True,
+    ).stdout.replace("#/schemas", "#/$defs")


typify doesn't work with a top-level schemas property, but does with the standard $defs property

weird that a help command is used here.

I think I accidentally deleted the comment explaining that this is the only way to get the JSON schema I could find

dfrankland · 2025-01-15T16:53:40Z

xcresult/tests/data/test1.junit.xml

-<testsuites name="xcresult" tests="17" failures="0" errors="0" time="20.664">
-    <testsuite name="XcresultparserTests.xctest" tests="17" disabled="0" errors="0" failures="0" time="11.490" id="808a5981-6ebf-5882-a048-5e06b4318cee">
-        <testcase name="testCleanCodeErrors()" classname="XcresultparserTests" time="0.959" id="99d37723-0bfe-5158-a239-3f887805070d">
+<testsuites name="xcresult: Xcresultparser-Package" tests="17" failures="0" errors="0">


I'm open to naming this differently, but this is test plan name

dfrankland · 2025-01-15T16:54:10Z

xcresult/tests/data/test1.junit.xml

-    <testsuite name="XcresultparserTests.xctest" tests="17" disabled="0" errors="0" failures="0" time="11.490" id="808a5981-6ebf-5882-a048-5e06b4318cee">
-        <testcase name="testCleanCodeErrors()" classname="XcresultparserTests" time="0.959" id="99d37723-0bfe-5158-a239-3f887805070d">
+<testsuites name="xcresult: Xcresultparser-Package" tests="17" failures="0" errors="0">
+    <testsuite name="XcresultparserTests.XcresultparserTests" tests="17" disabled="0" errors="0" failures="0">


I'm open to naming this differently too. This is the bundle name and the test suite name joined by a .

dfrankland · 2025-01-15T16:55:02Z

xcresult/tests/data/test1.junit.xml

        </testcase>
-        <testcase name="testCleanCodeWarnings()" classname="XcresultparserTests" time="0.174" id="6240defe-8527-5be7-9b44-1a997021aee5">


classname is essentially moved to test suite name, but we can do this differently

dfrankland · 2025-01-15T16:55:40Z

xcresult/tests/data/test1.junit.xml

-        <testcase name="testCleanCodeWarnings()" classname="XcresultparserTests" time="0.174" id="6240defe-8527-5be7-9b44-1a997021aee5">
+        <testcase name="testCleanCodeWarnings()" time="0.170" id="f851e19c-ea03-51c9-8723-f3be81ee8f32">


I'm not sure why time is slightly different, but I assume it's because the new xcrun xcresulttool reports seconds

dfrankland · 2025-01-15T17:14:30Z

xcresult/tests/data/test4.junit.xml

-        <testcase name="testInitWithMovieSearch()" classname="MovieSearchModelTests" time="0.011" id="6a0ee60a-0a5e-5050-83ac-3eb22a6cb142" file="/Users/work/src/iOS-example/UpcomingMoviesTests/MovieSearchModelTests.swift">
-            <failure message="XCTAssertEqual failed: (&quot;12345&quot;) is not equal to (&quot;12346&quot;)"/>
+    </testsuite>
+    <testsuite name="UpcomingMoviesTests.MovieSearchModelTests" tests="1" disabled="0" errors="0" failures="1">
+        <testcase name="testInitWithMovieSearch()" time="0.011" id="a4988fb7-1943-5c3b-8ac5-1eccca86cc46">
+            <failure message="MovieSearchModelTests.swift:21: XCTAssertEqual failed: (&quot;12345&quot;) is not equal to (&quot;12346&quot;)"/>


I forgot about this file attribute... The new xcrun xcresulttool output does not include any files now. It does include "source code references" in the output of the xcrun xcresulttool get test-results test-details though. Let's discuss on Slack or on a call

gnalh

Requesting changes based on this generating a new ID. We don't want to break quarantining and test identification for our users.

gnalh · 2025-01-15T17:39:02Z

xcresult/build.rs

Do you know if this will work across Xcode versions? I suspect the schema changes with the version -- although probably slightly.

gnalh · 2025-01-15T17:39:30Z

xcresult/create-json-schema.py

+        ["xcrun", "xcresulttool", "help", "get", "test-results", "tests"],
+        capture_output=True,
+        text=True,
+    ).stdout.replace("#/schemas", "#/$defs")


weird that a help command is used here.

gnalh · 2025-01-15T17:39:59Z

xcresult/create-json-schema.py

+        capture_output=True,
+        text=True,
+    ).stdout.replace("#/schemas", "#/$defs")
+    json_schema_lines = result.split("\n")[3:148]


I think this would be safer with a regex. This could change with versions.

gnalh · 2025-01-15T17:43:56Z

xcresult/tests/data/test1.junit.xml

        </testcase>
-        <testcase name="testCleanCodeWarnings()" classname="XcresultparserTests" time="0.174" id="6240defe-8527-5be7-9b44-1a997021aee5">
+        <testcase name="testCleanCodeWarnings()" time="0.170" id="f851e19c-ea03-51c9-8723-f3be81ee8f32">


Is this ID the same as previous?

gnalh · 2025-01-15T17:47:58Z

xcresult/src/xcresult.rs

+                if let Some(node_identifier) = &xcresult_test_case.node_identifier {
+                    let id = self.generate_id(node_identifier);
+                    test_case.extra.insert("id".into(), id.into());
+                }


This will break quarantining and test identification for Zillow. That ID is how we are identifying the test.

gnalh · 2025-01-15T17:51:43Z

xcresult/src/xcresult.rs

+    fn xcresult_test_suite_to_junit_test_suite<T: AsRef<str>>(
+        &self,
+        xcresult_test_suite: &schema::TestNode,
+        bundle_name: Option<T>,
+    ) -> TestSuite {
+        let name = bundle_name
+            .as_ref()
+            .map(|bn| format!("{}.{}", bn.as_ref(), xcresult_test_suite.name))
+            .unwrap_or_else(|| String::from(&xcresult_test_suite.name));
+        let mut test_suite = TestSuite::new(name);
+        test_suite.add_test_cases(
+            self.xcresult_test_cases_to_junit_test_cases(xcresult_test_suite.children.as_slice()),
+        );
+        test_suite
+    }


classpath is optional -- do we even show it in the UI? XCResult has its own id generation so leaving it out should have no impact.

dfrankland changed the title ~~use xcrun xcresulttool get test-results tests for parsing xcresults~~ Part 2 - use xcrun xcresulttool get test-results tests for parsing xcresults Jan 15, 2025

dfrankland commented Jan 15, 2025

View reviewed changes

dfrankland requested review from mmatheson and gnalh January 15, 2025 17:19

gnalh requested changes Jan 15, 2025

View reviewed changes

dfrankland changed the base branch from dylan/reorganize-xcresult-test-fixtures to main January 20, 2025 04:02

dfrankland added 7 commits January 19, 2025 20:03

use xcrun xcresulttool get test-results tests for parsing xcresults

5c892e0

integrate into CLI

60e9b4a

rename method / variable

c020385

remove unused dep

3d64e82

add xcode 16 setup

656d766

fix linux test

4e60eda

disable lints for codegen

bbafe11

dfrankland force-pushed the dylan/xcresult-parse branch from e1518d0 to bbafe11 Compare January 20, 2025 04:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Part 2 - use `xcrun xcresulttool get test-results tests` for parsing xcresults #300

Part 2 - use `xcrun xcresulttool get test-results tests` for parsing xcresults #300

dfrankland commented Jan 15, 2025 •

edited

Loading

trunk-io bot commented Jan 15, 2025

trunk-staging-io bot commented Jan 15, 2025 •

edited

Loading

dfrankland Jan 15, 2025

dfrankland Jan 15, 2025

dfrankland Jan 15, 2025

dfrankland Jan 15, 2025

dfrankland Jan 15, 2025

gnalh Jan 15, 2025

dfrankland Jan 15, 2025 •

edited

Loading

dfrankland Jan 15, 2025

dfrankland Jan 15, 2025

dfrankland Jan 15, 2025

dfrankland Jan 15, 2025

dfrankland Jan 15, 2025

gnalh left a comment

gnalh Jan 15, 2025

gnalh Jan 15, 2025

gnalh Jan 15, 2025

gnalh Jan 15, 2025

gnalh Jan 15, 2025

gnalh Jan 15, 2025

		</testcase>
		<testcase name="testCleanCodeWarnings()" classname="XcresultparserTests" time="0.174" id="6240defe-8527-5be7-9b44-1a997021aee5">

Part 2 - use xcrun xcresulttool get test-results tests for parsing xcresults #300

Are you sure you want to change the base?

Part 2 - use xcrun xcresulttool get test-results tests for parsing xcresults #300

Conversation

dfrankland commented Jan 15, 2025 • edited Loading

trunk-io bot commented Jan 15, 2025

trunk-staging-io bot commented Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dfrankland Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnalh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Part 2 - use `xcrun xcresulttool get test-results tests` for parsing xcresults #300

Part 2 - use `xcrun xcresulttool get test-results tests` for parsing xcresults #300

dfrankland commented Jan 15, 2025 •

edited

Loading

trunk-staging-io bot commented Jan 15, 2025 •

edited

Loading

dfrankland Jan 15, 2025 •

edited

Loading