terraform test: include expected diagnostics in verbose mode #37362
Conversation
| if ctx.Verbose() { | ||
| // in verbose mode, we still add all the original diagnostics for | ||
| // display even if they are expected. | ||
| run.Diagnostics = run.Diagnostics.Append(diags) | ||
| } else { | ||
| run.Diagnostics = run.Diagnostics.Append(unexpectedDiags) | ||
| } |
There was a problem hiding this comment.
Could this not likely change the status of the run?
There was a problem hiding this comment.
Or are we saying that's okay in verbose mode?
There was a problem hiding this comment.
I don't think it does anymore. I made a change recently where it is much more explicit about setting the status. So, I think from after this point it doesn't look at the set of run.Diagnostics as a whole to work out the test status anymore but sets it's explicitly based on checks made at each stage.
For example see here for apply and here for plan. In both cases, the status is computed based on just the diagnostics of the current step and then set separately to the big collection of diagnostics.
So, I think at this stage it is safe to add error diagnostics into the general diagnostics as long as we're not actually changing the result of the test when doing this.
|
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. |
When executing in verbose display mode, Terraform Test will now include any diagnostics that would normally be excluded due to be expected.
Fixes #34673
Target Release
1.14.x
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
CHANGELOG entry