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

test/cloudflare_record: Fix acceptance LOC tests #217

Merged

Conversation

jacobbednarz
Copy link
Member

This updates the resourceCloudflareRecordRead function to also run the
data attribute through the transformToCloudflareDNSData function
(like the Create/Update already do) in order to correctly parse the
floats that are returned as strings.

I've opened a discussion with the Cloudflare folks on fixing this
properly (not passing the floats as strings) in the response but I
suspect that won't be a quick fix and this will need to suffice while we
are working through that.

Fixes the following CI failure

------- Stdout: -------
=== RUN   TestAccCloudflareRecord_LOC
=== PAUSE TestAccCloudflareRecord_LOC
=== CONT  TestAccCloudflareRecord_LOC
--- FAIL: TestAccCloudflareRecord_LOC (2.98s)
    testing.go:527: Step 0 error: Check failed: Check 2/4 error: cloudflare_record.foobar: Attribute 'value' expected "37 46 46.000 N 122 23 35.000 W 0m 100m 0m 0m", got "37 46 46.000 N 122 23 35.000 W 0.00m 100.00m 0.00m 0.00m"
FAIL

FYI @radeksimko

This updates the `resourceCloudflareRecordRead` function to also run the
`data` attribute through the `transformToCloudflareDNSData` function
(like the `Create`/`Update` already do) in order to correctly parse the
floats that are returned as strings.

Fixes the following CI failure

```
------- Stdout: -------
=== RUN   TestAccCloudflareRecord_LOC
=== PAUSE TestAccCloudflareRecord_LOC
=== CONT  TestAccCloudflareRecord_LOC
--- FAIL: TestAccCloudflareRecord_LOC (2.98s)
    testing.go:527: Step 0 error: Check failed: Check 2/4 error: cloudflare_record.foobar: Attribute 'value' expected "37 46 46.000 N 122 23 35.000 W 0m 100m 0m 0m", got "37 46 46.000 N 122 23 35.000 W 0.00m 100.00m 0.00m 0.00m"
FAIL
```
@ghost ghost added the size/S label Feb 26, 2019
@jacobbednarz
Copy link
Member Author

This gets all the Cloudflare record acceptance tests passing

$ CLOUDFLARE_DOMAIN="$CF_DOMAIN" \
  CLOUDFLARE_EMAIL="$CF_EMAIL" \
  CLOUDFLARE_TOKEN="$CF_TOKEN" \
  TF_ACC=1 \
  go test -timeout 30s github.com/terraform-providers/terraform-provider-cloudflare/cloudflare -v -run "^(TestAccCloudflareRecord_Basic|TestAccCloudflareRecord_CaseInsensitive|TestAccCloudflareRecord_LOC|TestAccCloudflareRecord_SRV|TestAccCloudflareRecord_Proxied|TestAccCloudflareRecord_Updated|TestAccCloudflareRecord_typeForceNewRecord|TestAccCloudflareRecord_hostnameForceNewRecord|TestAccCloudflareRecord_CreateAfterManualDestroy|TestAccCloudflareRecord_TtlValidation|TestAccCloudflareRecord_TtlValidationUpdate)$"
=== RUN   TestAccCloudflareRecord_Basic
=== PAUSE TestAccCloudflareRecord_Basic
=== RUN   TestAccCloudflareRecord_CaseInsensitive
=== PAUSE TestAccCloudflareRecord_CaseInsensitive
=== RUN   TestAccCloudflareRecord_LOC
=== PAUSE TestAccCloudflareRecord_LOC
=== RUN   TestAccCloudflareRecord_SRV
=== PAUSE TestAccCloudflareRecord_SRV
=== RUN   TestAccCloudflareRecord_Proxied
=== PAUSE TestAccCloudflareRecord_Proxied
=== RUN   TestAccCloudflareRecord_Updated
=== PAUSE TestAccCloudflareRecord_Updated
=== RUN   TestAccCloudflareRecord_typeForceNewRecord
=== PAUSE TestAccCloudflareRecord_typeForceNewRecord
=== RUN   TestAccCloudflareRecord_hostnameForceNewRecord
=== PAUSE TestAccCloudflareRecord_hostnameForceNewRecord
=== RUN   TestAccCloudflareRecord_CreateAfterManualDestroy
=== PAUSE TestAccCloudflareRecord_CreateAfterManualDestroy
=== RUN   TestAccCloudflareRecord_TtlValidation
=== PAUSE TestAccCloudflareRecord_TtlValidation
=== RUN   TestAccCloudflareRecord_TtlValidationUpdate
=== PAUSE TestAccCloudflareRecord_TtlValidationUpdate
=== CONT  TestAccCloudflareRecord_Basic
=== CONT  TestAccCloudflareRecord_TtlValidation
=== CONT  TestAccCloudflareRecord_TtlValidationUpdate
=== CONT  TestAccCloudflareRecord_Proxied
=== CONT  TestAccCloudflareRecord_CreateAfterManualDestroy
--- PASS: TestAccCloudflareRecord_TtlValidation (0.02s)
--- PASS: TestAccCloudflareRecord_Basic (3.41s)
=== CONT  TestAccCloudflareRecord_hostnameForceNewRecord
--- PASS: TestAccCloudflareRecord_Proxied (4.46s)
=== CONT  TestAccCloudflareRecord_typeForceNewRecord
--- PASS: TestAccCloudflareRecord_TtlValidationUpdate (4.55s)
=== CONT  TestAccCloudflareRecord_Updated
--- PASS: TestAccCloudflareRecord_CreateAfterManualDestroy (5.65s)
=== CONT  TestAccCloudflareRecord_LOC
--- PASS: TestAccCloudflareRecord_LOC (4.50s)
=== CONT  TestAccCloudflareRecord_SRV
--- PASS: TestAccCloudflareRecord_hostnameForceNewRecord (7.08s)
--- PASS: TestAccCloudflareRecord_typeForceNewRecord (6.54s)
=== CONT  TestAccCloudflareRecord_CaseInsensitive
--- PASS: TestAccCloudflareRecord_Updated (7.63s)
--- PASS: TestAccCloudflareRecord_SRV (2.97s)
--- PASS: TestAccCloudflareRecord_CaseInsensitive (4.40s)
PASS
ok  	github.com/terraform-providers/terraform-provider-cloudflare/cloudflare	15.416s

Copy link
Contributor

@radeksimko radeksimko 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 👍

@@ -8,6 +8,7 @@ IMPROVEMENTS:

BUG FIXES:

* resource/cloudflare_record: Ensure `Create`/`Update`/`Read` functions all apply the record transformation and store the correct state values [GH-217]
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big deal, but we usually modify Changelog after merging PRs, via the GitHub UI (pencil icon). It helps us to avoid conflicts in case we merge other PRs whilst one is being reviewed.

@radeksimko radeksimko merged commit c2f13cf into cloudflare:master Feb 26, 2019
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