From 7452e60565e2f2de44f70d6fa95bda32446fe3f1 Mon Sep 17 00:00:00 2001 From: Matt Lord Date: Fri, 13 Dec 2024 18:52:57 -0500 Subject: [PATCH] Support optional unit test behavior for 5.7 Signed-off-by: Matt Lord --- .../unit_test_evalengine_mysql57.yml | 3 ++ .../unit_test_evalengine_mysql80.yml | 3 ++ .../unit_test_evalengine_mysql84.yml | 3 ++ .github/workflows/unit_test_mysql57.yml | 3 ++ .github/workflows/unit_test_mysql80.yml | 3 ++ .github/workflows/unit_test_mysql84.yml | 3 ++ .../vreplication/vreplication_test.go | 12 ++++-- go/test/utils/binlog.go | 40 ++++++++++++++++--- go/test/utils/binlog_test.go | 14 ++++++- .../vreplication/framework_test.go | 12 ++++-- .../vreplication/vplayer_flaky_test.go | 23 ++++++----- test/templates/unit_test.tpl | 3 ++ 12 files changed, 97 insertions(+), 25 deletions(-) diff --git a/.github/workflows/unit_test_evalengine_mysql57.yml b/.github/workflows/unit_test_evalengine_mysql57.yml index ecc366b38fe..d55b2732c86 100644 --- a/.github/workflows/unit_test_evalengine_mysql57.yml +++ b/.github/workflows/unit_test_evalengine_mysql57.yml @@ -163,6 +163,9 @@ jobs: export NOVTADMINBUILD=1 export VTEVALENGINETEST="1" + # We sometimes need to alter the behavior based on the platform we're + # testing, e.g. MySQL 5.7 vs 8.0. + export CI_DB_PLATFORM="mysql57" eatmydata -- make unit_test | tee -a output.txt | go-junit-report -set-exit-code > report.xml diff --git a/.github/workflows/unit_test_evalengine_mysql80.yml b/.github/workflows/unit_test_evalengine_mysql80.yml index e6e802b52d8..96af579742e 100644 --- a/.github/workflows/unit_test_evalengine_mysql80.yml +++ b/.github/workflows/unit_test_evalengine_mysql80.yml @@ -153,6 +153,9 @@ jobs: export NOVTADMINBUILD=1 export VTEVALENGINETEST="1" + # We sometimes need to alter the behavior based on the platform we're + # testing, e.g. MySQL 5.7 vs 8.0. + export CI_DB_PLATFORM="mysql80" eatmydata -- make unit_test | tee -a output.txt | go-junit-report -set-exit-code > report.xml diff --git a/.github/workflows/unit_test_evalengine_mysql84.yml b/.github/workflows/unit_test_evalengine_mysql84.yml index 46736dac349..efbe2b0eb9f 100644 --- a/.github/workflows/unit_test_evalengine_mysql84.yml +++ b/.github/workflows/unit_test_evalengine_mysql84.yml @@ -153,6 +153,9 @@ jobs: export NOVTADMINBUILD=1 export VTEVALENGINETEST="1" + # We sometimes need to alter the behavior based on the platform we're + # testing, e.g. MySQL 5.7 vs 8.0. + export CI_DB_PLATFORM="mysql84" eatmydata -- make unit_test | tee -a output.txt | go-junit-report -set-exit-code > report.xml diff --git a/.github/workflows/unit_test_mysql57.yml b/.github/workflows/unit_test_mysql57.yml index 3eaf02d1538..eed08e9ce4c 100644 --- a/.github/workflows/unit_test_mysql57.yml +++ b/.github/workflows/unit_test_mysql57.yml @@ -163,6 +163,9 @@ jobs: export NOVTADMINBUILD=1 export VTEVALENGINETEST="0" + # We sometimes need to alter the behavior based on the platform we're + # testing, e.g. MySQL 5.7 vs 8.0. + export CI_DB_PLATFORM="mysql57" eatmydata -- make unit_test | tee -a output.txt | go-junit-report -set-exit-code > report.xml diff --git a/.github/workflows/unit_test_mysql80.yml b/.github/workflows/unit_test_mysql80.yml index c036e6dd477..9e0ed7e6977 100644 --- a/.github/workflows/unit_test_mysql80.yml +++ b/.github/workflows/unit_test_mysql80.yml @@ -153,6 +153,9 @@ jobs: export NOVTADMINBUILD=1 export VTEVALENGINETEST="0" + # We sometimes need to alter the behavior based on the platform we're + # testing, e.g. MySQL 5.7 vs 8.0. + export CI_DB_PLATFORM="mysql80" eatmydata -- make unit_test | tee -a output.txt | go-junit-report -set-exit-code > report.xml diff --git a/.github/workflows/unit_test_mysql84.yml b/.github/workflows/unit_test_mysql84.yml index 84447ce390b..5948eb0836a 100644 --- a/.github/workflows/unit_test_mysql84.yml +++ b/.github/workflows/unit_test_mysql84.yml @@ -153,6 +153,9 @@ jobs: export NOVTADMINBUILD=1 export VTEVALENGINETEST="0" + # We sometimes need to alter the behavior based on the platform we're + # testing, e.g. MySQL 5.7 vs 8.0. + export CI_DB_PLATFORM="mysql84" eatmydata -- make unit_test | tee -a output.txt | go-junit-report -set-exit-code > report.xml diff --git a/go/test/endtoend/vreplication/vreplication_test.go b/go/test/endtoend/vreplication/vreplication_test.go index 4269bd68cab..6eab1d5495e 100644 --- a/go/test/endtoend/vreplication/vreplication_test.go +++ b/go/test/endtoend/vreplication/vreplication_test.go @@ -323,8 +323,10 @@ func testVreplicationWorkflows(t *testing.T, limited bool, binlogRowImage string defer func() { defaultReplicas = 1 }() if binlogRowImage != "" { - require.NoError(t, utils.SetBinlogRowImageMode("noblob", vc.ClusterConfig.tmpDir)) - defer utils.SetBinlogRowImageMode("", vc.ClusterConfig.tmpDir) + // Run the e2e test with binlog_row_image=NOBLOB and + // binlog_row_value_options=PARTIAL_JSON. + require.NoError(t, utils.SetBinlogRowImageMode("noblob", vc.ClusterConfig.tmpDir, true)) + defer utils.SetBinlogRowImageMode("", vc.ClusterConfig.tmpDir, false) } defaultCell := vc.Cells[defaultCellName] @@ -600,8 +602,10 @@ func TestCellAliasVreplicationWorkflow(t *testing.T) { keyspace := "product" shard := "0" - require.NoError(t, utils.SetBinlogRowImageMode("noblob", vc.ClusterConfig.tmpDir)) - defer utils.SetBinlogRowImageMode("", vc.ClusterConfig.tmpDir) + // Run the e2e test with binlog_row_image=NOBLOB and + // binlog_row_value_options=PARTIAL_JSON. + require.NoError(t, utils.SetBinlogRowImageMode("noblob", vc.ClusterConfig.tmpDir, true)) + defer utils.SetBinlogRowImageMode("", vc.ClusterConfig.tmpDir, false) cell1 := vc.Cells["zone1"] cell2 := vc.Cells["zone2"] diff --git a/go/test/utils/binlog.go b/go/test/utils/binlog.go index a9eafc4941d..05e5ebab645 100644 --- a/go/test/utils/binlog.go +++ b/go/test/utils/binlog.go @@ -19,6 +19,7 @@ package utils import ( "fmt" "os" + "strconv" "strings" ) @@ -28,9 +29,11 @@ const ( ) // SetBinlogRowImageMode creates a temp cnf file to set binlog_row_image=NOBLOB and -// binlog_row_value_options=PARTIAL_JSON for vreplication unit tests. -// It adds it to the EXTRA_MY_CNF environment variable which appends text from them into my.cnf. -func SetBinlogRowImageMode(mode string, cnfDir string) error { +// optionally binlog_row_value_options=PARTIAL_JSON (since it does not exist in 5.7) +// for vreplication unit tests. +// It adds it to the EXTRA_MY_CNF environment variable which appends text from them +// into my.cnf. +func SetBinlogRowImageMode(mode string, cnfDir string, includePartialJSON bool) error { var newCnfs []string // remove any existing extra cnfs for binlog row image @@ -56,8 +59,7 @@ func SetBinlogRowImageMode(mode string, cnfDir string) error { if err != nil { return err } - lm := strings.ToLower(mode) - if lm == "noblob" || lm == "minimal" { + if includePartialJSON { // We're testing partial binlog row images so let's also test partial // JSON values in the images. _, err = f.WriteString("\nbinlog_row_value_options=PARTIAL_JSON\n") @@ -78,3 +80,31 @@ func SetBinlogRowImageMode(mode string, cnfDir string) error { } return nil } + +// CIDBPlatformIsMySQL8orLater returns true if the CI_DB_PLATFORM environment +// variable is empty -- meaning we're not running in the CI and we assume +// MySQL8.0 or later is used, and you can understand the failures and make +// adjustments as necessary -- or it's set to reflect usage of MySQL 8.0 or +// later. This relies on the current standard values used such as mysql5.7, +// mysql8.0, mysql8.4, mariadb10.7, etc. This can be used when the CI test +// behavior needs to be altered based on the specific database platform +// we're testing against. +func CIDBPlatformIsMySQL8orLater() bool { + dbPlatform := strings.ToLower(os.Getenv("CI_DB_PLATFORM")) + if dbPlatform == "" { + // This is for local testing where we don't set the env var via + // the CI. + return true + } + if strings.HasPrefix(dbPlatform, "mysql") { + _, v, ok := strings.Cut(dbPlatform, "mysql") + if ok { + // We only want the major version. + version, err := strconv.Atoi(string(v[0])) + if err == nil && version >= 8 { + return true + } + } + } + return false +} diff --git a/go/test/utils/binlog_test.go b/go/test/utils/binlog_test.go index 593b964a171..ea1c577766c 100644 --- a/go/test/utils/binlog_test.go +++ b/go/test/utils/binlog_test.go @@ -28,15 +28,25 @@ import ( func TestUtils(t *testing.T) { tmpDir := "/tmp" cnfFile := fmt.Sprintf("%s/%s", tmpDir, BinlogRowImageCnf) + // Test that setting the mode will create the cnf file and add it to the EXTRA_MY_CNF env var. - require.NoError(t, SetBinlogRowImageMode("noblob", tmpDir)) + require.NoError(t, SetBinlogRowImageMode("noblob", tmpDir, false)) data, err := os.ReadFile(cnfFile) require.NoError(t, err) require.Contains(t, string(data), "binlog_row_image=noblob") require.Contains(t, os.Getenv(ExtraCnf), BinlogRowImageCnf) + // Test that sett the mode and passing true for includePartialJSON will set both variables + // as expected. + require.NoError(t, SetBinlogRowImageMode("noblob", tmpDir, true)) + data, err = os.ReadFile(cnfFile) + require.NoError(t, err) + require.Contains(t, string(data), "binlog_row_image=noblob") + require.Contains(t, string(data), "binlog_row_value_options=PARTIAL_JSON") + require.Contains(t, os.Getenv(ExtraCnf), BinlogRowImageCnf) + // Test that clearing the mode will remove the cnf file and the cnf from the EXTRA_MY_CNF env var. - require.NoError(t, SetBinlogRowImageMode("", tmpDir)) + require.NoError(t, SetBinlogRowImageMode("", tmpDir, false)) require.NotContains(t, os.Getenv(ExtraCnf), BinlogRowImageCnf) _, err = os.Stat(cnfFile) require.True(t, os.IsNotExist(err)) diff --git a/go/vt/vttablet/tabletmanager/vreplication/framework_test.go b/go/vt/vttablet/tabletmanager/vreplication/framework_test.go index 12a05a69dbc..34e9998ab24 100644 --- a/go/vt/vttablet/tabletmanager/vreplication/framework_test.go +++ b/go/vt/vttablet/tabletmanager/vreplication/framework_test.go @@ -178,10 +178,10 @@ func TestMain(m *testing.M) { exitCode := func() int { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - if err := utils.SetBinlogRowImageMode("full", tempDir); err != nil { + if err := utils.SetBinlogRowImageMode("full", tempDir, false); err != nil { panic(err) } - defer utils.SetBinlogRowImageMode("", tempDir) + defer utils.SetBinlogRowImageMode("", tempDir, false) cancel, ret := setup(ctx) if ret > 0 { return ret @@ -193,10 +193,14 @@ func TestMain(m *testing.M) { cancel() runNoBlobTest = true - if err := utils.SetBinlogRowImageMode("noblob", tempDir); err != nil { + // binlog-row-value-options=PARTIAL_JSON is only supported in MySQL 8.0 and later. + // We still run unit tests with MySQL 5.7, so we cannot add it to the cnf file + // when using 5.7 or mysqld will fail to start. + includePartialJSON := utils.CIDBPlatformIsMySQL8orLater() + if err := utils.SetBinlogRowImageMode("noblob", tempDir, includePartialJSON); err != nil { panic(err) } - defer utils.SetBinlogRowImageMode("", tempDir) + defer utils.SetBinlogRowImageMode("", tempDir, false) cancel, ret = setup(ctx) if ret > 0 { return ret diff --git a/go/vt/vttablet/tabletmanager/vreplication/vplayer_flaky_test.go b/go/vt/vttablet/tabletmanager/vreplication/vplayer_flaky_test.go index 90b514b5b6d..8e183bc75aa 100644 --- a/go/vt/vttablet/tabletmanager/vreplication/vplayer_flaky_test.go +++ b/go/vt/vttablet/tabletmanager/vreplication/vplayer_flaky_test.go @@ -33,6 +33,7 @@ import ( "vitess.io/vitess/go/mysql/replication" "vitess.io/vitess/go/sqltypes" + "vitess.io/vitess/go/test/utils" "vitess.io/vitess/go/vt/binlog/binlogplayer" "vitess.io/vitess/go/vt/log" "vitess.io/vitess/go/vt/logutil" @@ -1521,12 +1522,14 @@ func TestPlayerRowMove(t *testing.T) { // TestPlayerPartialImagesUpdatePK tests the behavior of the vplayer when we // have partial binlog images, meaning that binlog-row-image=NOBLOB and -// binlog-row-value-options=PARTIAL_JSON. These are both set together when -// running the unit tests with runNoBlobTest=true. So we skip the test if -// it's not set. +// binlog-row-value-options=PARTIAL_JSON. These are both set when running the +// unit tests with runNoBlobTest=true and the DB platform we're testing +// against is MySQL 8.0 or later (which you can control using the CI_DB_PLATFORM +// env variable), as binlog-row-value-options is new in MySQL 8.0. So we skip +// the test if it's not set. func TestPlayerPartialImagesUpdatePK(t *testing.T) { - if !runNoBlobTest { - t.Skip("Skipping test as runNoBlobTest is not set") + if !runNoBlobTest || !utils.CIDBPlatformIsMySQL8orLater() { + t.Skip("Skipping test as binlog_row_image=NOBLOB is not set or database type is not MySQL 8.0 or later") } defer deleteTablet(addTablet(100)) @@ -1757,10 +1760,12 @@ func TestPlayerTypes(t *testing.T) { {"2", "null", `{"name": null}`, "123", `{"a": [42, 100]}`, `{"foo": "bar"}`}, }, }} - if !runNoBlobTest { + if runNoBlobTest && utils.CIDBPlatformIsMySQL8orLater() { + // With partial JSON values we don't replicate the JSON columns that aren't + // actually updated. testcases = append(testcases, testcase{ input: "update vitess_json set val1 = '{\"bar\": \"foo\"}', val4 = '{\"a\": [98, 123]}', val5 = convert(x'7b7d' using utf8mb4) where id=1", - output: "update vitess_json set val1=JSON_OBJECT(_utf8mb4'bar', _utf8mb4'foo'), val2=JSON_OBJECT(), val3=CAST(123 as JSON), val4=JSON_OBJECT(_utf8mb4'a', JSON_ARRAY(98, 123)), val5=JSON_OBJECT() where id=1", + output: "update vitess_json set val1=JSON_OBJECT(_utf8mb4'bar', _utf8mb4'foo'), val4=JSON_OBJECT(_utf8mb4'a', JSON_ARRAY(98, 123)), val5=JSON_OBJECT() where id=1", table: "vitess_json", data: [][]string{ {"1", `{"bar": "foo"}`, "{}", "123", `{"a": [98, 123]}`, `{}`}, @@ -1768,11 +1773,9 @@ func TestPlayerTypes(t *testing.T) { }, }) } else { - // With partial JSON values we don't replicate the JSON columns that aren't - // actually updated. testcases = append(testcases, testcase{ input: "update vitess_json set val1 = '{\"bar\": \"foo\"}', val4 = '{\"a\": [98, 123]}', val5 = convert(x'7b7d' using utf8mb4) where id=1", - output: "update vitess_json set val1=JSON_OBJECT(_utf8mb4'bar', _utf8mb4'foo'), val4=JSON_OBJECT(_utf8mb4'a', JSON_ARRAY(98, 123)), val5=JSON_OBJECT() where id=1", + output: "update vitess_json set val1=JSON_OBJECT(_utf8mb4'bar', _utf8mb4'foo'), val2=JSON_OBJECT(), val3=CAST(123 as JSON), val4=JSON_OBJECT(_utf8mb4'a', JSON_ARRAY(98, 123)), val5=JSON_OBJECT() where id=1", table: "vitess_json", data: [][]string{ {"1", `{"bar": "foo"}`, "{}", "123", `{"a": [98, 123]}`, `{}`}, diff --git a/test/templates/unit_test.tpl b/test/templates/unit_test.tpl index 3704aebac4e..f802ee7ad4a 100644 --- a/test/templates/unit_test.tpl +++ b/test/templates/unit_test.tpl @@ -177,6 +177,9 @@ jobs: export NOVTADMINBUILD=1 export VTEVALENGINETEST="{{.Evalengine}}" + # We sometimes need to alter the behavior based on the platform we're + # testing, e.g. MySQL 5.7 vs 8.0. + export CI_DB_PLATFORM="{{.Platform}}" eatmydata -- make unit_test | tee -a output.txt | go-junit-report -set-exit-code > report.xml