DATA-DOG / go-sqlmock

Sql mock driver for golang to test database interactions
Other
6.16k stars 407 forks source link

stmt.Close() not returning close error #185

Open juliaogris opened 5 years ago

juliaogris commented 5 years ago

When usingWillReturnCloseError on an ExpectedPrepare but not using transactions,Close() is not returning the error given to WillReturnCloseError

Expected behaviour: stmt.Close() == wantErr Actual behaviour: stmt.Close() == nil

Here's a failing test function that should pass.

func TestExpectedPreparedStatementCloseErrorDB(t *testing.T) {
        conn, mock, err := New()
        if err != nil {
                t.Fatal("failed to open sqlmock database:", err)
        }

        want := errors.New("STMT ERROR")
        mock.ExpectPrepare("SELECT").WillReturnCloseError(want)

        stmt, err := conn.Prepare("SELECT")
        if err != nil {
                t.Fatal("unexpected error while preparing a statement:", err)
        }

        if err := stmt.Close(); err != want {
                t.Fatalf("got = %v, want = %v", err, want)
        }
}
l3pp4rd commented 5 years ago

this one is interesting, since the error is returned by Close and this function is called by go sql. it just does not return that error for some reason. and this seems to be related to standard library implementation. will need to have a look in more detail, how this is working

juliaogris commented 5 years ago

yup, @camh- (his test function btw) and I tried for a while to understand what's going on. It seems like db/sql.Stmt.Close is bailing early because cg stmtConnGrabber is nil - we gave up at that point (private field and private type with limited docs), but it might make more sense to you, @l3pp4rd ?

l3pp4rd commented 5 years ago

from what it seems, go does not lock the connection for the statement. and if the connection dies it will just open another one. for transactions the connection is locked and it can error there, otherwise the Prepare statement.Close cannot error at all as far as I see it unless it is in transaction. I do not like this, but it seems like it is how it is.. Will need to note this on that function. Maybe you could also ask this question in the golang issue list

proost commented 1 year ago

@l3pp4rd If it can't be fixed, I wish it was documented somewhere. how do you think that leaving note to that function?