From e112011439e6946a6453a7aa5bb17c76955774ee Mon Sep 17 00:00:00 2001 From: atercattus Date: Tue, 20 Aug 2024 10:06:57 +0400 Subject: [PATCH 1/4] fix a linter warnings --- driver/driver.go | 2 +- mysql/util.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/driver/driver.go b/driver/driver.go index 94ebabbf0..4054db2af 100644 --- a/driver/driver.go +++ b/driver/driver.go @@ -203,7 +203,7 @@ func (c *conn) CheckNamedValue(nv *sqldriver.NamedValue) error { } else { // we've found an error, if the error is driver.ErrSkip then // keep looking otherwise return the unknown error - if !goErrors.Is(sqldriver.ErrSkip, err) { + if !goErrors.Is(err, sqldriver.ErrSkip) { return err } } diff --git a/mysql/util.go b/mysql/util.go index a6679d0f6..01bd60f30 100644 --- a/mysql/util.go +++ b/mysql/util.go @@ -197,11 +197,10 @@ func PutLengthEncodedInt(n uint64) []byte { case n <= 0xffffff: return []byte{0xfd, byte(n), byte(n >> 8), byte(n >> 16)} - case n <= 0xffffffffffffffff: + default: // n <= 0xffffffffffffffff: return []byte{0xfe, byte(n), byte(n >> 8), byte(n >> 16), byte(n >> 24), byte(n >> 32), byte(n >> 40), byte(n >> 48), byte(n >> 56)} } - return nil } // LengthEncodedString returns the string read as a bytes slice, whether the value is NULL, From 2d7f728254c3e864df58d54323762d53148d408e Mon Sep 17 00:00:00 2001 From: atercattus Date: Tue, 20 Aug 2024 10:11:55 +0400 Subject: [PATCH 2/4] fix a datarace in tests --- driver/driver_options_test.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/driver/driver_options_test.go b/driver/driver_options_test.go index eaa863fbc..4e3aaa84f 100644 --- a/driver/driver_options_test.go +++ b/driver/driver_options_test.go @@ -10,15 +10,17 @@ import ( "reflect" "strconv" "strings" + "sync/atomic" "testing" "time" - "github.com/go-mysql-org/go-mysql/client" - "github.com/go-mysql-org/go-mysql/mysql" - "github.com/go-mysql-org/go-mysql/server" "github.com/pingcap/errors" "github.com/siddontang/go/log" "github.com/stretchr/testify/require" + + "github.com/go-mysql-org/go-mysql/client" + "github.com/go-mysql-org/go-mysql/mysql" + "github.com/go-mysql-org/go-mysql/server" ) var _ server.Handler = &mockHandler{} @@ -32,7 +34,7 @@ type testServer struct { type mockHandler struct { // the number of times a query executed - queryCount int + queryCount atomic.Int32 } func TestDriverOptions_SetRetriesOn(t *testing.T) { @@ -54,7 +56,7 @@ func TestDriverOptions_SetRetriesOn(t *testing.T) { // here we issue assert that even though we only issued 1 query, that the retries // remained on and there were 3 calls to the DB. - require.Equal(t, 3, srv.handler.queryCount) + require.EqualValues(t, 3, srv.handler.queryCount.Load()) } func TestDriverOptions_SetRetriesOff(t *testing.T) { @@ -75,7 +77,7 @@ func TestDriverOptions_SetRetriesOff(t *testing.T) { // here we issue assert that even though we only issued 1 query, that the retries // remained on and there were 3 calls to the DB. - require.Equal(t, 1, srv.handler.queryCount) + require.EqualValues(t, 1, srv.handler.queryCount.Load()) } func TestDriverOptions_SetCollation(t *testing.T) { @@ -309,7 +311,7 @@ func (h *mockHandler) UseDB(dbName string) error { } func (h *mockHandler) handleQuery(query string, binary bool, args []interface{}) (*mysql.Result, error) { - h.queryCount++ + h.queryCount.Add(1) ss := strings.Split(query, " ") switch strings.ToLower(ss[0]) { case "select": From e3476d9cae36ecdd88b451f99a835012684d34b3 Mon Sep 17 00:00:00 2001 From: lance6716 Date: Tue, 10 Sep 2024 13:55:21 +0800 Subject: [PATCH 3/4] add `-race` Signed-off-by: lance6716 --- .github/workflows/ci.yml | 4 ++-- driver/driver_options_test.go | 22 +++++++++++++++++++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 154a416de..fe75cb5aa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,8 +39,8 @@ jobs: - name: Run tests run: | # separate test to avoid RESET MASTER conflict - go test $(go list ./... | grep -v canal) - go test $(go list ./... | grep canal) + go test -race $(go list ./... | grep -v canal) + go test -race $(go list ./... | grep canal) mysqltest: strategy: diff --git a/driver/driver_options_test.go b/driver/driver_options_test.go index 4e3aaa84f..9bfb7fc7c 100644 --- a/driver/driver_options_test.go +++ b/driver/driver_options_test.go @@ -10,6 +10,7 @@ import ( "reflect" "strconv" "strings" + "sync" "sync/atomic" "testing" "time" @@ -35,14 +36,18 @@ type testServer struct { type mockHandler struct { // the number of times a query executed queryCount atomic.Int32 + modifier *sync.WaitGroup } func TestDriverOptions_SetRetriesOn(t *testing.T) { log.SetLevel(log.LevelDebug) srv := CreateMockServer(t) defer srv.Stop() + var wg sync.WaitGroup + srv.handler.modifier = &wg + wg.Add(3) - conn, err := sql.Open("mysql", "root@127.0.0.1:3307/test?readTimeout=1s") + conn, err := sql.Open("mysql", "root@127.0.0.1:3307/test?readTimeout=100ms") defer func() { _ = conn.Close() }() @@ -54,6 +59,7 @@ func TestDriverOptions_SetRetriesOn(t *testing.T) { // we want to get a golang database/sql/driver ErrBadConn require.ErrorIs(t, err, sqlDriver.ErrBadConn) + wg.Wait() // here we issue assert that even though we only issued 1 query, that the retries // remained on and there were 3 calls to the DB. require.EqualValues(t, 3, srv.handler.queryCount.Load()) @@ -63,8 +69,11 @@ func TestDriverOptions_SetRetriesOff(t *testing.T) { log.SetLevel(log.LevelDebug) srv := CreateMockServer(t) defer srv.Stop() + var wg sync.WaitGroup + srv.handler.modifier = &wg + wg.Add(1) - conn, err := sql.Open("mysql", "root@127.0.0.1:3307/test?readTimeout=1s&retries=off") + conn, err := sql.Open("mysql", "root@127.0.0.1:3307/test?readTimeout=100ms&retries=off") defer func() { _ = conn.Close() }() @@ -75,6 +84,7 @@ func TestDriverOptions_SetRetriesOff(t *testing.T) { // we want the native error from this driver implementation require.ErrorIs(t, err, mysql.ErrBadConn) + wg.Wait() // here we issue assert that even though we only issued 1 query, that the retries // remained on and there were 3 calls to the DB. require.EqualValues(t, 1, srv.handler.queryCount.Load()) @@ -311,6 +321,12 @@ func (h *mockHandler) UseDB(dbName string) error { } func (h *mockHandler) handleQuery(query string, binary bool, args []interface{}) (*mysql.Result, error) { + defer func() { + if h.modifier != nil { + h.modifier.Done() + } + }() + h.queryCount.Add(1) ss := strings.Split(query, " ") switch strings.ToLower(ss[0]) { @@ -329,7 +345,7 @@ func (h *mockHandler) handleQuery(query string, binary bool, args []interface{}) }, binary) } else { if strings.Contains(query, "slow") { - time.Sleep(time.Second * 5) + time.Sleep(time.Second) } var aValue uint64 = 1 From f6e046b2dcdf90836f49cd51f1a5eb8532ab7b2f Mon Sep 17 00:00:00 2001 From: lance6716 Date: Tue, 10 Sep 2024 13:59:07 +0800 Subject: [PATCH 4/4] fix UT Signed-off-by: lance6716 --- driver/driver_options_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver/driver_options_test.go b/driver/driver_options_test.go index 9bfb7fc7c..d195e22af 100644 --- a/driver/driver_options_test.go +++ b/driver/driver_options_test.go @@ -165,7 +165,7 @@ func TestDriverOptions_ReadTimeout(t *testing.T) { srv := CreateMockServer(t) defer srv.Stop() - conn, err := sql.Open("mysql", "root@127.0.0.1:3307/test?readTimeout=1s") + conn, err := sql.Open("mysql", "root@127.0.0.1:3307/test?readTimeout=100ms") defer func() { _ = conn.Close() }()