Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor JUnit XML output of terraform test into a new junit package #36304

Merged
merged 14 commits into from
Jan 13, 2025

Conversation

SarahFrench
Copy link
Member

@SarahFrench SarahFrench commented Jan 10, 2025

This PR refactors code that allows users to create a JUnit XML output file describing the outcome of a terraform test command.

Previously this feature was implemented as a Test View (i.e. the struct that managed reporting test outcomes to the terminal), see original comment here for context. This wasn't a good fit; the Test interface has several methods to allow continuous printing of updates to a terminal, whereas creating a JUnit output file requires waiting until all the tests are complete before preparing and saving the file.

This PR introduces the new interface JUnit which is a means through which a test run may create a file locally that summarises a test suite, after it's finished sending output to a View. The JUnit output feature has been refactored to use this interface.

Also, I've updated the JUnit output so that sources are available when creating parts of the XML file from diagnostics.

Future work not in this PR:

  • Add user feedback to say that this feature is not compatible with remote execution of tests
  • Address TODOs/FIXMEs in junitXMLTestReport

Target Release

1.11.x

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

This change is user-facing but is part of an experimental feature, so changelogs are not required

Comment on lines 103 to +105
view := views.NewTest(args.ViewType, c.View)
var junitXMLView *views.TestJUnitXMLFile

// The specified testing directory must be a relative path, and it must
Copy link
Member Author

Choose a reason for hiding this comment

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

This diff is being rendered in a confusing way. The change I made was to move the JUnit-related block (checking experimental status, setting up vars etc) to go underneath the block starting config, configDiags := c.loadConfigWithTests(".", args.TestDirectory). This is due to needing c.configLoader to be a non-nil pointer when we call NewTestJUnitXMLFile.

@SarahFrench SarahFrench force-pushed the junit-output-as-artifact branch from d8f69c7 to 872e950 Compare January 10, 2025 18:48
@SarahFrench SarahFrench marked this pull request as ready for review January 10, 2025 19:19
@SarahFrench SarahFrench requested a review from a team as a code owner January 10, 2025 19:19
@SarahFrench
Copy link
Member Author

SarahFrench commented Jan 10, 2025

Opening for review- I've left some comments via self-review and I've re-done this work so that the commits are clean and show the changes 'building up' over time instead of a big-bang refactor.

Copy link
Member

@liamcervante liamcervante left a comment

Choose a reason for hiding this comment

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

Implementation looks fine, just a nit around the naming of the new package. I think it could be a bit more descriptive than just artifact.

internal/command/artifact/junit.go Outdated Show resolved Hide resolved
@SarahFrench SarahFrench changed the title Refactor JUnit XML output of terraform test into a new artifact package Refactor JUnit XML output of terraform test into a new junit package Jan 13, 2025
Copy link
Member

@liamcervante liamcervante left a comment

Choose a reason for hiding this comment

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

Looks good 🎉

@SarahFrench
Copy link
Member Author

Thanks!

@SarahFrench SarahFrench merged commit ab6e4f2 into main Jan 13, 2025
11 checks passed
@SarahFrench SarahFrench deleted the junit-output-as-artifact branch January 13, 2025 13:26
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.

2 participants