-
Notifications
You must be signed in to change notification settings - Fork 8
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
Testcontainers for Java: Add showcase examples for different scenarios #54
Conversation
b3224c9
to
a6287c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks!
Left some minor comments but please wait for comments from others
testing/testcontainers/java/src/main/java/io/crate/example/testing/Application.java
Outdated
Show resolved
Hide resolved
testing/testcontainers/java/src/main/java/io/crate/example/testing/Application.java
Outdated
Show resolved
Hide resolved
testing/testcontainers/java/src/test/java/io/crate/example/testing/TestJDBCURLScheme.java
Outdated
Show resolved
Hide resolved
...ng/testcontainers/java/src/test/java/io/crate/example/testing/TestSharedSingletonMatrix.java
Outdated
Show resolved
Hide resolved
...ng/testcontainers/java/src/test/java/io/crate/example/testing/TestSharedSingletonMatrix.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx @amotl !
Left some first comments, but I will also checkout the branch and give it a try locally as well.
testing/testcontainers/java/src/test/java/io/crate/example/testing/TestJDBCURLScheme.java
Outdated
Show resolved
Hide resolved
27a1fa4
to
67c7050
Compare
67c7050
to
5178c77
Compare
Wrap connection into the try-with-resources so that it closes Statement and result set, and will get closed itself at the end.
5178c77
to
310f397
Compare
@amotl I've pushed several changes/improvements, please take a look. (apologies, but It was easier than providing comments). I've also added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matriv: Thanks for giving this some more love. I've added a few remarks to acknowledge my learning from your adjustments, and further suggestions for improvements.
testing/testcontainers/java/src/main/java/io/crate/example/testing/Application.java
Outdated
Show resolved
Hide resolved
testing/testcontainers/java/src/main/java/io/crate/example/testing/Application.java
Show resolved
Hide resolved
testing/testcontainers/java/src/main/java/io/crate/example/testing/Application.java
Outdated
Show resolved
Hide resolved
testing/testcontainers/java/src/test/java/io/crate/example/testing/TestClassScope.java
Outdated
Show resolved
Hide resolved
testing/testcontainers/java/src/test/java/io/crate/example/testing/utils/TestingHelpers.java
Show resolved
Hide resolved
...ng/testcontainers/java/src/test/java/io/crate/example/testing/TestSharedSingletonMatrix.java
Outdated
Show resolved
Hide resolved
...iners/java/src/test/java/io/crate/example/testing/TestSharedSingletonEnvironmentVersion.java
Outdated
Show resolved
Hide resolved
testing/testcontainers/java/src/test/java/io/crate/example/testing/TestManual.java
Outdated
Show resolved
Hide resolved
...tainers/java/src/test/java/io/crate/example/testing/TestManualWithLegacyCrateJdbcDriver.java
Outdated
Show resolved
Hide resolved
testing/testcontainers/java/src/test/java/io/crate/example/testing/TestSharedSingleton.java
Outdated
Show resolved
Hide resolved
|
||
# Start CrateDB. | ||
docker run -it --rm --publish=4200:4200 --publish=5432:5432 \ | ||
crate:latest -Cdiscovery.type=single-node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did I miss something or the discovery-type
option is not passed to the JdbcDatabaseContainer ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for bringing this up.
In this particular case, outlining basic examples only, CrateDB will also happily start without any options, right? So it would be acceptable to omit the -Cdiscovery.type=single-node
option?
However, propagating command-line options to CrateDB will become important when aiming to use this in a multi-node scenario like what crate-java-testing provides per CrateTestCluster
1, right? Did you already take this into account, @matriv, or would this be up to a second iteration?
Footnotes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I haven't tried testcontainers in multi-node setup, don't even know if that's supported by their existing infrastructure.
- This test, doesn't have to do with testcontainers, the script starts a docker container manually and then runs the tests, connecting to the bound port of the container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CrateDB will also happily start without any options, right?
That is correct, it goes ahead with a "best-effort cluster bootstrapping" and it works.
Hi. I've just added a final commit per 32fb516, effectively carrying over the summary from the pull request description into the "What's inside" section of the README. Can you have another look at this? Thanks already! P.S.: The backlog items also outlined within the PR description have been converged into GH-55, so this would be ready for merging if you agree 1. Thanks. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you!
Merged. Thanks! |
About
Foundation for a potential tutorial about how to use Testcontainers for Java with CrateDB, and for re-using blueprints at, for example, crate/crate-jdbc#377.
=> README -- direct view
References
Details
The patch exercises different features of Testcontainers for Java with a basic query
SELECT * FROM sys.summits LIMIT 3;
.CRATEDB_VERSION
environment variable, suitable for running a test matrix on different versions of CrateDB, shared across multiple test classes.Backlog
Startup
Database provisioning
Other test frameworks
Testcontainers for Java also provides integrations for other test frameworks. Currently, all test cases are based on JUnit 4.
Footnotes
Sometimes it might be useful to define a container that is only started once for several test classes. There is no special support for this use case provided by the Testcontainers extension. Instead, this can be implemented using the Singleton pattern. See also Manual container lifecycle control » Singleton containers. ↩