[DXCDT-12] Use wiremock within custom domain verification test #501

sergiught · 2022-01-25T20:19:38Z

Proposed Changes

In this PR we are introducing a wiremock container in order for us to not rely on digital ocean directly for verifying custom domains with tests.

We will now have the potential to apply the same strategy to multiple tests, specially ones that might require paid subscriptions.

Acceptance Test Output

$ make testacc TESTS=TestAccXXX

=== RUN   TestAccCustomDomainVerification
--- PASS: TestAccCustomDomainVerification (0.46s)
PASS

Process finished with the exit code 0

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

willvedd

Great work, this definitely pushes us forward and will empower others to contribute with confidence.

I've made a bunch of small comments, mostly around naming and clarity. However the one major point I'd like to explore is the abstraction of the provider initialization so that we don't need to declare the schema twice to accommodate two sets of configuration. Instead, I think it would be wise to have a single provider initialization function that accepted some configuration. That way we are only defining and updating that schema in a single place and we don't need to hardcode any configuration.

willvedd · 2022-01-26T13:05:14Z

CONTRIBUTING.md

@@ -40,11 +46,10 @@ AUTH0_CLIENT_ID=<your-auth0-client-id>
 AUTH0_CLIENT_SECRET=<your-auth0-client-secret>
 ```

-Then, run `make testacc`. 
+Then, run `docker-compose up -d && make testacc`. 


Thoughts about bundling the docker-compose up directly into a separate command in the makefile? Perhaps make testacc_local? The makefile does a good job of hiding away a lot of the implementation details and we don't want to require folks to need to know to run the docker compose locally.

However, this would just be for local invocation, I do like that it's separated in the Github workflow above.

Further, you're omitting the --build arg here whereas it exists in the workflow, I forget if building the image is a default behavior of up but would think that we'd want to be consistent in both places.

Sure, we can add a makefile command to bootstrap the dev env.

When docker-compose up is ran and the image is not available on the host machine it will always default to building it. The --build is there just to always force rebuilding and not use the one from cache.

willvedd · 2022-01-26T13:16:06Z

CONTRIBUTING.md


 **Note:** The acceptance tests make calls to a real Auth0 tenant, and create
-real resources. Certain tests, for example for custom domains
-(`TestAccCustomDomain`), also require a paid Auth0 subscription to be able to
+real resources. Certain tests also require a paid Auth0 subscription to be able to


Can we list which ones specifically so folks could know to expect those failing tests?

willvedd · 2022-01-26T13:20:24Z

auth0/provider_test.go

 	"github.com/hashicorp/terraform-plugin-sdk/terraform"

 	"gopkg.in/auth0.v5/management"
 )

+func providerWithWiremock() *schema.Provider {


While Wiremock is a testing utility we're using, it may not be descriptive at first glance. Would you consider something like providerWithMockedRequests, mockedProvider or something similar?

Updated it to providerWithTestingConfiguration.

willvedd · 2022-01-26T13:26:25Z

auth0/provider_test.go

+		},
+	}
+}
+


This entire function is awfully similar to the init function in provider.go. It appears that the only differences are in configuration being passed in. Would you consider introducing a function that instantiates a provider with the configuration as an argument? Doing so would allow us to only need to instantiate and modify the provider schema in one location, as opposed to two for any modifications of resources. It would also mean that we don't need to hardcode the mocked service's host.

willvedd · 2022-01-26T13:28:04Z

auth0/resource_auth0_custom_domain_verification_test.go

 	resource.Test(t, resource.TestCase{
 		Providers: map[string]terraform.ResourceProvider{
-			"auth0":        Provider(),
-			"digitalocean": digitalocean.Provider(),
+			"auth0": providerWithWiremock(),


Checking my understanding here, this is the exact spot where we'd swap the real provider with the provider with mocked requests, when we adjust the tests going forward?

Yep, that's correct:)

willvedd · 2022-01-26T13:39:05Z

CONTRIBUTING.md

 ## Getting started

 To work on the provider, you'll need [Go](http://www.golang.org) installed on
-your machine (version 1.10+ is *required*). You'll also need to correctly setup
+your machine (version 1.13+ is *required*). You'll also need to correctly set up


This is updated to 1.13 to support the usage of modules, correct?

Yep, that's the main reason I bumped it.

willvedd · 2022-01-26T13:44:51Z

CONTRIBUTING.md

+## Prerequisites
+
+- [Go 1.13+](https://go.dev/)
+- [Docker](https://docs.docker.com/get-docker/)
+- [Docker-Compose](https://docs.docker.com/compose/install/)


Can we framed these as dependencies for testing? Otherwise it may appear that docker and docker compose are fundamental to the contribution.

Well in a way they are fundamental to the contribution, to ensure that all tests are passing, or if we want to add new test cases for new functionality we will need the mocks as well.

Edit: Made it more clear tho that those are used within testing.

willvedd

Solid work!

sergiught force-pushed the patch/DXCDT-12-wiremock branch from a5f5b3a to 9b76d93 Compare January 26, 2022 08:33

sergiught added 4 commits January 26, 2022 10:10

Add wiremock container

b263df5

Remove digitalocean

2ebadea

Update CONTRIBUTING.md

1ce30a7

Use wiremock in custom domain verification test

7a7ecd1

sergiught changed the base branch from master to patch/fix-logstream-tests January 26, 2022 09:28

sergiught marked this pull request as ready for review January 26, 2022 09:28

sergiught requested review from alexkappa and willvedd January 26, 2022 09:28

sergiught self-assigned this Jan 26, 2022

sergiught force-pushed the patch/DXCDT-12-wiremock branch from 9b76d93 to 7a7ecd1 Compare January 26, 2022 09:44

willvedd reviewed Jan 26, 2022

View reviewed changes

Base automatically changed from patch/fix-logstream-tests to master January 26, 2022 14:03

sergiught added 2 commits January 26, 2022 18:12

Refactor provider so it's not accessible globally any more

a43de93

Add make command for running docker-compose up

2d85f12

sergiught requested a review from willvedd January 26, 2022 17:14

Improve contributing.md

b7096d5

sergiught force-pushed the patch/DXCDT-12-wiremock branch from eefc558 to b7096d5 Compare January 26, 2022 17:19

willvedd approved these changes Jan 26, 2022

View reviewed changes

sergiught merged commit 29fbde4 into master Jan 26, 2022

sergiught deleted the patch/DXCDT-12-wiremock branch January 26, 2022 21:46

Uh oh!

[DXCDT-12] Use wiremock within custom domain verification test #501

[DXCDT-12] Use wiremock within custom domain verification test #501

Uh oh!

Conversation

sergiught commented Jan 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Acceptance Test Output

Community Note

Uh oh!

willvedd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergiught Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

willvedd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sergiught commented Jan 25, 2022 •

edited

Loading

sergiught Jan 26, 2022 •

edited

Loading