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

fix: aws terraform-tests performance_insights_retention_period #1597

Merged

Conversation

fnaranjo-vmw
Copy link
Contributor

#187103734

Recently, a new PR for validating this property was merged: hashicorp/terraform-provider-aws#35870

I believe it's valuable to have our tests break when changes like this are introduced in the providers. Therefore, we should try have behaviours such as this encoded in our tests. Things like: "invalid values for property X are not validated by TF".

Checklist:

  • Have you added Release Notes in the docs repositories?
  • Have you ran make run-integration-tests and make run-terraform-tests?
  • Have you ran acceptance tests for the service under change?
  • Have you followed the Conventional Commits specification?

[#187103734](https://www.pivotaltracker.com/story/show/187103734)

Recently, a new PR for validating this property was merged:
hashicorp/terraform-provider-aws#35870

I believe it's valuable to have our tests break when changes
like this are introduced in the providers. Therefore, we should
try have behaviours such as this encoded in our tests. Things
like: "invalid values for property X are not validated by TF".
@fnaranjo-vmw fnaranjo-vmw force-pushed the fix-performance-insights-retention-period-#187103734 branch from 5ccf70d to ef9c9c9 Compare February 26, 2024 11:24
@fnaranjo-vmw fnaranjo-vmw merged commit bf284f2 into main Feb 26, 2024
6 checks passed
@fnaranjo-vmw fnaranjo-vmw deleted the fix-performance-insights-retention-period-#187103734 branch February 26, 2024 12:31
fnaranjo-vmw added a commit that referenced this pull request Feb 27, 2024
[#187061563](https://www.pivotaltracker.com/story/show/187061563)

Related PRs:
- #1600

By adding this test, if such validation ever gets implemented
in the provider we'll know it since the test will start failing
as happened for:
#1597
fnaranjo-vmw added a commit that referenced this pull request Feb 27, 2024
* fix: test fifo properties are validated by the IAAS not TF

[#187061563](https://www.pivotaltracker.com/story/show/187061563)

Related PRs:
- #1600

By adding this test, if such validation ever gets implemented
in the provider we'll know it since the test will start failing
as happened for:
#1597

* Remove failing assertions

These assertions were added in the following PR,
but they are make the pipeline fail (probably because those fields
are not explicitly null but empty instead)
#1600

---------

Co-authored-by: Felisia Martini <fmartini@pivotal.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

1 participant