6799 - Adding support for Testcontainers #6800
Conversation
…me minor refactoring to extend the definition of integration tests. IQSS#6799
There was a problem hiding this comment.
Overall, I'm excited about the kind of testing that Testcontainers promises to give us. I added a couple questions about pom.xml changes. Thanks for spearheading this effort, @poikilotherm !
| @@ -765,6 +784,7 @@ | |||
| <configuration> | |||
| <!-- testsToExclude come from the profile--> | |||
| <excludedGroups>${testsToExclude}</excludedGroups> | |||
| <skip>${skipUnitTests}</skip> | |||
There was a problem hiding this comment.
What's going on here? It's <skip> instead of the <skipUnitTests> seen elsewhere? I think I'd rather see a smaller diff with this line removed. I would also suggest removing the <skipUnitTests>false</skipUnitTests> property above. I'm happy to talk this out in IRC.
There was a problem hiding this comment.
@poikilotherm and I talked this out. It's all set. Please see my summary at #6800 (review)
capitalize a word to match our style
There was a problem hiding this comment.
Looks good. @poikilotherm and I just talked out the skip unit tests thing. The long version is at http://irclog.iq.harvard.edu/dataverse/2020-04-08#i_121928
The short version "the only way to skip ONLY the unit tests is by switching them off via configuration" with the goal of NOT running unit tests (which get run all the time) when using Testcontainers. We don't want people hacking on Testcontainer tests to be bogged down by the time it takes unit tests to run. Also "skip" is legit in pom.xml.
I didn't run the code at all or test the new "tc" profile but @poikilotherm included instructions for doing so.
This pull request lays some groundwork. I'm looking forward to the future pull requests that make use of Testcontainers! 🎉
What this PR does / why we need it:
This PR introduces basic support for Testcontainers usage in integration tests using JUnit 5, which was mentioned in a few places like #6783 and talked about with @scolapasta and @qqmyers in a VC about #6694.
For now this is basically a docs change and a new Maven profile. Very basic, very "stay.out.of.my.way". Would be happy to rely on it for any PR on #6694 et al. Actually, integration testing some of authentication providers might be possible that way for the first time.
Which issue(s) this PR closes:
Closes #6799
Special notes for your reviewer:
Nada.
Suggestions on how to test this:
mvn -P tc verifyshould show 0 tests. (Obviously)Does this PR introduce a user interface change?:
Nada.
Is there a release notes update needed for this change?:
Depends. Maybe. Testcontainers is a huge and great thing, it might be neat to give @bsideup @kiview and @rnorth a shoutout when we really start to use this.
Additional documentation:
Provided in the PR.