Skip to content

Conversation

@andrewmogan
Copy link
Collaborator

In the process of trying to centralize some Slack message payload generation logic, I noticed significant duplication of junit XML parsing logic (my fault). This PR centralizes all parsing of junit XML files output by integration tests into the JUnitXMLParser class. Furthermore, this parser class makes use of the IntegrationTestSummary, PytestResult, and TestCaseResult classes, which better reflect the hierarchical structure of pytest output. For example, the listrev_test.py result is summarized in a PytestResult, while individual tests such as test_nanorc_success are summarized in a TestCaseResult. The IntegrationTestSummary then contains a list of PytestResults (which themselves contain a list of TestCaseResults) and computes the totals automatically. These classes were previously contained in the CI dashboard module which parses integration test results, but are now contained in scripts/github-ci/integration_test_summary.py. Basically, this provides better separation of concerns and reduces unnecessary coupling.

Realizing this structure involves several changes to other modules account for this:

  • In addition to markdown, integration test summaries are uploaded as json for more flexible parsing downstream
  • The CI dashboard integration test parser now uses that json input
  • The integration test page renderer now accounts for the updated structure

Successful workflows using this branch:

Note that slack_payload_generator.py is still using its own, redundant parsing logic. This PR is already quite invasive, so I'll file a separate PR for overhauling the Slack message logic, which similarly could use some refactoring.

@jcfreeman2
Copy link
Collaborator

Approved, based on the following:

  • In both the integration and extended integration test workflows Andrew links to, it's clear that the output is the same as normal, except for the addition of a basic, nicely formatted JSON file which summarizes the integration test results.

  • The dashboard also looks totally fine (Last updated: 2026-01-23 19:22 UTC)

  • The changes he made to the amogan/refactor_xml_parsing branch since the workflows he linked to all involve (1) removing diagnostic things (cats, lss, etc.) which don't affect functionality or (2) removing references to amogan/refactor_xml_parsing in the code itself, which you certainly wouldn't want to have after merging into develop.

  • I confirmed there are no stray references to amogan/refactor_xml_parsing in the code on this feature branch.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@andrewmogan andrewmogan marked this pull request as ready for review January 26, 2026 14:38
@andrewmogan andrewmogan merged commit bc9f4f8 into develop Jan 26, 2026
@andrewmogan andrewmogan deleted the amogan/refactor_xml_parsing branch January 26, 2026 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants