Skip to content

helper/resource: Add ParallelTest() function to allow opt-in acceptance testing concurrency with t.Parallel() #18688

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Sep 13, 2018

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Aug 15, 2018

Please note: this is an alternate implementation to #16807, using a separate function rather than requiring adjustment to each existing TestCase and submitted just for an additional reference point. The preferred solution may be either implementation or a combination of the two! I think more importantly we should continue the conversation for supporting this type of functionality as its been implemented or requested by a few of the major providers. 😄

While this initial implementation is a very simple wrapper function, implementing this in the helper/resource package provides some downstream benefits:

  • Provides a standard interface for plugin developers to enable parallel acceptance testing
  • Existing plugins can simply convert resource.Test to resource.ParallelTest references (as appropriate) to enable the functionality, rather than worrying about additional line(s) to each acceptance test function or TestCase
  • Potential enhancements to ParallelTest (e.g. adding an environment variable to skip enabling the behavior) are consistently propagated

…ce testing concurrency with t.Parallel()

While this initial implementation is a very simple wrapper function, implementing this in the helper/resource package provides some downstream benefits:
* Provides a standard interface for plugin developers to enable parallel acceptance testing
* Existing plugins can simply convert resource.Test to resource.ParallelTest references (as appropriate) to enable the functionality, rather than worrying about additional line(s) to each acceptance test function or TestCase
* Potential enhancements to ParallelTest (e.g. adding an environment variable to skip enabling the behavior) are consistently propagated
@paddycarver
Copy link
Contributor

I can't believe I didn't think of that before adding t.Parallel() to hundreds of tests. 😓

@MalloZup
Copy link

MalloZup commented Sep 10, 2018

@bflad what is the next step for this PR to get merged? i really like this feature and could help us in our terraform libvirt provider having this to the schema helper

Thank you for your attention. Have nice day.! 🌞 🌻

@bflad bflad requested a review from a team September 10, 2018 16:31
Copy link
Contributor

@appilon appilon left a comment

Choose a reason for hiding this comment

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

This seems reasonable to me, providers can opt-in 1 test at a time. I'm not an authority on core though so I would prefer see another approval maybe? But I will get it started.

Copy link
Contributor

@apparentlymart apparentlymart left a comment

Choose a reason for hiding this comment

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

This doesn't have any implications for Terraform Core, but indeed LGTM!

@MalloZup
Copy link

Thanks :love:

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@bflad
Copy link
Contributor Author

bflad commented Sep 13, 2018

Thanks, everyone!! 🚀

@bflad bflad merged commit a47583d into master Sep 13, 2018
@bflad bflad deleted the f-resource-paralleltest branch September 13, 2018 15:25
bflad added a commit that referenced this pull request Sep 13, 2018
@MalloZup
Copy link

🚀 🎸 🎉

@metacpp
Copy link

metacpp commented Sep 17, 2018

@bflad thanks for the fantastic work!

One more thing to take care: the output of testing result is a known issue for go test running in parallel. Please visit golang/go#19280 for the details. The proposal is to output test result in a structural (for example, JSON) format, see golang/go#2981. And it was released in v1.10 through go test -json, we need to remind provider developer for this.

@ghost
Copy link

ghost commented Apr 2, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 2, 2020
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants