Open alewgbl opened 4 years ago
Hi, I think defer will defer the close to happen after the assertion right? instead it should be called before the assertion
@l3pp4rd It's true that the defer
call will close the DB handle at the end of the function, however the behaviour is different depending on what's in that deferred function call.
Also, it doesn't make sense to me that I must close the DB handle after I make the assertion. What if some other error happens before the defer
call is made? I'm not so sure I'd get to log an error from the db.Close()
call at that point.
Well, I do not get why you are closing it in a test anyway. It is a mock database, it does not need to be closed. If the logic you are testing, is closing db, then you can make that expectation. Even when this method is deffered, you will call it usually from your test.
I actually do not know why it may close it before, maybe go garbage collector notices this in some way, or all conections were closed. But I do not see any problem here unless it is in the go db driver, maybe closing db has interesting behavior
have you investigated the behavior of close?
@l3pp4rd: I think @alewgbl is closing the db handle as per the example on the README.md
The thing going on here is:
db.Close()
is returning an error with the message "all expectations were already fulfilled, call to database Close was not expected"
(debugging this line passes 3 times)On the 3 cases, at the point of the assertion, all the expectation are met but then a later call to db.Close()
is executed and it doesn't know that we already asserted that expectations were met.
If you add a call to mock.ExpectClose()
before the assertion all the 3 cases fails because no db.Close()
was called before the assertion.
For what I understand, sqlmock is strict on the assertion of expectations. You must have a corresponding mock.ExpectXYZ()
for every DB interaction, otherwise executing that interaction will fail with an error saying something like "call to ... was not expected"
.
I've been experiencing this also, this is working for me:
func mockDB(t *testing.T) (*sql.DB, sqlmock.Sqlmock) {
t.Helper()
db, mock, err := sqlmock.New()
if err != nil {
t.Fatalf("an error '%s' was not expected when opening a stub database connection", err)
}
t.Cleanup(func() {
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("db expectations failed: %s", err)
}
// For some reason if we want to close the db, we also have to call
// this, if not the previous method fails 🤷♂️
mock.ExpectClose()
if err := db.Close(); err != nil {
t.Errorf("error closing the db: %w", err)
}
})
return db, mock
}
If I remove the mock.ExpectClose()
it fails randomly, and the same happens if I move this before the mock.ExpectationsWereMet
.
mock.ExpectClose()
if err := db.Close(); err != nil {
t.Errorf("error closing the db: %w", err)
}
Hi, very likely then that from some go version it now closes db in separate go routine and though synchronously we can no longer receive the close call to the driver. Which is something I would not expect since closing may give an error
I've found a case where calling
db.Close()
in different ways only sometimes satisfiesmock.ExpectClose()
. For example, if I callsqlmock.New()
and then calldefer db.Close()
on the DB handle that is returned,mock.ExpectClose()
is not satisfied. However if I perform a nil check on the deferred result ofdb.Close()
thenmock.ExpectClose()
is satisfied.I would expect all cases of
db.Close()
to be consistent: either satisfy or don't satisfymock.ExpectClose()
.My goal is to catch non-nil errors from
db.Close()
and ensure they are logged.Test cases showing the behaviour:
go version go1.12.7 linux/amd64 go-sqlmock v1.3.3