DATA-DOG / go-sqlmock

Sql mock driver for golang to test database interactions
Other
6.02k stars 406 forks source link

Options for non-strict mock - allow unexpected calls #259

Closed serbrech closed 2 months ago

serbrech commented 3 years ago

While strict mock is very valuable, it's also very restrictive in how we can use this library. strict mocks force all expectation to be met, and also that anything that happens on the mock was expected.

This makes tests break for unrelated changes, and encourages tests that verify a lot of behaviors in a single test. for me, testing that a transaction gets commited in a specific case, is a different test than testing what query was sent.

It would be nice if we could add an option to sqlmock.New() that relaxes the expectations.

sqlmock.New(sqlmock.AllowUnexpectedCalls()) // or something similar

I would like to only fail the test if the expectation that I have defined are not met, but ignore calls that were not expected.

l3pp4rd commented 3 years ago

What is the point of a test if it is not strict? How can devs push commits without updates for tests? And why then you write tests in the first place?

serbrech commented 3 years ago

You might want to test the behavior after the call to the DB, not the fact that the call was made. Unit tests are supposed to test one thing at a time.

Maybe I setup the DB with fake data, and run my tests, checking that I publish a message to Kafka. But I don't want necessarily to explicitly verify that each and every calls to the dB was made. And I don't necessarily want that the test breaks each time some code that touches the DB changes either.

serbrech commented 3 years ago

"How can devs push commits without updates for tests?" I don't think this is a welcome way to answer a suggestion/contribution. I'm not asking how a dev can write so-called unit tests when he is testing more than one behavior at a time.

l3pp4rd commented 3 years ago

well, my intention for this library is to be used for unit tests. if you are testing a unit which is too large and may have unexpected behaviors in terms of interactions with db, then I do not think this library is suitable. If we add such features like, allowing repeatable calls or non explicit matching, then the library would turn into something different. The change you are proposing, is not correlating with the way I understand unit tests and I cannot accept it. I imagine there might be cases when it may be useful to do it in different way, but for that, there is a forking option if you do not fit in 90% of use cases. Adding that other 10% of corner cases, would make the library lose the purpose and become bloated and even confusing. I've built a few open source libraries, which are widely used and most often, if trying to satisfy all the possible use cases, it becomes more confusing and in the end unusable.

diegommm commented 2 months ago

Hi @serbrech! I understand your concern, you would be looking for something more like a noop driver instead. I think sqlmock is more tailored to the opposite, so it would appear that you're trying to test several layers of abstraction at the same time. To that extent, I would suggest the following approach, which is something I typically do:

This may be too abstract, but I hope it helps. This is obviously one testing strategy, and it can be used very well with sqlmock and many other mocking libraries. When you see this becoming too big, it might be a good sign that you have too much responsibility concentrated, and you may benefit from using interfaces and mocking them. By using mocks of interfaces, you can abstract away you repository code (i.e. the code accessing the database) from the code that does other operations, and you no longer need sqlmock for that.

sqlmock is tailored to be a good database-level mocking tool. Higher level operations where lower level database operations may not be important is a good sign that you may benefit from repository interfaces, which you can read about elsewhere on the internet. In the meantime, I think that making calls optional is not something that goes with the spirit of this library for the reasons above, so closing this issue.

Thank you for your feedback!