testing framework: implement panic handling #33525
Merged
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
jbardin
approved these changes
Jul 18, 2023
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR just adds the default
logging.PanicHandler()into the go routines started by the testing framework.I was going to add some complex panic handling, where if a given test panics then the whole test command won't fail but just the current test case. Then the rest of the test cases could finish executing safely.
However, these test cases aren't like testing programming languages in which panics are expected, and something that the writer of the tests should go and fix. In Terraform panics are always bad, they're not something the user can recover from, and not something they should expect and have to deal with. If Terraform panics, they need to file a bug with us. This is all to point to the conclusion that we don't need to recover from panics in this way, it's not something the users should ever see and if they do, skipping it and carrying on isn't the right thing to do anyway. We want to make it annoying for them so they'll come and tell us and we can fix.