(Feat): Parse BEP TestSummary #242
Conversation
|
Merging to
|
There was a problem hiding this comment.
Once again, sanitized to include real data but stripped for just relevant tests
| }); | ||
| bep_test_events.push(build_event); | ||
| } | ||
| _ => {} |
There was a problem hiding this comment.
Is it possible to explicitly match the remaining combinations so that we don't add more permutations without handling them in the future?
There was a problem hiding this comment.
Spoke offline, there are 28 different variants here, so probably not worth. I may just make this an if though
context/src/junit/junit_path.rs
Outdated
| #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] | ||
| #[cfg_attr(feature = "pyo3", gen_stub_pyclass_enum, pyclass(eq, eq_int))] | ||
| #[cfg_attr(feature = "wasm", derive(Tsify))] | ||
| pub enum TestRunnerJunitStatus { | ||
| #[default] | ||
| Passed, | ||
| Failed, | ||
| Flaky, | ||
| } | ||
|
|
||
| /// Encapsulates the glob path for a junit and, if applicable, the flakiness already | ||
| /// assigned by the user's test runner. See bazel_bep/parser.rs for more. | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub struct JunitPathWrapper { | ||
| /// Path or glob pattern to the junit file. | ||
| pub junit_path: String, | ||
| /// Refers to an optional status parsed from the test runner's output, before junits have been parsed. | ||
| pub status: Option<TestRunnerJunitStatus>, | ||
| } |
There was a problem hiding this comment.
Related to genericizing this, I think we ought to consider this a JunitReportStatus and JunitReportFileWithStatus. Test runners can tell us the status, but it should be fairly trivial for us to derive it from the JUnit report by seeing how many failures and reruns there were. We could even record both what the test runner says and what we derive from the JUnit report for the express reason of checking if those agree and processing quarantining as the test runner says (lest our inference is incorrect). Given that, I think we should always be able to populate status and if that's not true, I think we should probably consider adding an Unknown variant instead of making status Option<T>. With this, we can then extend our quarantining check for all JUnit reports rather than just what Bazel or other test runners tell us.
There was a problem hiding this comment.
I added a follow up ticket to add more generic support for all JUnit report files:
https://linear.app/trunk/issue/TRUNK-13911/parse-junit-files-and-check-for-reruns-for-quarantining-in-analytics
It's a part of a larger project to support same-upload reruns flake classification:
https://linear.app/trunk/project/same-upload-test-reruns-7cc622cfc105/issues
There was a problem hiding this comment.
Spoke offline, I will undertake the small refactor/renaming here, and then those tickets will capture the follow-up work for parsing the junits themselves
| @@ -39,6 +40,8 @@ pub struct FileSet { | |||
| pub file_set_type: FileSetType, | |||
| pub files: Vec<BundledFile>, | |||
| pub glob: String, | |||
| /// Added in v0.6.11. Populated when parsing from BEP, not from junit globs | |||
| pub test_runner_status: Option<JunitReportStatus>, | |||
There was a problem hiding this comment.
I ended up making it optional again since that allows us to better deserialize. Otherwise we need to have some versioned logic here for deserializing from meta.json, which doesn't seem worth the effort for this particular case
Follow-up to #215 (incomplete, just timestamps) and #226 (more BEP debugging). Parses bazel TestSummary events in order to override the failure status of a given junit. This allows us to properly return a zero exit code if bazel marks a test as flaky, rather than waiting for a quarantine status to properly return.
This should hopefully also allow us to better inform the rules engine for test runner-identified flakiness from bazel
Notes: