From 6aed29e0b25cfd7eacf6e203d52de758c96e2e99 Mon Sep 17 00:00:00 2001 From: Rick <1450685+LinuxSuRen@users.noreply.github.com> Date: Tue, 18 Apr 2023 12:25:55 +0800 Subject: [PATCH] fix: the code smells from sonarqube (#43) --- .vscode/extensions.json | 5 + Dockerfile | 14 ++- cmd/root_test.go | 2 +- cmd/run_test.go | 19 ++-- pkg/runner/kubernetes/verify_test.go | 29 +++--- pkg/runner/reporter_discard.go | 4 +- pkg/runner/reporter_memory.go | 19 ++-- pkg/runner/reporter_memory_test.go | 14 ++- pkg/runner/simple.go | 8 +- pkg/runner/simple_test.go | 150 +++++++++++---------------- pkg/server/remote_server.go | 15 +-- pkg/server/remote_server_test.go | 7 ++ 12 files changed, 149 insertions(+), 137 deletions(-) create mode 100644 .vscode/extensions.json diff --git a/.vscode/extensions.json b/.vscode/extensions.json new file mode 100644 index 0000000..9f0834c --- /dev/null +++ b/.vscode/extensions.json @@ -0,0 +1,5 @@ +{ + "recommendations": [ + "linuxsuren.api-testing" + ] +} \ No newline at end of file diff --git a/Dockerfile b/Dockerfile index 1fad9d5..f0e9d5d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,15 @@ -FROM golang:1.18 as builder +FROM golang:1.18 AS builder WORKDIR /workspace -COPY . . +COPY cmd/ cmd/ +COPY pkg/ pkg/ +COPY sample/ sample/ +COPY go.mod go.mod +COPY go.sum go.sum +COPY main.go main.go +COPY README.md README.md +COPY LICENSE LICENSE + RUN go mod download RUN CGO_ENABLE=0 go build -ldflags "-w -s" -o atest . @@ -19,5 +27,7 @@ LABEL "maintainer"="Rick " LABEL "Name"="API testing" COPY --from=builder /workspace/atest /usr/local/bin/atest +COPY --from=builder /workspace/LICENSE /LICENSE +COPY --from=builder /workspace/README.md /README.md CMD ["atest", "server"] diff --git a/cmd/root_test.go b/cmd/root_test.go index cea9c33..e7a704e 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -9,7 +9,7 @@ import ( exec "github.com/linuxsuren/go-fake-runtime" ) -func Test_setRelativeDir(t *testing.T) { +func TestSetRelativeDir(t *testing.T) { type args struct { configFile string testcase *atesting.TestCase diff --git a/cmd/run_test.go b/cmd/run_test.go index 778742d..a65b475 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -21,9 +21,9 @@ func TestRunSuite(t *testing.T) { hasError bool }{{ name: "simple", - suiteFile: "testdata/simple-suite.yaml", + suiteFile: simpleSuite, prepare: func() { - gock.New("http://foo"). + gock.New(urlFoo). Get("/bar"). Reply(http.StatusOK). JSON("{}") @@ -31,9 +31,9 @@ func TestRunSuite(t *testing.T) { hasError: false, }, { name: "response is not JSON", - suiteFile: "testdata/simple-suite.yaml", + suiteFile: simpleSuite, prepare: func() { - gock.New("http://foo"). + gock.New(urlFoo). Get("/bar"). Reply(http.StatusOK) }, @@ -69,9 +69,9 @@ func TestRunCommand(t *testing.T) { hasErr bool }{{ name: "status code is not match", - args: []string{"-p", "testdata/simple-suite.yaml"}, + args: []string{"-p", simpleSuite}, prepare: func() { - gock.New("http://foo").Get("/bar") + gock.New(urlFoo).Get("/bar") }, hasErr: true, }, { @@ -81,9 +81,9 @@ func TestRunCommand(t *testing.T) { hasErr: false, }, { name: "normal case", - args: []string{"-p", "testdata/simple-suite.yaml"}, + args: []string{"-p", simpleSuite}, prepare: func() { - gock.New("http://foo").Get("/bar").Reply(http.StatusOK).JSON("{}") + gock.New(urlFoo).Get("/bar").Reply(http.StatusOK).JSON("{}") }, hasErr: false, }} @@ -177,3 +177,6 @@ func TestPreRunE(t *testing.T) { }) } } + +const urlFoo = "http://foo" +const simpleSuite = "testdata/simple-suite.yaml" diff --git a/pkg/runner/kubernetes/verify_test.go b/pkg/runner/kubernetes/verify_test.go index b3b4be6..5ebbb4b 100644 --- a/pkg/runner/kubernetes/verify_test.go +++ b/pkg/runner/kubernetes/verify_test.go @@ -12,7 +12,7 @@ import ( ) func TestKubernetesValidatorFunc(t *testing.T) { - os.Setenv("KUBERNETES_SERVER", "http://foo") + os.Setenv("KUBERNETES_SERVER", urlFoo) os.Setenv("KUBERNETES_TOKEN", "token") gock.InterceptClient(kubernetes.GetClient()) defer gock.RestoreClient(http.DefaultClient) @@ -92,44 +92,49 @@ func TestKubernetesValidatorFunc(t *testing.T) { } } -func emptyPrepare() {} +func emptyPrepare() { + // only for testing +} func preparePod() { - gock.New("http://foo"). + gock.New(urlFoo). Get("/api/v1/namespaces/ns/pods/foo"). - MatchHeader("Authorization", "Bearer token"). + MatchHeader("Authorization", defaultToken). Reply(http.StatusOK). JSON(`{"kind":"pod"}`) } func prepareDeploy() { - gock.New("http://foo"). + gock.New(urlFoo). Get("/apis/apps/v1/namespaces/ns/deployments/foo"). - MatchHeader("Authorization", "Bearer token"). + MatchHeader("Authorization", defaultToken). Reply(http.StatusOK). JSON(`{"kind":"deploy"}`) } func prepareStatefulset() { - gock.New("http://foo"). + gock.New(urlFoo). Get("/apis/apps/v1/namespaces/ns/statefulsets/foo"). - MatchHeader("Authorization", "Bearer token"). + MatchHeader("Authorization", defaultToken). Reply(http.StatusOK). JSON(`{"kind":"statefulset"}`) } func prepareDaemonset() { - gock.New("http://foo"). + gock.New(urlFoo). Get("/apis/apps/v1/namespaces/ns/daemonsets/foo"). - MatchHeader("Authorization", "Bearer token"). + MatchHeader("Authorization", defaultToken). Reply(http.StatusOK). JSON(`{"kind":"daemonset"}`) } func prepareCRDVM() { - gock.New("http://foo"). + gock.New(urlFoo). Get("/apis/bar/v2/namespaces/ns/vms/foo"). - MatchHeader("Authorization", "Bearer token"). + MatchHeader("Authorization", defaultToken). Reply(http.StatusOK). JSON(`{"kind":"vm"}`) } + +const urlFoo = "http://foo" +const defaultToken = "Bearer token" diff --git a/pkg/runner/reporter_discard.go b/pkg/runner/reporter_discard.go index 0aa425d..7136689 100644 --- a/pkg/runner/reporter_discard.go +++ b/pkg/runner/reporter_discard.go @@ -9,7 +9,9 @@ func NewDiscardTestReporter() TestReporter { } // PutRecord does nothing -func (r *discardTestReporter) PutRecord(*ReportRecord) {} +func (r *discardTestReporter) PutRecord(*ReportRecord) { + // Do nothing which is the design purpose +} // GetAllRecords does nothing func (r *discardTestReporter) GetAllRecords() []*ReportRecord { diff --git a/pkg/runner/reporter_memory.go b/pkg/runner/reporter_memory.go index b9d29df..06e3463 100644 --- a/pkg/runner/reporter_memory.go +++ b/pkg/runner/reporter_memory.go @@ -34,6 +34,17 @@ func (r *memoryTestReporter) GetAllRecords() []*ReportRecord { return r.records } +func getMaxAndMin(max, min, duration time.Duration) (time.Duration, time.Duration) { + if max < duration { + max = duration + } + + if min > duration { + min = duration + } + return max, min +} + // ExportAllReportResults exports all the report results func (r *memoryTestReporter) ExportAllReportResults() (result ReportResultSlice, err error) { resultWithTotal := map[string]*ReportResultWithTotal{} @@ -42,13 +53,7 @@ func (r *memoryTestReporter) ExportAllReportResults() (result ReportResultSlice, duration := record.Duration() if item, ok := resultWithTotal[api]; ok { - if item.Max < duration { - item.Max = duration - } - - if item.Min > duration { - item.Min = duration - } + item.Max, item.Min = getMaxAndMin(item.Max, item.Min, duration) item.Error += record.ErrorCount() item.Total += duration item.Count += 1 diff --git a/pkg/runner/reporter_memory_test.go b/pkg/runner/reporter_memory_test.go index f78e8e0..14932ad 100644 --- a/pkg/runner/reporter_memory_test.go +++ b/pkg/runner/reporter_memory_test.go @@ -10,6 +10,10 @@ import ( "github.com/stretchr/testify/assert" ) +const urlFoo = "http://foo" +const urlBar = "http://bar" +const urlFake = "http://fake" + func TestExportAllReportResults(t *testing.T) { now := time.Now() @@ -24,28 +28,28 @@ func TestExportAllReportResults(t *testing.T) { }, { name: "normal", records: []*runner.ReportRecord{{ - API: "http://foo", + API: urlFoo, Method: http.MethodGet, BeginTime: now, EndTime: now.Add(time.Second * 3), }, { - API: "http://foo", + API: urlFoo, Method: http.MethodGet, BeginTime: now, EndTime: now.Add(time.Second * 4), Error: errors.New("fake"), }, { - API: "http://foo", + API: urlFoo, Method: http.MethodGet, BeginTime: now, EndTime: now.Add(time.Second * 2), }, { - API: "http://bar", + API: urlBar, Method: http.MethodGet, BeginTime: now, EndTime: now.Add(time.Second), }, { - API: "http://fake", + API: urlFake, Method: http.MethodGet, BeginTime: now, EndTime: now.Add(time.Second * 5), diff --git a/pkg/runner/simple.go b/pkg/runner/simple.go index 8fd46cd..1c04e5b 100644 --- a/pkg/runner/simple.go +++ b/pkg/runner/simple.go @@ -217,7 +217,7 @@ func (r *simpleTestCaseRunner) RunTestCase(testcase *testing.TestCase, dataConte } if len(testcase.Request.Form) > 0 { - if testcase.Request.Header["Content-Type"] == "multipart/form-data" { + if testcase.Request.Header[contentType] == "multipart/form-data" { multiBody := &bytes.Buffer{} writer := multipart.NewWriter(multiBody) for key, val := range testcase.Request.Form { @@ -226,8 +226,8 @@ func (r *simpleTestCaseRunner) RunTestCase(testcase *testing.TestCase, dataConte _ = writer.Close() requestBody = multiBody - testcase.Request.Header["Content-Type"] = writer.FormDataContentType() - } else if testcase.Request.Header["Content-Type"] == "application/x-www-form-urlencoded" { + testcase.Request.Header[contentType] = writer.FormDataContentType() + } else if testcase.Request.Header[contentType] == "application/x-www-form-urlencoded" { data := url.Values{} for key, val := range testcase.Request.Form { data.Set(key, val) @@ -436,3 +436,5 @@ func jsonSchemaValidation(schema string, body []byte) (err error) { } return } + +const contentType = "Content-Type" diff --git a/pkg/runner/simple_test.go b/pkg/runner/simple_test.go index cbfb282..6e8e7f4 100644 --- a/pkg/runner/simple_test.go +++ b/pkg/runner/simple_test.go @@ -17,6 +17,10 @@ import ( ) func TestTestCase(t *testing.T) { + fooRequst := atest.Request{ + API: urlFoo, + } + tests := []struct { name string execer fakeruntime.Execer @@ -32,14 +36,11 @@ func TestTestCase(t *testing.T) { }, }, execer: fakeruntime.FakeExecer{ExpectError: errors.New("fake")}, - verify: func(t *testing.T, output interface{}, err error) { - assert.NotNil(t, err) - }, }, { name: "normal, response is map", testCase: &atest.TestCase{ Request: atest.Request{ - API: "http://localhost/foo", + API: urlFoo, Header: map[string]string{ "key": "value", }, @@ -67,7 +68,7 @@ func TestTestCase(t *testing.T) { }, execer: fakeruntime.FakeExecer{}, prepare: func() { - gock.New("http://localhost"). + gock.New(urlLocalhost). Get("/foo"). MatchHeader("key", "value"). Reply(http.StatusOK). @@ -81,16 +82,14 @@ func TestTestCase(t *testing.T) { }, { name: "normal, response is slice", testCase: &atest.TestCase{ - Request: atest.Request{ - API: "http://localhost/foo", - }, + Request: fooRequst, Expect: atest.Response{ StatusCode: http.StatusOK, Body: `["foo", "bar"]`, }, }, prepare: func() { - gock.New("http://localhost"). + gock.New(urlLocalhost). Get("/foo"). Reply(http.StatusOK). BodyString(`["foo", "bar"]`) @@ -103,7 +102,7 @@ func TestTestCase(t *testing.T) { name: "normal, response from file", testCase: &atest.TestCase{ Request: atest.Request{ - API: "http://localhost/foo", + API: urlFoo, Method: http.MethodPost, BodyFromFile: "testdata/generic_response.json", }, @@ -112,79 +111,56 @@ func TestTestCase(t *testing.T) { }, }, prepare: func() { - gock.New("http://localhost"). + gock.New(urlLocalhost). Post("/foo").BodyString(genericBody). Reply(http.StatusOK).BodyString("123") }, - verify: func(t *testing.T, output interface{}, err error) { - assert.NotNil(t, err) - }, }, { name: "response from a not found file", testCase: &atest.TestCase{ Request: atest.Request{ - API: "http://localhost/foo", + API: urlFoo, Method: http.MethodPost, BodyFromFile: "testdata/fake.json", }, }, - verify: func(t *testing.T, output interface{}, err error) { - assert.NotNil(t, err) - }, }, { name: "bad request", testCase: &atest.TestCase{ - Request: atest.Request{ - API: "http://localhost/foo", - }, + Request: fooRequst, Expect: atest.Response{ StatusCode: http.StatusOK, }, }, prepare: func() { - gock.New("http://localhost"). + gock.New(urlLocalhost). Get("/foo").Reply(http.StatusBadRequest) }, - verify: func(t *testing.T, output interface{}, err error) { - assert.NotNil(t, err) - }, }, { name: "error with request", testCase: &atest.TestCase{ - Request: atest.Request{ - API: "http://localhost/foo", - }, + Request: fooRequst, }, prepare: func() { - gock.New("http://localhost"). + gock.New(urlLocalhost). Get("/foo").ReplyError(errors.New("error")) }, - verify: func(t *testing.T, output interface{}, err error) { - assert.NotNil(t, err) - }, }, { name: "not match with body", testCase: &atest.TestCase{ - Request: atest.Request{ - API: "http://localhost/foo", - }, + Request: fooRequst, Expect: atest.Response{ Body: "bar", }, }, prepare: func() { - gock.New("http://localhost"). + gock.New(urlLocalhost). Get("/foo").Reply(http.StatusOK).BodyString("foo") }, - verify: func(t *testing.T, output interface{}, err error) { - assert.NotNil(t, err) - }, }, { name: "not match with header", testCase: &atest.TestCase{ - Request: atest.Request{ - API: "http://localhost/foo", - }, + Request: fooRequst, Expect: atest.Response{ Header: map[string]string{ "foo": "bar", @@ -192,56 +168,35 @@ func TestTestCase(t *testing.T) { }, }, prepare: func() { - gock.New("http://localhost"). + gock.New(urlLocalhost). Get("/foo").Reply(http.StatusOK).SetHeader("foo", "value") }, - verify: func(t *testing.T, output interface{}, err error) { - assert.NotNil(t, err) - }, }, { name: "not found from fields", testCase: &atest.TestCase{ - Request: atest.Request{ - API: "http://localhost/foo", - }, + Request: fooRequst, Expect: atest.Response{ BodyFieldsExpect: map[string]interface{}{ "foo": "bar", }, }, }, - prepare: func() { - gock.New("http://localhost"). - Get("/foo").Reply(http.StatusOK).BodyString(genericBody) - }, - verify: func(t *testing.T, output interface{}, err error) { - assert.NotNil(t, err) - }, + prepare: prepareForFoo, }, { name: "body filed not match", testCase: &atest.TestCase{ - Request: atest.Request{ - API: "http://localhost/foo", - }, + Request: fooRequst, Expect: atest.Response{ BodyFieldsExpect: map[string]interface{}{ "name": "bar", }, }, }, - prepare: func() { - gock.New("http://localhost"). - Get("/foo").Reply(http.StatusOK).BodyString(genericBody) - }, - verify: func(t *testing.T, output interface{}, err error) { - assert.NotNil(t, err) - }, + prepare: prepareForFoo, }, { name: "invalid filed finding", testCase: &atest.TestCase{ - Request: atest.Request{ - API: "http://localhost/foo", - }, + Request: fooRequst, Expect: atest.Response{ BodyFieldsExpect: map[string]interface{}{ "items[1]": "bar", @@ -249,7 +204,7 @@ func TestTestCase(t *testing.T) { }, }, prepare: func() { - gock.New("http://localhost"). + gock.New(urlLocalhost). Get("/foo").Reply(http.StatusOK).BodyString(`{"items":[]}`) }, verify: func(t *testing.T, output interface{}, err error) { @@ -261,7 +216,7 @@ func TestTestCase(t *testing.T) { // name: "verify failed", // testCase: &atest.TestCase{ // Request: atest.Request{ - // API: "http://localhost/foo", + // API: urlFoo, // }, // Expect: atest.Response{ // Verify: []string{ @@ -270,7 +225,7 @@ func TestTestCase(t *testing.T) { // }, // }, // prepare: func() { - // gock.New("http://localhost"). + // gock.New(urlLocalhost). // Get("/foo").Reply(http.StatusOK).BodyString(`{"items":[]}`) // }, // verify: func(t *testing.T, output interface{}, err error) { @@ -282,9 +237,7 @@ func TestTestCase(t *testing.T) { { name: "failed to compile", testCase: &atest.TestCase{ - Request: atest.Request{ - API: "http://localhost/foo", - }, + Request: fooRequst, Expect: atest.Response{ Verify: []string{ `println("12")`, @@ -292,7 +245,7 @@ func TestTestCase(t *testing.T) { }, }, prepare: func() { - gock.New("http://localhost"). + gock.New(urlLocalhost). Get("/foo").Reply(http.StatusOK).BodyString(`{"items":[]}`) }, verify: func(t *testing.T, output interface{}, err error) { @@ -302,9 +255,7 @@ func TestTestCase(t *testing.T) { }, { name: "failed to compile", testCase: &atest.TestCase{ - Request: atest.Request{ - API: "http://localhost/foo", - }, + Request: fooRequst, Expect: atest.Response{ Verify: []string{ `1 + 1`, @@ -312,7 +263,7 @@ func TestTestCase(t *testing.T) { }, }, prepare: func() { - gock.New("http://localhost"). + gock.New(urlLocalhost). Get("/foo").Reply(http.StatusOK).BodyString(`{"items":[]}`) }, verify: func(t *testing.T, output interface{}, err error) { @@ -346,10 +297,10 @@ func TestTestCase(t *testing.T) { name: "multipart form request", testCase: &atest.TestCase{ Request: atest.Request{ - API: "http://localhost/foo", + API: urlFoo, Method: http.MethodPost, Header: map[string]string{ - "Content-Type": "multipart/form-data", + contentType: "multipart/form-data", }, Form: map[string]string{ "key": "value", @@ -357,20 +308,18 @@ func TestTestCase(t *testing.T) { }, }, prepare: func() { - gock.New("http://localhost"). + gock.New(urlLocalhost). Post("/foo").Reply(http.StatusOK).BodyString(`{"items":[]}`) }, - verify: func(t *testing.T, output interface{}, err error) { - assert.Nil(t, err) - }, + verify: noError, }, { name: "normal form request", testCase: &atest.TestCase{ Request: atest.Request{ - API: "http://localhost/foo", + API: urlFoo, Method: http.MethodPost, Header: map[string]string{ - "Content-Type": "application/x-www-form-urlencoded", + contentType: "application/x-www-form-urlencoded", }, Form: map[string]string{ "key": "value", @@ -378,12 +327,10 @@ func TestTestCase(t *testing.T) { }, }, prepare: func() { - gock.New("http://localhost"). + gock.New(urlLocalhost). Post("/foo").Reply(http.StatusOK).BodyString(`{"items":[]}`) }, - verify: func(t *testing.T, output interface{}, err error) { - assert.Nil(t, err) - }, + verify: noError, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -391,6 +338,9 @@ func TestTestCase(t *testing.T) { if tt.prepare != nil { tt.prepare() } + if tt.verify == nil { + tt.verify = hasError + } runner := NewSimpleTestCaseRunner().WithOutputWriter(os.Stdout) if tt.execer != nil { runner.WithExecer(tt.execer) @@ -465,5 +415,21 @@ const defaultSchemaForTest = `{"properties": { "type":"object" }` +func hasError(t *testing.T, output interface{}, err error) { + assert.NotNil(t, err) +} + +func noError(t *testing.T, output interface{}, err error) { + assert.Nil(t, err) +} + +func prepareForFoo() { + gock.New(urlLocalhost). + Get("/foo").Reply(http.StatusOK).BodyString(genericBody) +} + //go:embed testdata/generic_response.json var genericBody string + +const urlFoo = "http://localhost/foo" +const urlLocalhost = "http://localhost" diff --git a/pkg/server/remote_server.go b/pkg/server/remote_server.go index a15cc88..4103093 100644 --- a/pkg/server/remote_server.go +++ b/pkg/server/remote_server.go @@ -24,16 +24,19 @@ func NewRemoteServer() RunnerServer { return &server{} } +func withDefaultValue(old, defVal any) any { + if old == "" || old == nil { + old = defVal + } + return old +} + // Run start to run the test task func (s *server) Run(ctx context.Context, task *TestTask) (reply *HelloReply, err error) { - if task.Level == "" { - task.Level = "info" - } + task.Level = withDefaultValue(task.Level, "info").(string) + task.Env = withDefaultValue(task.Env, map[string]string{}).(map[string]string) var suite *testing.TestSuite - if task.Env == nil { - task.Env = map[string]string{} - } // TODO may not safe in multiple threads oldEnv := map[string]string{} diff --git a/pkg/server/remote_server_test.go b/pkg/server/remote_server_test.go index 40b1686..cdf5869 100644 --- a/pkg/server/remote_server_test.go +++ b/pkg/server/remote_server_test.go @@ -159,6 +159,13 @@ func TestUniqueSlice(t *testing.T) { assert.Equal(t, []string{"a", "b"}, uniqueSlice.GetAll()) } +func TestWithDefaultValue(t *testing.T) { + assert.Equal(t, withDefaultValue("a", "b"), "a") + assert.Equal(t, withDefaultValue("", "b"), "b") + assert.Equal(t, withDefaultValue(nil, map[string]string{"key": "val"}), map[string]string{"key": "val"}) + assert.Equal(t, withDefaultValue(map[string]string{"key": "val"}, map[string]string{"key": "value"}), map[string]string{"key": "val"}) +} + //go:embed testdata/simple.yaml var simpleSuite string