terraform-provider-cloudamqp icon indicating copy to clipboard operation
terraform-provider-cloudamqp copied to clipboard

Published 20 hours ago • verified publisher icon cloudamqp

Testing with Go VCR and stored fixtures

Open tbroden84 opened this issue 1 year ago • 7 comments

WHY are these changes introduced?

During development to have the confidence to change, update and add new features, improvements, fixes. Have as much as possible of the Provider covered by tests. For this Terraform Acceptance test (extension of Go test) can be used. Downside since all our test require infrastructure resources which takes time to create, update and clean up. With the use of Go VCR, it's possible to record the request/responses done to API backend. Then reply these instead of doing the real requests, speed up each test significantly.

Requires: https://github.com/84codes/go-api/pull/48

WHAT is this pull request doing?

  • Adapt the Provider to be able to use Go VCR via http client when running acceptance test.
    • Record request/responses to fixtures
    • Replay requests/responses from fixtures
  • Separate resources configurations to be appended.
  • Template configuration with dynamic values.

HOW can this pull request be tested?

Run the test created in this PR against the stored fixtures (cassettes).

tbroden84 avatar Feb 05 '24 13:02 tbroden84

Strange behaviour on vpc_gcp_peering test. During recording state it succeed but fails during playback. Says it's missing interaction for /api/plans and /api/regions used for online validation during terraform plan for instances. They are there in the fixture cassette in the initial requests.

However seems to be an overall issue using two instances in one test. State-data are getting mixed up, first gets seconds attributes and vice verse.

tbroden84 avatar Mar 06 '24 15:03 tbroden84

Data sources and resources that covered with basic tests. Data sources:

  • alarm
  • nodes
  • notification

Resources:

  • alarm
  • aws_eventbridge
  • extra_disk_size
  • integration_log
  • integration_metrics
  • instance
  • node_actions
  • notification
  • plugin
  • plugin_community
  • rabbitmq_configuration
  • security_firewall
  • vpc_connect
  • webhook

tbroden84 avatar Mar 13 '24 09:03 tbroden84

Those that are skipped: Data sources:

  • account_vpcs
  • account
  • credentials
  • instance
  • plugin_community
  • plugin
  • upgradable_version
  • vpc_gcp_info
  • vpc_info

Resources:

  • account_actions
  • custom_domain
  • upgrade_rabbitmq
  • vpc_gcp_peering (passes initial recording but fails during playback)
  • vpc_peering

tbroden84 avatar Mar 13 '24 09:03 tbroden84

Test run is very fast now :) great work!

magnushoerberg avatar Mar 15 '24 11:03 magnushoerberg

Needed to run go mod tidy so update needed for go.mod and go.sum?

Will be solved once go-api is bumped with: https://github.com/84codes/go-api/pull/48

tbroden84 avatar Mar 15 '24 12:03 tbroden84

Looks better now with go.mod/go.sum.

tbroden84 avatar Mar 15 '24 12:03 tbroden84

I get this error:

dotenv -f .env go test -v ./cloudamqp/ ...

Hmm, mine TestAccIntegrationLog_Basic test passes. Running TF_ACC=1 dotenv -f .env go test -v ./cloudamqp/ otherwise the tests will be skipped.

Perhaps also need to share the .env for all tokens in log/metric integration.

Edit: tokens are sanitized so during playback not needed in the .env file (only during recording). Updated how access_key_id is handled.

tbroden84 avatar Mar 15 '24 12:03 tbroden84

@dentarg, @magnushoerberg after earlier discussion, all tests are active in the CI workflow. Also went over refactoring the sanitize of sensitive API key/token data (used by log & metrics integrations), re-recorded all tests and other smaller fixes.

Could anyone of you please just double check that it looks ok? Tried cover as much of the resources as possible with at least one basic test (create, import, delete) as a beginning. Then we can extend them over time. Would be nice to get this in place to further check the Terraform Plugin SDK v2 upgrade and move in go-api into the provider.

Notes: Plugins and community plugins both tests takes 20 s each, due to sleeps. Tried to filter out the request we are waiting on, but couldn't find a good way to catch the correct data. All plugins will be fetched and filtered out based on the name of the plugin being enable. However don't have that information when building the shouldSaveHook.

Webhook, re-recording keeps on failing due to timeout in the backend. Will have to update go-api to handle this, together that we also support updates. See this as a separated PR specific for this with a new recording.

tbroden84 avatar Apr 23 '24 09:04 tbroden84

Plugins and community plugins both tests takes 20 s each, due to sleeps.

We could make this configurable for users, and use it ourselves in the tests? (could be a follow-up PR if so)

dentarg avatar Apr 23 '24 09:04 dentarg

Plugins and community plugins both tests takes 20 s each, due to sleeps.

We could make this configurable for users, and use it ourselves in the tests? (could be a follow-up PR if so)

Hmm both already have a configurable sleep, didn't think about. Will try run a new recording with lower sleep and see if this will speed up things.

tbroden84 avatar Apr 23 '24 09:04 tbroden84

Should be possible to try without re-recording?

dentarg avatar Apr 23 '24 10:04 dentarg

Yeah, while re-recording and without filter. We still have requests made between sleep, this halved the time playing the test. Used old recorded cassette with lower sleep in replay, lowered it down to about 2 s instead.

Updated the sleep and added a comment about using default sleep during record to both tests.

tbroden84 avatar Apr 23 '24 10:04 tbroden84

I see, when recording with a very small sleep value, there will be many requests made, not optimal. Nice with the comment for that. Could possible be configured via ENV? So there is one set of settings for recording, another set for running the tests. Something we can explore later on.

Can we do something about TestAccVpcConnect_Azure_Basic? Takes 10s

dentarg avatar Apr 23 '24 11:04 dentarg

Could possible be configured via ENV?

Good point.

Can we do something about TestAccVpcConnect_Azure_Basic? Takes 10s

Will check.

tbroden84 avatar Apr 23 '24 11:04 tbroden84

New basic test (create and import) for webhook resource, it has been updated in the provider. In order to test updating the webhook resource, the queue must exists. Otherwise the validation and state file will contain wrong information after the update. Would require a secondary provider, that can access message broker HTTP API.

tbroden84 avatar Apr 26 '24 13:04 tbroden84

How did you solve that? Did you add a queue by hand before recording?

dentarg avatar Apr 29 '24 05:04 dentarg

Async operation, so the initial create of the webhook will return "correct" value. Then update the status once all concurrent tries have been failing. While doing the update, this is not the case.

Would preferable be made with the queues, so the real scenario can be tested. Including update.

tbroden84 avatar Apr 29 '24 07:04 tbroden84

Never did a deep dive into the webhook application. Looks like we have an issue in the API handling.

tbroden84 avatar Apr 29 '24 07:04 tbroden84

Async operation, so the initial create of the webhook will return "correct" value.

Sounds good enough for this PR?

dentarg avatar Apr 29 '24 08:04 dentarg

Async operation, so the initial create of the webhook will return "correct" value.

Sounds good enough for this PR?

Yes I would say so, similar to other integrations tests that making sure it can be created in the API.

Did some investigation, used the wrong request body earlier today while double checking the API. Didn't make sense that the test should fail with update. Made adjustment and re-recording to also include the update step: https://github.com/cloudamqp/terraform-provider-cloudamqp/pull/257/commits/985c125d16c8ffa6a0408186f1bd3fb295e8c8d1

tbroden84 avatar Apr 29 '24 12:04 tbroden84