Provide a Testcontainer for Neo4j.#993
Conversation
michael-simons
requested review from
bsideup,
kiview and
rnorth
as code owners
kiview
left a comment
kiview
left a comment
There was a problem hiding this comment.
Hey @michael-simons,
great PR and cool to see, that you are already using Testcontainers.
Just some small remarks, I'll give it a proper review later.
modules/neo4j/src/main/java/org/testcontainers/containers/Neo4jContainer.java
Outdated
Show resolved
Hide resolved
modules/neo4j/src/main/java/org/testcontainers/containers/Neo4jContainer.java
Outdated
Show resolved
Hide resolved
modules/neo4j/src/main/java/org/testcontainers/containers/Neo4jContainer.java
Outdated
Show resolved
Hide resolved
...s/neo4j/src/test/java/org/testcontainers/containers/Neo4jContainerEnterpriseEditionTest.java
Outdated
Show resolved
Hide resolved
modules/neo4j/src/main/java/org/testcontainers/containers/Neo4jContainer.java
Outdated
Show resolved
Hide resolved
rnorth
left a comment
rnorth
left a comment
There was a problem hiding this comment.
This is a really nice PR, and a great addition to what is available for developers. Thank you.
I just have a couple of queries (pretty minor), but in general I think we'll be happy to accept this soon.
Cheers!
modules/neo4j/src/test/java/org/testcontainers/containers/Neo4jContainerTest.java
Outdated
Show resolved
Hide resolved
michael-simons
force-pushed
the
master
branch
2 times, most recently
from
e9697f6 to
72ba7ff
Compare
|
Thanks a lot for your detailed feedback everyone. I have used the provided mechanism for accepting licenses (only necessary for our enterprise license). I added AssertJ to check for exceptions in tests, also rewrote the older ones. Note that I rebased onto your master, so you don't have to deal with merges. |
|
I don't understand why CI fails now… |
|
I can't see a reason for that CI failure, but have re-triggered... |
|
Bizarrely it looks like we've suddenly been hit with this issue (or something like it): gradle/gradle#3318 This is obviously not a problem with the PR itself, so I'll have to investigate! |
|
Please could you rebase onto master/merge? I think we have fixed a strange problem on Travis CI in #1004, so this should resolve the failing build. |
Co-Authored-By: michael-simons <[email protected]>
…r-license-acceptance.txt to .gitignore. This allows the user or another developer to just copy our example into place.
michael-simons
force-pushed
the
master
branch
from
3dafbd0 to
1ba19b0
Compare
|
Done. |
|
The test failure has nothing to do with this PR - I'll tackle that with #1008. Please ignore! |
rnorth
left a comment
rnorth
left a comment
There was a problem hiding this comment.
This looks good to me now. Thank you for the contribution @michael-simons!
|
Fantastic! Thanks for accepting my contribution. |
3 participants
I'd like to propose a Testcontainer for Neo4j, an open source graph database as in this pull request.
The test container is build with our official Docker image and provides the following features to the user:
While we offer a test harness and users may opt to use an embedded version of our database during test, it seems that many people think it's a better practice to test any database access against "the real thing".
The container would be useful to us and allegedly to some other projects using Neo4j as well.
We have been using a container very similar while spiking our efforts of a reactive client for Neo4j.