Skip to content

Mildwonkey/tests#24522

Merged
mildwonkey merged 6 commits intof-init-new-installerfrom
mildwonkey/tests
Apr 1, 2020
Merged

Mildwonkey/tests#24522
mildwonkey merged 6 commits intof-init-new-installerfrom
mildwonkey/tests

Conversation

@mildwonkey
Copy link
Copy Markdown
Contributor

terraform tests passing!

I'd like to draw your attention to the two commits that add helper functions, as modifying them based on PR feedback would be easiest done right meow. 😁

testSetResourceInstanceCurrent and testSetResourceInstanceTainted are
wrapper functions around states.Module.SetResourceInstanceCurrent()
used to set a resource in state. They work with current, non-deposed
resources with no dependencies.
@ghost ghost added the sdkv1 [PRs only] Marks changes that may potentially need to be ported to the plugi nSDK label Apr 1, 2020
@mildwonkey
Copy link
Copy Markdown
Contributor Author

since our tests are currently not running:

go test ./terraform/...
ok  	github.com/hashicorp/terraform/terraform

@mildwonkey
Copy link
Copy Markdown
Contributor Author

Also: I removed terraform/module_dependencies but I suspect in hindsight that was too soon to remove the entire file - I don't know the plans for ConfigTreeDependencies. I'll revert that if needed.

Copy link
Copy Markdown
Contributor

@apparentlymart apparentlymart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

My intent in the other branch was to remove the callers into the module_dependencies.go file but I'm not sure I got all of them yet, because my focus was on the codepaths related to provider installation and calling NewContext. We might need to keep those around until command/providers.go is updated, but I'm not totally sure. Could you check if it's possible to go install a Terraform binary under this change? If this set of changes can compile then I think it's fine to move ahead, but if it breaks the build then it'll of course make other concurrent work trickier. 🤔

@mildwonkey
Copy link
Copy Markdown
Contributor Author

mildwonkey commented Apr 1, 2020

It does break the build, so I'll revert it for the time being with the caveat that I'm not going to fix the related tests which are now broken :)

Update (based on internal conversation): for the nonce I've commented out the single remaining call to ConfigTreeDependencies in the terraform providers command so we can remove the files. That command is already on the path to being refactored.

@mildwonkey mildwonkey merged commit 7c0db66 into f-init-new-installer Apr 1, 2020
@mildwonkey mildwonkey deleted the mildwonkey/tests branch April 1, 2020 17:13
@mildwonkey mildwonkey restored the mildwonkey/tests branch April 1, 2020 17:13
@mildwonkey mildwonkey deleted the mildwonkey/tests branch April 1, 2020 17:13
@mildwonkey mildwonkey restored the mildwonkey/tests branch April 1, 2020 17:13
@mildwonkey mildwonkey deleted the mildwonkey/tests branch April 1, 2020 17:15
apparentlymart pushed a commit that referenced this pull request Apr 6, 2020
* terraform: add helper functions for creating test state

testSetResourceInstanceCurrent and testSetResourceInstanceTainted are
wrapper functions around states.Module.SetResourceInstanceCurrent()
used to set a resource in state. They work with current, non-deposed
resources with no dependencies.

testSetResourceInstanceDeposed can be used to set a desosed resource in state.

* terraform: update all tests to use modern providers and state
pselle pushed a commit that referenced this pull request Apr 6, 2020
* terraform: add helper functions for creating test state

testSetResourceInstanceCurrent and testSetResourceInstanceTainted are
wrapper functions around states.Module.SetResourceInstanceCurrent()
used to set a resource in state. They work with current, non-deposed
resources with no dependencies.

testSetResourceInstanceDeposed can be used to set a desosed resource in state.

* terraform: update all tests to use modern providers and state
@ghost
Copy link
Copy Markdown

ghost commented May 2, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators May 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

sdkv1 [PRs only] Marks changes that may potentially need to be ported to the plugi nSDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants