neo4j / neo4j-go-driver

Neo4j Bolt Driver for Go
Apache License 2.0
490 stars 69 forks source link

Unable to mock ManagedTransaction interface for testing #527

Open adrianiacobghiula opened 1 year ago

adrianiacobghiula commented 1 year ago

Driver version: Go driver v5.12.0

Interfaces in neo4j package like ManagedTransaction have a "non-exported method" -> legacy() Transaction https://github.com/neo4j/neo4j-go-driver/blob/5.0/neo4j/transaction_with_context.go#L33

If one would want to mock this ManagedTransaction interface will end up in errors like Type cannot implement 'ManagedTransaction' as it has a non-exported method and is defined in a different package

it is not the only interface having these "non-exported method"(s)

Main issue appears trying to unit test something like greeting, err := session.ExecuteWrite(ctx, func(transaction neo4j.ManagedTransaction) (any, error) {

StephenCathcart commented 1 year ago

Hi @adrianiacobghiula, thank you very much for the feedback.

An issue was raised a while back (see 378) which may provide some insight into why we have non-exported methods and the difficulty at the moment in removing these.

The short version is that a legacy function is needed so that the legacy APIs can easily delegate to the new context-aware APIs.

It was looking like these could be removed post 5.x, however, they have now grown with further non-exported methods which won't be removed once the legacy "non-context.Context" API is removed. @lukestoward provided a workaround by creating their own mocks due to the current limitation with mockery.

We're currently having some internal discussions about potential future options, such as publishling a separate package like neo4j-testing where we provide stubs so people don’t have to deal with this. I will keep you posted if any progress is made one this.

adrianiacobghiula commented 1 year ago

hi @StephenCathcart , I found a manual solution for the mock generator mockgen github.com/neo4j/neo4j-go-driver/v5/neo4j SessionWithContext to explicit say the mock struct implements neo4j.SessionWithContext like this

// MockSessionWithContext is a mock of SessionWithContext interface.
type MockSessionWithContext struct {
    ctrl     *gomock.Controller
    recorder *MockSessionWithContextMockRecorder
    // MANUAL CHANGE: added next line
    neo4j.SessionWithContext
}

I am looking to see if it makes sense to request a change on golang/mock generation to always have this in case the Interface has non-exported methods

StephenCathcart commented 1 year ago

Hi @adrianiacobghiula, thank you for providing a workaround to the mock generator.

I'll close this issue for now given that we can at least work around it, and I'll communicate any future changes regarding our own testing package (if we go down that route), or any other solutions that will make mocking the driver easier.

sonnysideup commented 6 months ago

@StephenCathcart Any update on a neoj4-testing package. My team is about to start using the neo4j Go driver and I need to mock out a few fakes, and this task looks a daunting given the size and complexity of all the interfaces.

StephenCathcart commented 6 months ago

Hey @sonnysideup! As of now, our team has not had the opportunity to explore the development of a dedicated neo4j testing package. However, recognizing the demand and potential impact it could have on users like yourself, I am reopening this issue to maintain transparency and will discuss it with the team to see if we can prioritise its development.

In the meantime, it would be incredibly helpful if you could provide some specific examples or scenarios you're struggling with. This information will enable us to better understand your needs and tailor any potential solutions more closely to the challenges being faced.