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

feat(test-structure): add save test data if dne #1319

Merged

Conversation

bt-macole
Copy link
Contributor

@bt-macole bt-macole commented Jun 28, 2023

Description

SaveTestData is a powerful feature. But with great power comes great responsibility. If a user is not careful, this feature can cause unintended side effects including duplicating resources and overwriting state.

Adding support for only saving if a file does not exists prevent overwriting contents and provides terratest users protection from themselves.

resolves #1318

Adds SaveTerraformOptionsIfNotPresent and updates SaveTestData to include an additional overwrite bool attribute.

This change is backwards compatible.

Read the Gruntwork contribution guidelines.

  • Update the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added / Removed / Updated [X].

Added SaveTerraformOptionsIfNotPresent

@bt-macole bt-macole requested a review from denis256 as a code owner June 28, 2023 02:54
@denis256
Copy link
Member

modules/test-structure/save_test_data.go:203:13: cannot use t (variable of type "github.com/gruntwork-io/terratest/modules/testing".TestingT) as type string in argument to t.Fatalf

@bt-macole bt-macole force-pushed the add-SaveTerraformOptionsIfNotPresent branch 2 times, most recently from 73dbf30 to 8a9fbde Compare July 10, 2023 20:07
@bt-macole
Copy link
Contributor Author

modules/test-structure/save_test_data.go:203:13: cannot use t (variable of type "github.com/gruntwork-io/terratest/modules/testing".TestingT) as type string in argument to t.Fatalf

fixed:

--- PASS: TestSaveAndLoadTestData (0.00s)
PASS
ok      github.com/gruntwork-io/terratest/modules/test-structure        0.587s

@bt-macole
Copy link
Contributor Author

@denis256 anything else needed here? not urgent, but we'd like to start using this feature in some of our test suite to avoid some accidental duplicated resources.

@denis256
Copy link
Member

Hello,
the biggest concern is that PR doesn't have any test which will validate

$ grep -irn "SaveTerraformOptionsIfNotPresent" .
./modules/test-structure/save_test_data.go:27:// SaveTerraformOptionsIfNotPresent serializes and saves TerraformOptions into the given folder if the file does not exist or the json is
./modules/test-structure/save_test_data.go:30:func SaveTerraformOptionsIfNotPresent(t testing.TestingT, testFolder string, terraformOptions *terraform.Options) {
$

Implementation of SaveTerraformOptionsIfNotPresent can be changed in the future, so it will not be only a call of SaveTestData and there will be no tests to validate that SaveTerraformOptionsIfNotPresent will continue to work as before

SaveTestData is a powerful feature. But with great power comes great responsibility. If a user is not careful, this feature can cause unintended side effects including duplicating resources and overwriting state.

Adding support for only saving if a file does not exists prevent overwriting contents and provides terratest users protection from themselves.

resolves gruntwork-io#1318
@bt-macole bt-macole force-pushed the add-SaveTerraformOptionsIfNotPresent branch from 8a9fbde to bb0b060 Compare July 21, 2023 14:32
@bt-macole
Copy link
Contributor Author

bt-macole commented Jul 21, 2023

@denis256 great point. added a test to test that functionality directly as well as a test that ensures the original overwriting behavior of SaveTerraformOptions is unchanged.

@denis256 denis256 merged commit adad5f9 into gruntwork-io:master Jul 21, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support optionally not overwriting saved terraformOptions if present
2 participants