From a89d96064a40c227f43b7803a7275140f450128a Mon Sep 17 00:00:00 2001 From: Nadia Santalla Date: Wed, 13 Nov 2024 15:32:56 +0100 Subject: [PATCH 1/6] output_test: run tests in parallel --- output_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/output_test.go b/output_test.go index 34a2fb7..0a95d31 100644 --- a/output_test.go +++ b/output_test.go @@ -12,6 +12,8 @@ import ( ) func TestOutputNew(t *testing.T) { + t.Parallel() + testcases := map[string]struct { input output.Params expectError bool @@ -45,11 +47,15 @@ func TestOutputNew(t *testing.T) { } func TestOutputDescription(t *testing.T) { + t.Parallel() + var out Output require.NotEmpty(t, out.Description()) } func TestOutputStart(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() out, err := New(output.Params{ConfigArgument: "test.out", FS: fs}) @@ -69,6 +75,8 @@ func TestOutputStart(t *testing.T) { // TestOutputStop tests that the metrics are correctly collected and written to the file. func TestOutputStop(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() out, err := New(output.Params{ConfigArgument: "test.out", FS: fs}) @@ -99,6 +107,7 @@ func makeSample(name string, value float64) metrics.Sample { } func TestDeriveMetricNameAndValue(t *testing.T) { + t.Parallel() testcases := map[string]struct { input metrics.Sample @@ -138,7 +147,11 @@ func TestDeriveMetricNameAndValue(t *testing.T) { } for name, tc := range testcases { + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + actualName, actualValue := deriveMetricNameAndValue(tc.input) require.Equal(t, tc.expectedName, actualName) require.Equal(t, tc.expectedValue, actualValue) @@ -147,6 +160,8 @@ func TestDeriveMetricNameAndValue(t *testing.T) { } func TestGetStats(t *testing.T) { + t.Parallel() + testcases := map[string]struct { input []float64 expected stats @@ -174,7 +189,11 @@ func TestGetStats(t *testing.T) { } for name, tc := range testcases { + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + actual := getStats(tc.input) if tc.expected != actual { t.Log("expected:", tc.expected, "actual:", actual) @@ -185,6 +204,8 @@ func TestGetStats(t *testing.T) { } func TestIsValidMetricName(t *testing.T) { + t.Parallel() + testcases := map[string]struct { input string expected bool @@ -208,7 +229,11 @@ func TestIsValidMetricName(t *testing.T) { } for name, tc := range testcases { + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + actual := isValidMetricNameRe(tc.input) if actual != tc.expected { t.Log("expected:", tc.expected, "actual:", actual, "input:", tc.input) @@ -225,6 +250,8 @@ func TestIsValidMetricName(t *testing.T) { } func TestSanitizeLabelName(t *testing.T) { + t.Parallel() + testcases := map[string]struct { input string expected string @@ -258,6 +285,8 @@ func TestSanitizeLabelName(t *testing.T) { } func TestBufferedMetricTextOutputValue(t *testing.T) { + t.Parallel() + type metricData struct { name string kvs []string @@ -338,7 +367,11 @@ func TestBufferedMetricTextOutputValue(t *testing.T) { } for name, tc := range testcases { + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + var buf bytes.Buffer to := newBufferedMetricTextOutput(&buf, tc.kvs...) for _, d := range tc.data { @@ -358,6 +391,8 @@ func joinNewline(s ...string) string { } func TestBufferedMetricTextOutputStats(t *testing.T) { + t.Parallel() + type metricData struct { name string kvs []string @@ -495,6 +530,8 @@ func TestBufferedMetricTextOutputStats(t *testing.T) { } func TestTargetMetricsCollectionWriteOne(t *testing.T) { + t.Parallel() + c := newTargetMetricsCollection() require.Len(t, c, 0) @@ -545,6 +582,8 @@ func TestTargetMetricsCollectionWriteOne(t *testing.T) { } func TestTargetMetricsCollectionWriteMany(t *testing.T) { + t.Parallel() + c := newTargetMetricsCollection() require.Len(t, c, 0) From d8e038585661f57b81f3744d00e1a5b059c1bca5 Mon Sep 17 00:00:00 2001 From: Nadia Santalla Date: Wed, 13 Nov 2024 15:40:55 +0100 Subject: [PATCH 2/6] output_test: skip broken tests --- output_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/output_test.go b/output_test.go index 0a95d31..d61676e 100644 --- a/output_test.go +++ b/output_test.go @@ -54,6 +54,8 @@ func TestOutputDescription(t *testing.T) { } func TestOutputStart(t *testing.T) { + t.Skip("Skipping broken test") + t.Parallel() fs := afero.NewMemMapFs() @@ -75,6 +77,8 @@ func TestOutputStart(t *testing.T) { // TestOutputStop tests that the metrics are correctly collected and written to the file. func TestOutputStop(t *testing.T) { + t.Skip("Skipping broken test") + t.Parallel() fs := afero.NewMemMapFs() @@ -530,6 +534,8 @@ func TestBufferedMetricTextOutputStats(t *testing.T) { } func TestTargetMetricsCollectionWriteOne(t *testing.T) { + t.Skip("Skipping broken test") + t.Parallel() c := newTargetMetricsCollection() @@ -582,6 +588,8 @@ func TestTargetMetricsCollectionWriteOne(t *testing.T) { } func TestTargetMetricsCollectionWriteMany(t *testing.T) { + t.Skip("Skiping broken test") + t.Parallel() c := newTargetMetricsCollection() From 23330afcd63593ddbb4b3c577554e1d1f8438749 Mon Sep 17 00:00:00 2001 From: Nadia Santalla Date: Wed, 13 Nov 2024 16:06:21 +0100 Subject: [PATCH 3/6] ci: run unit tests too --- .github/workflows/push-pr-release.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/push-pr-release.yaml b/.github/workflows/push-pr-release.yaml index bcdc108..418e755 100644 --- a/.github/workflows/push-pr-release.yaml +++ b/.github/workflows/push-pr-release.yaml @@ -45,8 +45,7 @@ jobs: # No point in running tests for foreign architectures we cannot run. if: steps.runner-info.outputs.native == 'true' run: |- - # TODO: Run also unit tests. They are currently broken and need fixing. - go test ./integration/... + go test ./... - name: Upload artifact to release if: github.event_name == 'release' From a9bc6efc8ed482f3d2adb152bbaae965d02456d3 Mon Sep 17 00:00:00 2001 From: Nadia Santalla Date: Wed, 13 Nov 2024 16:25:12 +0100 Subject: [PATCH 4/6] output_test: fix TestOutputStart expectations and lack of logger --- output_test.go | 42 ++++++++++++++---------------------------- 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/output_test.go b/output_test.go index d61676e..1c94afd 100644 --- a/output_test.go +++ b/output_test.go @@ -2,9 +2,11 @@ package sm import ( "bytes" + "io" "strings" "testing" + "github.com/sirupsen/logrus" "github.com/spf13/afero" "github.com/stretchr/testify/require" "go.k6.io/k6/metrics" @@ -53,14 +55,12 @@ func TestOutputDescription(t *testing.T) { require.NotEmpty(t, out.Description()) } -func TestOutputStart(t *testing.T) { - t.Skip("Skipping broken test") - +func TestOutputStartStop(t *testing.T) { t.Parallel() fs := afero.NewMemMapFs() - out, err := New(output.Params{ConfigArgument: "test.out", FS: fs}) + out, err := New(output.Params{ConfigArgument: "test.out", FS: fs, Logger: nopLogger()}) require.NoError(t, err) err = out.Start() @@ -69,34 +69,13 @@ func TestOutputStart(t *testing.T) { err = out.Stop() require.NoError(t, err) - // At this point we should have an empty file. - fi, err := fs.Stat("test.out") - require.NoError(t, err) - require.Equal(t, int64(0), fi.Size()) -} - -// TestOutputStop tests that the metrics are correctly collected and written to the file. -func TestOutputStop(t *testing.T) { - t.Skip("Skipping broken test") - - t.Parallel() - - fs := afero.NewMemMapFs() - - out, err := New(output.Params{ConfigArgument: "test.out", FS: fs}) - require.NoError(t, err) - - err = out.Start() + fileOut, err := fs.Open("test.out") require.NoError(t, err) - // TODO(mem): add samples - - err = out.Stop() + output, err := io.ReadAll(fileOut) require.NoError(t, err) - fi, err := fs.Stat("test.out") - require.NoError(t, err) - require.Equal(t, int64(0), fi.Size()) + require.Contains(t, string(output), "probe_script_duration_seconds") } func makeSample(name string, value float64) metrics.Sample { @@ -660,3 +639,10 @@ func TestTargetMetricsCollectionWriteMany(t *testing.T) { require.Equal(t, expected, buf.String()) } + +func nopLogger() *logrus.Logger { + l := logrus.New() + l.SetOutput(io.Discard) + + return l +} From 1ab37f2a310559a2ec52e6e80d89de280f230e75 Mon Sep 17 00:00:00 2001 From: Nadia Santalla Date: Wed, 13 Nov 2024 16:49:30 +0100 Subject: [PATCH 5/6] output_test: add missing metrics to expectations --- output_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/output_test.go b/output_test.go index 1c94afd..cb8c578 100644 --- a/output_test.go +++ b/output_test.go @@ -513,8 +513,6 @@ func TestBufferedMetricTextOutputStats(t *testing.T) { } func TestTargetMetricsCollectionWriteOne(t *testing.T) { - t.Skip("Skipping broken test") - t.Parallel() c := newTargetMetricsCollection() @@ -550,6 +548,8 @@ func TestTargetMetricsCollectionWriteOne(t *testing.T) { c.Write(&buf) expected := joinNewline( + `probe_http_got_expected_response{url="http://example.com",method="GET",scenario="s",group="g"} 1`, + `probe_http_error_code{url="http://example.com",method="GET",scenario="s",group="g"} 0`, `probe_http_info{tls_version="1.3",proto="1.1",k="v",url="http://example.com",method="GET",scenario="s",group="g"} 1`, `probe_http_requests_total{url="http://example.com",method="GET",scenario="s",group="g"} 1`, `probe_http_requests_failed_total{url="http://example.com",method="GET",scenario="s",group="g"} 0`, @@ -561,6 +561,7 @@ func TestTargetMetricsCollectionWriteOne(t *testing.T) { `probe_http_duration_seconds{phase="tls",url="http://example.com",method="GET",scenario="s",group="g"} 0.001`, `probe_http_duration_seconds{phase="processing",url="http://example.com",method="GET",scenario="s",group="g"} 0.001`, `probe_http_duration_seconds{phase="transfer",url="http://example.com",method="GET",scenario="s",group="g"} 0.001`, + `probe_http_total_duration_seconds{url="http://example.com",method="GET",scenario="s",group="g"} 0.001`, ) require.Equal(t, expected, buf.String()) From 8978e271bff4311874b1e9748e0a14a25cdb22a6 Mon Sep 17 00:00:00 2001 From: Nadia Santalla Date: Wed, 13 Nov 2024 16:52:37 +0100 Subject: [PATCH 6/6] output_test: add missing metrics to expectations (bis) --- output_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/output_test.go b/output_test.go index cb8c578..af0f573 100644 --- a/output_test.go +++ b/output_test.go @@ -568,8 +568,6 @@ func TestTargetMetricsCollectionWriteOne(t *testing.T) { } func TestTargetMetricsCollectionWriteMany(t *testing.T) { - t.Skip("Skiping broken test") - t.Parallel() c := newTargetMetricsCollection() @@ -605,6 +603,8 @@ func TestTargetMetricsCollectionWriteMany(t *testing.T) { c.Write(&buf) expected := joinNewline( + `probe_http_got_expected_response{url="http://example.com",method="GET",scenario="s",group="g"} 1`, + `probe_http_error_code{url="http://example.com",method="GET",scenario="s",group="g"} 0`, `probe_http_info{tls_version="1.3",proto="1.1",k="v",url="http://example.com",method="GET",scenario="s",group="g"} 1`, `probe_http_requests_total{url="http://example.com",method="GET",scenario="s",group="g"} 2`, `probe_http_requests_failed_total{url="http://example.com",method="GET",scenario="s",group="g"} 1`, @@ -636,6 +636,11 @@ func TestTargetMetricsCollectionWriteMany(t *testing.T) { `probe_http_duration_seconds{phase="transfer",url="http://example.com",method="GET",scenario="s",group="g"} 0.001`, `probe_http_duration_seconds_count{phase="transfer",url="http://example.com",method="GET",scenario="s",group="g"} 2`, `probe_http_duration_seconds_sum{phase="transfer",url="http://example.com",method="GET",scenario="s",group="g"} 0.002`, + `probe_http_total_duration_seconds_min{url="http://example.com",method="GET",scenario="s",group="g"} 0.001`, + `probe_http_total_duration_seconds_max{url="http://example.com",method="GET",scenario="s",group="g"} 0.001`, + `probe_http_total_duration_seconds{url="http://example.com",method="GET",scenario="s",group="g"} 0.001`, + `probe_http_total_duration_seconds_count{url="http://example.com",method="GET",scenario="s",group="g"} 2`, + `probe_http_total_duration_seconds_sum{url="http://example.com",method="GET",scenario="s",group="g"} 0.002`, ) require.Equal(t, expected, buf.String())