terraform test: Another round of the JUnit XML output experiment #34973
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JUnit XML output from "terraform test" is currently experimental, because we're still trying to decide how best to map Terraform's testing concepts onto the Java-oriented JUnit XML format. The first round of feedback showed that some JUnit consumers completely ignore test suite names, and instead expect each test case to specify which "class" it belongs to. Those tools then might allow grouping or filtering the tests by "class". Despite the Java-oriented terminology, it seems that "classname" is really just interpreted something like "test group name", where the classname and the name together represent the full test case identifier. With that in mind then, we'll now duplicate the test suite name (which in our case is the filename of the test scenario file) into the classname attribute of each testcase element. For now we'll retain the per-suite names, always containing the same value, because we also have evidence of some consumers preferring to find this information in that location.
For now this is only to have a place to save the execution duration, but wrapped in a separate struct so that we can distinguish between cases when it is set and when it isn't, because not all test runners will necessarily populate this and even those that do might not populate it in situations where, for example, a test run gets skipped due to an upstream failure. As of this commit, nothing makes use of this new information. A future commit will use it as part of the experimental JUnit XML output from "terraform test", since the JUnit XML format de-facto _requires_ an execution duration for every test case.
Our currently-experimental JUnit XML output didn't previously include the "time" attribute for test runs at all, but from testing with common JUnit XML consumers we've learned that populating this is effectively mandatory because otherwise software just assumes that it's zero, producing a misleading result. We'll now populate it based on the test runner's measure of the execution time, assuming that the test runner actually measured it. If not, we'll leave it omitted and thus get the same behavior as before, which in practice means that most software will treat it as zero anyway. Because JUnit XML output is still experimental with the goal of getting feedback on exactly how we map the Terraform testing model onto that Java-oriented format, this is intentionally not supported for remote test runs in Terraform Cloud yet. It isn't really practical for Terraform Cloud to participate in short-lived Terraform CLI experiments, because Terraform Cloud has a different release schedule than Terraform CLI does. However, if this experiment is successful then we will eventually need to find a way to plumb this additional information through the JSON logs and ensure that it arrives in the locally-generated JUnit XML output.
liamcervante
approved these changes
Apr 11, 2024
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
# for free
to subscribe to this conversation on GitHub.
Already have an account?
#.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #34291 we introduced an experimental option for
terraform testto produce JUnit XML output, in the hope of collecting feedback to eventually resolve #34264.The first round of experiment yielded some good information about how different JUnit XML-consuming software expects the data to be structured. As expected, because this format was designed for Java there are various different ways to map non-object-oriented language concepts onto it, and thus we didn't quite nail it on the first round.
This second round makes two changes in response to the feedback:
The test scenario filename also gets populated into the
classnameattribute of each test case, in addition to its previous placement as the name of a test suite, because some consuming software seems to ignore test suites entirely and uses the test runs exclusively.Although the name "classname" suggests a Java class, in practice this seems to have been interpreted as something more general like "test group name", and so now the
classnameandnamefields together uniquely identify a test run, and software can use theclassnameto group the test runs by which scenario file they were defined in.We now set the
timeattribute for each run to the duration of the run execution in fractional seconds. It appears that in most software this field is effectively required, in that its absence is treated astime="0"rather than as "time unspecified".This meant extending the test runner to actually measure the time and record it as a new field in the internal run model, which the JUnit XML renderer then reacts to. Since this remains just experimental I only did that for the local test runner. If this experiment is successful then we'll need to figure out how to plumb all this through the Terraform Cloud remote test execution environment too, but the focus for now is only on learning what JUnit XML structure makes sense, so we'll keep the experiment limited to local execution to allow iterating more quickly.
This PR does not address the third main piece of feedback about including the specific test failure messages in the failure message instead of just a fixed placeholder message. That would require some more invasive changes to how the test runner recognizes and reports test failures as opposed to execution errors. However, it's already clear where in the JUnit XML structure that information would go, and so I think we can safely assume that we'll be able to do that at some later point without changing any other details of the output.
The option still remains experimental, so my intention for merging it would be to ask those who gave feedback the first time to try again once we have an alpha release containing this update. If that feedback suggests that we're on the right track, then we can start discussion about when and how to implement this "for real", with a more maintainable implementation approach and with the requisite support from Terraform Cloud's remote test execution environment.