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

Implement assertions to work with Neo4j Java Driver #75

Open
fbiville opened this issue May 26, 2020 · 7 comments
Open

Implement assertions to work with Neo4j Java Driver #75

fbiville opened this issue May 26, 2020 · 7 comments

Comments

@fbiville
Copy link
Collaborator

This will allow Assertj-Neo4j to work in scenarios where tests run against a remote Neo4j instance (whether via testcontainers or a hosted standalone instance).

Note that the dependency on the Neo4j Java Driver should be optional, as people relying on Neo4j test harness do not need the driver at all and the other assertions should continue to work.

@fbiville
Copy link
Collaborator Author

fbiville commented May 26, 2020

With this issue, assertj-neo4j would end up with two "usage modes":

  1. the "historical" one where Assertj-Neo4j assumes users start an embedded Neo4j instance or an in-process server (typically via Neo4j test harness). It provides assertions on the main interfaces of Neo4j Java API.
  2. the new one where Neo4j is deployed separately (e.g. with Testcontainers) and the Neo4j Java driver is the main entry point. In this case, there will be assertions only on the single Result type (which is an iterable of Record, a glorified Map<String, Object>).

I think the integration testing trend with Neo4j is to rely on solutions such as Testcontainers and its Neo4j module (no more classpath conflicts etc etc).

Now, my question is: given these two "usage modes" are completely separate, does it make sense to have them in the same repo?

One advantage I see is the discoverability. People interested in writing assertions on Neo4j types have a single place to look at.

One drawback is that people using the "historical" mode do not care about the Java driver that they won't use at all for the existing assertions. Same applies for users of the new mode, where they only care about the Java driver and not about Neo4j Java APIs.

What do you think @joel-costigliola?

As a reference, you can see the excellent blogpost of @michael-simons about the different flavors of testing with Neo4j: https://medium.com/neo4j/testing-your-neo4j-based-java-application-34bef487cc3c (you can skip the SDN / OGM section).

Patouche added a commit to Patouche/assertj-neo4j that referenced this issue May 27, 2020
Patouche added a commit to Patouche/assertj-neo4j that referenced this issue May 27, 2020
@joel-costigliola
Copy link
Owner

joel-costigliola commented Aug 25, 2020

@fbiville is the Result class shared between the API and the driver?

I would go for a single library, users of the java driver will only see Result assertions as they will be calling assertThat(Result), even if there are a bunch of other assertion types, they won't be accessible through assertThat.
I think that's a good enough developer experience since there is no possible confusion about what is asserted.

@michael-simons
Copy link

Hey everyone.

org.neo4j.driver.Result (and related Async and Rx variants) are integral part of the Neo4j driver API, not of the test harness or embedded Neo4j. Using embedded Neo4j and it's Cypher capabilities, it would be org.neo4j.graphdb.Result. Those are not related and won't be related anytime soon.

The Result is indeed an iterator over Record. However, the Result is transactional bounded.

@fbiville
Copy link
Collaborator Author

Thanks for chiming in @michael-simons .
That's also what I understood, i.e. no overlap between embedded Neo4j and Driver APIs.

I'm undecided about the best path to follow. My gut feeling is that embedded Neo4j is a less and less relevant scenario to support, while Docker/TestContainers seems to be the prevalent choice nowadays.
We could go for a full rewrite to support only the latter as there has not been any recent releases anyway.
WDYT?

@michael-simons
Copy link

For application developers testing the real thing - aka server connection over bolt - is the preferred way to go. Doesn't matter if the test runs against a test database instance or test container.

For plugin developers, they must use embedded.

However, here's the catch: Even the embedded instance allows you to open up a bolt connection over local socket against which you can use the driver.

Thus, even more an argument for going the assertThat(org.neo4j.driver.Result).

Also: A plugin developer is probably much more interested in testing the output of their procedures. So they would maybe write unit tests for their plugin and assert the internal / embedded results in some other wise and than call their plugins through the driver and cypher and than assert that result.

Makes sense?

@Patouche
Copy link

OK, I will take that direction to implement this.

With the local socket on embedded instance, it's make perfect sense to use directly org.neo4j.driver.Result instead of the embedded one.

As @joel-costigliola said, we will make in mind to have the best developer experience for its assertions.

Thanks a lot for the feedback !

@michael-simons
Copy link

Thank you @Patouche and all others for working on that! Feel free to reach out with any other questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants