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 several integration tests and related code #234

Merged
merged 1 commit into from
Jun 13, 2019
Merged

Fix several integration tests and related code #234

merged 1 commit into from
Jun 13, 2019

Conversation

bkabrda
Copy link
Contributor

@bkabrda bkabrda commented Jun 12, 2019

  • Set datadog_downtime.timezone attribute in resourceDatadogDowntimeRead,
    handle it in import test
  • Make datadog_downtime.tags a TypeSet to be able to properly ignore
    differences in ordering of its contents
  • Fix two regressions caused by calling resourceDatadogMonitorRead from
    resourceDatadogMonitorUpdate and resourceDatadogMonitorCreate:
    • Make sure that API sometimes changing "metric alert" to "query alert"
      continues to be ignored
    • Fix tests for threshold values to always expect floats

The last change should be mentioned in the changelog, because that will actually create a non-empty plan for people who have used integers as values for thresholds previously - with next apply, these are all going to change to floats (one-time only). It's not really a regression, since the values will just change their type, but still noteworthy IMO.

@ghost ghost added the size/M label Jun 12, 2019
Copy link
Contributor

@nmuesch nmuesch left a comment

Choose a reason for hiding this comment

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

Overall looks good! Thanks for fixing up these 💜 Couple of small questions
For the float -> int thing. We're just changing the test here, that one time diff would still happen before these changes right? If thats the case I don't think we need to call it out in the changelog.

@bkabrda bkabrda mentioned this pull request Jun 13, 2019
@bkabrda
Copy link
Contributor Author

bkabrda commented Jun 13, 2019

Yeah, you're probably right about that one-time diff, so let's not put it into the changelog.

* Set datadog_downtime.timezone attribute in resourceDatadogDowntimeRead,
  handle it in import test
* Make datadog_downtime.tags a TypeSet to be able to properly ignore
  differences in ordering of its contents
* Fix two regressions caused by calling resourceDatadogMonitorRead from
  resourceDatadogMonitorUpdate and resourceDatadogMonitorCreate:
  * Make sure that API sometimes changing "metric alert" to "query alert"
    continues to be ignored
  * Fix tests for threshold values to always expect floats
Copy link
Contributor

@nmuesch nmuesch left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 Thanks!

@bkabrda
Copy link
Contributor Author

bkabrda commented Jun 13, 2019

Thanks for the review! Merging.

@bkabrda bkabrda merged commit ace799f into DataDog:master Jun 13, 2019
jbenais pushed a commit to jbenais/terraform-provider-datadog that referenced this pull request Aug 20, 2019
Fix several integration tests and related code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants