Implement controlling destroy functionality within Terraform Test

[liamcervante]

This PR merges the changes within f-controlling-destroys with the changes from main, and is then merging those changes into main.

For now, I haven't added any changelog entries but I will do before we merge this. I just want to make sure my merge hasn't undone any changes.

There's still changes to the docs in here from before - these need to be ported over to the new repo before this is merged.

[github-actions]

Changelog Warning

Currently this PR would target a v1.14 release. Please add a changelog entry for in the .changes/v1.14 folder, or discuss which release you'd like to target with your reviewer. If you believe this change does not need a changelog entry, please add the 'no-changelog-needed' label.

[SarahFrench]

Thanks for dusting off this feature branch!

Given that it's been several months since I worked on it, I took a look with mostly fresh eyes and I spotted some potential issues that I've commented on the code.

Edit to add: there are some check failures in internal/moduletest/graph/test_graph_builder.go

[SarahFrench]

I realised that this warning diagnostic can be confusing for users if they combine use of a backend block and skip_cleanup, like below:

run "setup" {
  backend "local" {} # This implies skip_cleanup = true
}

run "test" {
  skip_cleanup = true # This triggers a warning to let users know that 
}

Ideally the diagnostic would be able to differentiate whether the prior skip cleanup instruction was supplied via an explicit skip_cleanup attr or through use of a backend block, as the skip_cleanup attr in the later run will impact the state stored in the backend's state file.

I think making this distinction would be possible by updating this code to also checking if there is an entry for tf.BackendConfigs[run.StateKey]; if theres an entry in skipCleanups` and also an entry in BackendConfigs for that state key then the run block currently being parsed contains a duplicate skip_cleanup=true instruction.

[liamcervante]

I think #37385 should address this 🤞

[SarahFrench]

I think there's a problem here, and to illustrate it here's a manual test I did using this branch:

Config:

run "setup" {
  skip_cleanup = true
}

run "test" {
  skip_cleanup = true
}

When I run terraform test I get this error:

│ Warning: Duplicate "skip_cleanup" block
│ 
│   on main.tftest.hcl line 5:
│    5: run "test" {
│ 
│ The run "test" has a skip_cleanup attribute set, but shares state with a later run "setup"
│ that also has skip_cleanup set. The later run takes precedence, and this attribute is
│ ignored for the earlier run.

I think the logic is currently 'the wrong way round'; the diagnostic is saying that "setup" is a later run after "test", which is incorrect, and also it appears that any instance of skip_cleanup=true after the first instance raises this warning and is ignored. I think that the warning should continue to be raised on each instance, but each time the code should also update the entry in skipCleanups for the current state key. This would address this part of the RFC:

restore the state to the last executed run block with the attribute set as true

[liamcervante]

I think #37385 should address this 🤞

[SarahFrench]

I observed that during in test case # 1, where no artefacts are made, a non-empty manifest file is created. When I debug the test and check the temp dir's files, the .terraform/test/ directory only contains manifest.json (no state files) and the manifest refers to a non-existent file:

{
  "version": 0,
  "files": {
    "main.tftest.hcl": {
      "states": {
        "": {
          "path": ".terraform/test/Qwlngn0M.tfstate",
          "reason": ""
        }
      }
    }
  }
}

Does this need to be fixed? This could be confusing when debugging issues with the test command, though arguably you could say that this is an implementation detail and users shouldn't be inspecting the file.