Integrate use of backend blocks in tests with skip_cleanup feature #36848
Conversation
SarahFrench
changed the title
SarahFrench
added
the
no-changelog-needed
| // We don't return destroyDiags here because the calling code sets the return code for the test operation | ||
| // based on whether the tests passed or not; cleanup is not a factor. | ||
| // Users will be aware of issues with cleanup due to destroyDiags being rendered to the View. | ||
| return nil |
There was a problem hiding this comment.
Is this accurate? Or would we update this to return destroyDiags?
| // We don't create a state artefact when the node state's corresponding run block has a backend, | ||
| // UNLESS an error occurs when returning the state to match that run block's config during cleanup. | ||
| return nil |
There was a problem hiding this comment.
This prevents the state artefact being made for the state key if there's a backend in play but only if there are no errors from the cleanup
There was a problem hiding this comment.
The manifest file still exists and references an uncreated file. However this seems to be the expected behaviour for when no state artefacts are created during a test operation.
| @@ -94,9 +94,11 @@ func (t *TestStateTransformer) Transform(g *terraform.Graph) error { | |||
| if err != nil { | |||
| return fmt.Errorf("error retrieving state for state key %q from backend: error retrieving state manager: %w", key, err) | |||
| } | |||
| stmgr.RefreshState() | |||
There was a problem hiding this comment.
Realising that I never read in the state from the state file (lol) made me realise that my tests weren't very effective previously. I've updated some tests in ways that detect that state is used or not (see non deterministic ID of test_resource being deterministic due to state in TestTest_UseOfBackends_priorStateUsedByBackend)
|
I'm happy to receive feedback on this PR, but work will be paused for about a week and I've returned it to draft. Instead please see the config validation work I split out of this PR. |
…eanup errors have occurred
SarahFrench
force-pushed
the
sarah/skip-cleanup-with-backend
branch
from
86e5c1f to
371a391
Compare
SarahFrench
changed the base branch from
f-controlling-destroys
to
sarah/backend-skip-cleanup-validation
…/or reuse prior state
…or during cleanup
SarahFrench
force-pushed
the
sarah/skip-cleanup-with-backend
branch
from
371a391 to
73608f9
Compare
This PR:
Adds validation to stop users settingskip_cleanup=falsein a run block with abackendblockAdds tests for that validation, and also to show it's ok to setskip_cleanup=truein a run block with abackendblock