From c5b78373fbbd13c62bf2ce42adc4f90cc2d692aa Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Thu, 2 Jan 2025 11:42:28 -0800 Subject: [PATCH] Go: test fixes + reporting (#2867) * test fixes + reporting Signed-off-by: Yury-Fridlyand --- .github/workflows/go.yml | 25 ++++++++++++++--------- go/DEVELOPER.md | 2 ++ go/Makefile | 29 +++++++++++++++------------ go/api/options/zadd_options.go | 3 +-- go/integTest/glide_test_suite_test.go | 4 ++-- go/integTest/shared_commands_test.go | 22 ++++++++++---------- 6 files changed, 47 insertions(+), 38 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 044c0ac369..96052130cd 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -102,8 +102,8 @@ jobs: - name: Install & build & test working-directory: go run: | - LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$GITHUB_WORKSPACE/go/target/release/deps/ - make install-tools-go${{ matrix.go }} build unit-test integ-test + make install-tools-go${{ matrix.go }} build + make -k unit-test integ-test - uses: ./.github/workflows/test-benchmark with: @@ -118,6 +118,7 @@ jobs: path: | utils/clusters/** benchmarks/results/** + go/reports/** lint: timeout-minutes: 10 @@ -205,8 +206,8 @@ jobs: - name: Install & build & test working-directory: go run: | - LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$GITHUB_WORKSPACE/go/target/release/deps/ - make install-tools-go${{ matrix.go }} build unit-test integ-test + make install-tools-go${{ matrix.go }} build + make -k unit-test integ-test - name: Upload test reports if: always() @@ -217,6 +218,7 @@ jobs: path: | utils/clusters/** benchmarks/results/** + go/reports/** test-modules: if: (github.repository_owner == 'valkey-io' && github.event_name == 'workflow_dispatch') || github.event.pull_request.head.repo.owner.login == 'valkey-io' @@ -239,10 +241,13 @@ jobs: - name: Build and test working-directory: ./go run: | - make install-tools-go1.20.0 - LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$GITHUB_WORKSPACE/go/target/release/deps/ - make build - make modules-test cluster-endpoints=${{ secrets.MEMDB_MODULES_ENDPOINT }} tls=true + make install-tools-go1.20.0 build modules-test cluster-endpoints=${{ secrets.MEMDB_MODULES_ENDPOINT }} tls=true - # TODO: - # Upload test reports + - name: Upload test reports + if: always() + continue-on-error: true + uses: actions/upload-artifact@v4 + with: + name: test-reports-modules + path: | + go/reports/** diff --git a/go/DEVELOPER.md b/go/DEVELOPER.md index 12562c9e0f..8dcaf2cb7b 100644 --- a/go/DEVELOPER.md +++ b/go/DEVELOPER.md @@ -167,6 +167,8 @@ By default, those test suite start standalone and cluster servers without TLS an make integ-test standalone-endpoints=localhost:6379 cluster-endpoints=localhost:7000 tls=true ``` +Test reports generated in `reports` folder. + ### Generate protobuf files During the initial build, Go protobuf files were created in `go/protobuf`. If modifications are made to the protobuf definition files (.proto files located in `glide-core/src/protobuf`), it becomes necessary to regenerate the Go protobuf files. To do so, run: diff --git a/go/Makefile b/go/Makefile index c4d4b5aeb4..62eabbaa8b 100644 --- a/go/Makefile +++ b/go/Makefile @@ -1,3 +1,5 @@ +SHELL:=/bin/bash + install-build-tools: go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.33.0 cargo install cbindgen @@ -40,6 +42,7 @@ clean: rm -f benchmarks/benchmarks rm -rf protobuf rm -rf target + rm -rf reports build-glide-client: cargo build --release @@ -76,8 +79,11 @@ format: # unit tests - skip complete IT suite (including MT) unit-test: + mkdir -p reports + set -o pipefail; \ LD_LIBRARY_PATH=$(shell find . -name libglide_rs.so|grep -w release|tail -1|xargs dirname|xargs readlink -f):${LD_LIBRARY_PATH} \ - go test -v -race ./... -skip TestGlideTestSuite $(if $(test-filter), -run $(test-filter)) + go test -v -race ./... -skip TestGlideTestSuite $(if $(test-filter), -run $(test-filter)) \ + | tee >(go tool test2json -t -p github.com/valkey-io/valkey-glide/go/glide/utils | go-test-report -o reports/unit-tests.html -t unit-test > /dev/null) # integration tests - run subtask with skipping modules tests integ-test: export TEST_FILTER = -skip TestGlideTestSuite/TestModule $(if $(test-filter), -run $(test-filter)) @@ -88,17 +94,14 @@ modules-test: export TEST_FILTER = $(if $(test-filter), -run $(test-filter), -ru modules-test: __it __it: + mkdir -p reports + set -o pipefail; \ LD_LIBRARY_PATH=$(shell find . -name libglide_rs.so|grep -w release|tail -1|xargs dirname|xargs readlink -f):${LD_LIBRARY_PATH} \ go test -v -race ./integTest/... \ - $(TEST_FILTER) \ - $(if $(filter true, $(tls)), --tls,) \ - $(if $(standalone-endpoints), --standalone-endpoints=$(standalone-endpoints)) \ - $(if $(cluster-endpoints), --cluster-endpoints=$(cluster-endpoints)) - -# Note: this task is no longer run by CI because: -# - build failures that occur while running the task can be hidden by the task; CI still reports success in these scenarios. -# - there is not a good way to both generate a test report and log the test outcomes to GH actions. -# TODO: fix this and include -run/-skip flags -test-and-report: - mkdir -p reports - go test -v -race ./... -json | go-test-report -o reports/test-report.html + $(TEST_FILTER) \ + $(if $(filter true, $(tls)), --tls,) \ + $(if $(standalone-endpoints), --standalone-endpoints=$(standalone-endpoints)) \ + $(if $(cluster-endpoints), --cluster-endpoints=$(cluster-endpoints)) \ + | tee >(go tool test2json -t -p github.com/valkey-io/valkey-glide/go/glide/integTest | go-test-report -o reports/integ-tests.html -t integ-test > /dev/null) +# code above ^ is similar to `go test .... -json | go-test-report ....`, but it also prints plain text output to stdout +# `go test` prints plain text, tee duplicates it to stdout and to `test2json` which is coupled with `go-test-report` to generate the report diff --git a/go/api/options/zadd_options.go b/go/api/options/zadd_options.go index 7926b346cc..f10c010e4e 100644 --- a/go/api/options/zadd_options.go +++ b/go/api/options/zadd_options.go @@ -22,8 +22,7 @@ func NewZAddOptionsBuilder() *ZAddOptions { return &ZAddOptions{} } -// `conditionalChange“ defines conditions for updating or adding elements with {@link SortedSetBaseCommands#zadd} -// command. +// `conditionalChange` defines conditions for updating or adding elements with `ZADD` command. func (options *ZAddOptions) SetConditionalChange(c ConditionalChange) *ZAddOptions { options.conditionalChange = c return options diff --git a/go/integTest/glide_test_suite_test.go b/go/integTest/glide_test_suite_test.go index eb80993d9d..2ba275799a 100644 --- a/go/integTest/glide_test_suite_test.go +++ b/go/integTest/glide_test_suite_test.go @@ -264,8 +264,8 @@ func (suite *GlideTestSuite) clusterClient(config *api.GlideClusterClientConfigu } func (suite *GlideTestSuite) runWithClients(clients []api.BaseClient, test func(client api.BaseClient)) { - for i, client := range clients { - suite.T().Run(fmt.Sprintf("Testing [%v]", i), func(t *testing.T) { + for _, client := range clients { + suite.T().Run(fmt.Sprintf("%T", client)[5:], func(t *testing.T) { test(client) }) } diff --git a/go/integTest/shared_commands_test.go b/go/integTest/shared_commands_test.go index dd0fc29022..eefd38e9b0 100644 --- a/go/integTest/shared_commands_test.go +++ b/go/integTest/shared_commands_test.go @@ -55,7 +55,7 @@ func (suite *GlideTestSuite) TestSetWithOptions_ReturnOldValue() { func (suite *GlideTestSuite) TestSetWithOptions_OnlyIfExists_overwrite() { suite.runWithDefaultClients(func(client api.BaseClient) { - key := "TestSetWithOptions_OnlyIfExists_overwrite" + key := uuid.New().String() suite.verifyOK(client.Set(key, initialValue)) opts := api.NewSetOptionsBuilder().SetConditionalSet(api.OnlyIfExists) @@ -70,7 +70,7 @@ func (suite *GlideTestSuite) TestSetWithOptions_OnlyIfExists_overwrite() { func (suite *GlideTestSuite) TestSetWithOptions_OnlyIfExists_missingKey() { suite.runWithDefaultClients(func(client api.BaseClient) { - key := "TestSetWithOptions_OnlyIfExists_missingKey" + key := uuid.New().String() opts := api.NewSetOptionsBuilder().SetConditionalSet(api.OnlyIfExists) result, err := client.SetWithOptions(key, anotherValue, opts) @@ -81,7 +81,7 @@ func (suite *GlideTestSuite) TestSetWithOptions_OnlyIfExists_missingKey() { func (suite *GlideTestSuite) TestSetWithOptions_OnlyIfDoesNotExist_missingKey() { suite.runWithDefaultClients(func(client api.BaseClient) { - key := "TestSetWithOptions_OnlyIfDoesNotExist_missingKey" + key := uuid.New().String() opts := api.NewSetOptionsBuilder().SetConditionalSet(api.OnlyIfDoesNotExist) suite.verifyOK(client.SetWithOptions(key, anotherValue, opts)) @@ -94,7 +94,7 @@ func (suite *GlideTestSuite) TestSetWithOptions_OnlyIfDoesNotExist_missingKey() func (suite *GlideTestSuite) TestSetWithOptions_OnlyIfDoesNotExist_existingKey() { suite.runWithDefaultClients(func(client api.BaseClient) { - key := "TestSetWithOptions_OnlyIfDoesNotExist_existingKey" + key := uuid.New().String() opts := api.NewSetOptionsBuilder().SetConditionalSet(api.OnlyIfDoesNotExist) suite.verifyOK(client.Set(key, initialValue)) @@ -112,7 +112,7 @@ func (suite *GlideTestSuite) TestSetWithOptions_OnlyIfDoesNotExist_existingKey() func (suite *GlideTestSuite) TestSetWithOptions_KeepExistingExpiry() { suite.runWithDefaultClients(func(client api.BaseClient) { - key := "TestSetWithOptions_KeepExistingExpiry" + key := uuid.New().String() opts := api.NewSetOptionsBuilder().SetExpiry(api.NewExpiryBuilder().SetType(api.Milliseconds).SetCount(uint64(2000))) suite.verifyOK(client.SetWithOptions(key, initialValue, opts)) @@ -139,7 +139,7 @@ func (suite *GlideTestSuite) TestSetWithOptions_KeepExistingExpiry() { func (suite *GlideTestSuite) TestSetWithOptions_UpdateExistingExpiry() { suite.runWithDefaultClients(func(client api.BaseClient) { - key := "TestSetWithOptions_UpdateExistingExpiry" + key := uuid.New().String() opts := api.NewSetOptionsBuilder().SetExpiry(api.NewExpiryBuilder().SetType(api.Milliseconds).SetCount(uint64(100500))) suite.verifyOK(client.SetWithOptions(key, initialValue, opts)) @@ -166,14 +166,14 @@ func (suite *GlideTestSuite) TestSetWithOptions_UpdateExistingExpiry() { func (suite *GlideTestSuite) TestGetEx_existingAndNonExistingKeys() { suite.runWithDefaultClients(func(client api.BaseClient) { - key := "TestGetEx_ExisitingKey" + key := uuid.New().String() suite.verifyOK(client.Set(key, initialValue)) result, err := client.GetEx(key) assert.Nil(suite.T(), err) assert.Equal(suite.T(), initialValue, result.Value()) - key = "TestGetEx_NonExisitingKey" + key = uuid.New().String() result, err = client.Get(key) assert.Nil(suite.T(), err) assert.Equal(suite.T(), "", result.Value()) @@ -182,7 +182,7 @@ func (suite *GlideTestSuite) TestGetEx_existingAndNonExistingKeys() { func (suite *GlideTestSuite) TestGetExWithOptions_PersistKey() { suite.runWithDefaultClients(func(client api.BaseClient) { - key := "TestGetExWithOptions_PersistKey" + key := uuid.New().String() suite.verifyOK(client.Set(key, initialValue)) opts := api.NewGetExOptionsBuilder().SetExpiry(api.NewExpiryBuilder().SetType(api.Milliseconds).SetCount(uint64(2000))) @@ -205,7 +205,7 @@ func (suite *GlideTestSuite) TestGetExWithOptions_PersistKey() { func (suite *GlideTestSuite) TestGetExWithOptions_UpdateExpiry() { suite.runWithDefaultClients(func(client api.BaseClient) { - key := "TestGetExWithOptions_UpdateExpiry" + key := uuid.New().String() suite.verifyOK(client.Set(key, initialValue)) opts := api.NewGetExOptionsBuilder().SetExpiry(api.NewExpiryBuilder().SetType(api.Milliseconds).SetCount(uint64(2000))) @@ -227,7 +227,7 @@ func (suite *GlideTestSuite) TestGetExWithOptions_UpdateExpiry() { func (suite *GlideTestSuite) TestSetWithOptions_ReturnOldValue_nonExistentKey() { suite.runWithDefaultClients(func(client api.BaseClient) { - key := "TestSetWithOptions_ReturnOldValue_nonExistentKey" + key := uuid.New().String() opts := api.NewSetOptionsBuilder().SetReturnOldValue(true) result, err := client.SetWithOptions(key, anotherValue, opts)