Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow running Check tests on imported resources #717

Closed
greentruff opened this issue Mar 1, 2021 · 3 comments · Fixed by #1052
Closed

Allow running Check tests on imported resources #717

greentruff opened this issue Mar 1, 2021 · 3 comments · Fixed by #1052
Labels
enhancement New feature or request subsystem/tests Issues and feature requests related to the testing framework.
Milestone

Comments

@greentruff
Copy link

greentruff commented Mar 1, 2021

SDK version

v1.15.0

I checked 2.4.4 and the behavior for imported tests is identical, state is not modified.

Use-cases

The terraform plugin for datadog supports modifying certain properties of default rules. Default rules cannot be created by users and are instead a special resource which must be imported before being modified. See documentation the documentation here.

Implementation of the resource is here

We would like to implement tests which check that the fields are being modified correctly.

Attempted Solutions

The existing tests misused the test framework and use both ImportState: True and Check. The Checks are ignored in this case, testing with ImportStateCheck the state is right after import and does not take into account other changes on top of it. It is also not possible to split the check into multiple steps as ImportState will revert back to the previous state after an import step.

Proposal

Add a flag to keep state after an import step such as ImportPersistState: True which returns the updated state so that following steps can modify it.

References

@greentruff greentruff added the enhancement New feature or request label Mar 1, 2021
@bflad bflad added the subsystem/tests Issues and feature requests related to the testing framework. label Mar 15, 2022
@bflad
Copy link
Contributor

bflad commented Aug 25, 2022

Thanks so much for filing this, @greentruff and this is a great idea! Apologies for the long delay in responding.

I believe our preference here would be something as you mention in your proposal, a new ImportState* flag that can be added during the "import TestStep mode". Then provider developers would be able to do something like:

resource.TestCase{
  // ... other fields omitted for brevity ...
  Steps: []resource.TestStep{
    // Importing TestStep
    {
      ImportState:       true,
      ImportStateId:     /* required if first TestStep */,
      ImportStateVerify: true,
      Config: /* required if first TestStep */,
      ImportStatePersist: true, // <-- New flag to trigger "keeping" the imported state
    },
    // Checking TestStep
    {
      Config: /* ... */,
      Check: /* ... */,
    },
  },
}

Naming suggestions are welcome, but I think aligning with the other ImportState* ones will suffice for now. A future iteration of the testing framework can clean up the TestStep implementations so its clearer which are meant for "apply" steps versus "import" steps.

One detail we do need to capture in provider defined implementations is that if there is no prior state, then the Config and ImportStateId* are required, which should be enforced by the testing framework.

Thanks again!

@sebasslash
Copy link

sebasslash commented Aug 25, 2022

Hi @bflad! Can confirm a solution like this would also be beneficial for our use case in terraform-provider-tfe, where we need to create resources externally, import them and then subsequently test against these resources. Ideally, I'd like to be able to reuse state from the import step(s) in later test steps -- so I think ImportStatePersist: true would work really well here. ❤️

bendbennett added a commit that referenced this issue Sep 7, 2022
…unNewTest and when called at this point will overwrite the contents of the .terraform directory with the current version of the provider (#717)
bendbennett added a commit that referenced this issue Sep 7, 2022
@bflad bflad added this to the v2.22.0 milestone Sep 7, 2022
bendbennett added a commit that referenced this issue Sep 7, 2022
bendbennett added a commit that referenced this issue Sep 7, 2022
bendbennett added a commit that referenced this issue Sep 7, 2022
…'re not persisting the state generated by the import (#717)
bendbennett added a commit that referenced this issue Sep 7, 2022
…port test works when ImportStatePersist is false (#717)
bendbennett added a commit that referenced this issue Sep 7, 2022
bendbennett added a commit that referenced this issue Sep 8, 2022
#1052)

* Use ImportStatePersist to preserve state generated by import operation (#717)

* Removing additional call to Init as this has already been called in runNewTest and when called at this point will overwrite the contents of the .terraform directory with the current version of the provider  (#717)

* Adding CHANGELOG entry (#717)

* Adding test coverage (#717)

* Linting (#717)

* Appears that init needs to be run within testStepNewImportState if we're not persisting the state generated by the import (#717)

* Fixing test and adding a couple of additional tests to verify that import test works when ImportStatePersist is false (#717)

* Linting (#717)
@github-actions
Copy link

github-actions bot commented Oct 9, 2022

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request subsystem/tests Issues and feature requests related to the testing framework.
Projects
None yet
3 participants