neo4j/neo4j-go-driver

(v5) WithContext types are troublesome to mock

lukestoward opened this issue · 6 comments

I've been experimenting with the Context aware v5.0.0 preview and I've ran into some issues when trying to write unit tests.

In version 4.X of the Go package interface types such as Result, Session and Transaction have exported methods only. Which can easily be mocked using tools such as vektra/mockery. However, when using version 5.0.0-preview, the new context aware types ResultWithContext, SessionWithContext and TransactionWithContext contain new unexported methods such as legacy() which in some cases return unexported types such as *result.

Whilst mockery can still create mocks for the interfaces, I am unable to use the mock ResultWithContext as the legacy() method has an unexported return value *result.

I get the following error when building using the mock of ResultWithContext:

mocks/ResultWithContext.go:398:46: result not exported by package neo4j

Would it be possible to remove the unexported methods on those interfaces, or more importantly, remove the unexported return value *result.

If you would like to generate the mock yourself, run the following:

$ go install github.com/vektra/mockery/v2@latest
$ mockery --with-expecter --exported --dir $(go list -m -f '{{ .Dir }}' github.com/neo4j/neo4j-go-driver/v5)/neo4j --name ResultWithContext --output mocks  

@fbiville any thoughts on this?

Hi @lukestoward, first of all: thanks for trying out the preview version and providing feedbck!

The unexported legacy method is needed so that the legacy APIs can easily delegate to the new context-aware APIs. Let me try to see if I can at least change the return type to a public interface.

Note that the hypothetical fix would only applied to the 5.0 branch, which contains many more changes now.

Hi @fbiville, that shouldn't be a problem. Thanks for taking a look and let me know how you get on.

Hello, I think #379 fixes it but it would be great if you could confirm it.

Hi @fbiville, this does indeed look like it's fixed the issue.

One caveat is that I am still unable to use vektra/mockery as there is an issue with providing an implementation for an interface with unexported methods. This is more a limitation with mockery, since it is possible to satisfy an interface with unexported methods in a different package by embedding the interface directly in your type. That said it's a minor thing, but I will have to manually implement my own mocks. (unless I'm missing an option with the mock-gen tool 🤔)

Anyway, thank you for addressing this issue. 👍

Back from sick leave 🐛
Unfortunately, I need these unexported methods on the interface, at least as long as the legacy "non-context.Context" API are around, so most likely for the whole lifetime of 5.x versions.