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

Support running docker in tests on CircleCI build #1837

Merged
merged 6 commits into from Feb 3, 2020

Conversation

shakuzen
Copy link
Member

@shakuzen shakuzen commented Feb 3, 2020

We want to use Testcontainers for testing various functionality with real backends running. We need to run such tests on a machine type executor on CircleCI. This makes a separate CircleCI job to run those, using a newly added Gradle task dockerTest that only runs the tests tagged (with JUnit Jupiter's @Tag("docker")) with the docker label (see sample test). The default test task will not run the tests tagged with docker.

Unblocks things like #1429

Also upgrades to CircleCI's version 2.1 configuration which allows some deduplication of the executor, which was in order as different build images were being used in different jobs (by mistake). Also dedups the almost identical steps used in both the build and docker-test jobs.

We want to use Testcontainers for testing various functionality with real backends running. Hopefully the `setup_remote_docker` command allows this to work.
@shakuzen shakuzen added the type: task A general task label Feb 3, 2020
@shakuzen shakuzen added this to the 1.4.0 milestone Feb 3, 2020
@shakuzen
Copy link
Member Author

shakuzen commented Feb 3, 2020

Well, that didn't work at all (as mentioned in testcontainers/testcontainers-java#1014 (comment)). I'm going to take a new approach of using a JUnit Jupiter tag for tests that need docker support and excluding them from the default run. I'll make a separate machine executor that runs the docker tests in parallel. This will make it nicer for those building locally that don't have docker installed/running and for parallelizing the running of docker tests, which inherently have some extra overhead.

@shakuzen shakuzen changed the title Allow docker in docker for CircleCI build Support running docker in tests on CircleCI build Feb 3, 2020
Adds a `dockerTest` task to the build that runs the tests tagged with `docker`, which are excluded in the default `test` task. This will be used in the CircleCI build from a separate `machine` executor, which is required to use Testcontainers on CircleCI.
@shakuzen
Copy link
Member Author

shakuzen commented Feb 3, 2020

This seems to be working well. Only thing to maybe look into later is getting the docker-test job to run faster. It takes longer than the build that runs thousands of tests when it's only running 1 test. See https://circleci.com/workflow-run/892f1433-6a71-4b02-bdc6-da968442f3d9

I will update the pull request to remove the placeholder test tagged with @Tag("docker") which was only there to ensure this worked.

Please take a look @izeye and @jeqo. Let me know what you think, since you both have a pull request blocked by not being able to use testcontainers on our CI build.

Copy link
Contributor

@jeqo jeqo left a comment

Choose a reason for hiding this comment

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

LGMT. Though, no much I can say until I enable testcontainer tests on my PR 😅

@izeye
Copy link
Contributor

izeye commented Feb 3, 2020

@shakuzen Looks good to me, too. Thanks!

@shakuzen
Copy link
Member Author

shakuzen commented Feb 3, 2020

Thanks for the review. Let me know if you have any issues or feedback after rebasing your PRs on this post-merge.

@shakuzen shakuzen merged commit 7b5dbfe into micrometer-metrics:master Feb 3, 2020
@shakuzen shakuzen deleted the yo-dawg-docker branch February 3, 2020 16:11
izeye added a commit to izeye/micrometer that referenced this pull request Feb 3, 2020
shakuzen added a commit that referenced this pull request Feb 6, 2020
Tests the ElasticMeterRegistry against a running Elasticsearch instance in a docker container using testcontainers. Currently tested against Elasticsearch 6 and 7.

See gh-1837
Closes gh-1429

Co-authored-by: Adrian Cole <adriancole@users.noreply.github.com>
Co-authored-by: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants