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

Unable to mock ManagedTransaction interface for testing #527

Open
adrianiacobghiula opened this issue Sep 1, 2023 · 5 comments
Open

Unable to mock ManagedTransaction interface for testing #527

adrianiacobghiula opened this issue Sep 1, 2023 · 5 comments

Comments

@adrianiacobghiula
Copy link

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link

@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
Copy link
Contributor

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.

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

No branches or pull requests

3 participants