From 976f3273f90af18a0dbb4af1573b09d043295bc5 Mon Sep 17 00:00:00 2001 From: CtrlSpice Date: Fri, 18 Oct 2024 13:46:25 -0700 Subject: [PATCH] made tests nicer, and other changes based on review --- .github/{workflow => workflows}/go.yml | 0 .../{workflow => workflows}/integration.sh | 0 .github/{workflow => workflows}/span.json | 0 desktopexporter/go.sum | 6 +- desktopexporter/internal/server/server.go | 10 +- .../internal/server/server_test.go | 251 ++++++------------ 6 files changed, 83 insertions(+), 184 deletions(-) rename .github/{workflow => workflows}/go.yml (100%) rename .github/{workflow => workflows}/integration.sh (100%) rename .github/{workflow => workflows}/span.json (100%) diff --git a/.github/workflow/go.yml b/.github/workflows/go.yml similarity index 100% rename from .github/workflow/go.yml rename to .github/workflows/go.yml diff --git a/.github/workflow/integration.sh b/.github/workflows/integration.sh similarity index 100% rename from .github/workflow/integration.sh rename to .github/workflows/integration.sh diff --git a/.github/workflow/span.json b/.github/workflows/span.json similarity index 100% rename from .github/workflow/span.json rename to .github/workflows/span.json diff --git a/desktopexporter/go.sum b/desktopexporter/go.sum index 0b59d00..516b1bb 100644 --- a/desktopexporter/go.sum +++ b/desktopexporter/go.sum @@ -26,8 +26,6 @@ github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= -github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ= github.com/hashicorp/go-version v1.7.0 h1:5tqGy27NaOTB8yJKUZELlFAS/LTKJkrmONwQKeRZfjY= github.com/hashicorp/go-version v1.7.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= @@ -85,6 +83,8 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/zeebo/assert v1.3.0 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ= +github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0= github.com/zeebo/xxh3 v1.0.2 h1:xZmwmqxHZA8AI603jOQ0tMqmBr9lPeFwGg6d+xy9DC0= github.com/zeebo/xxh3 v1.0.2/go.mod h1:5NWz9Sef7zIDm2JHfFlcQvNekmcEl9ekUZQQKCYaDcA= go.opentelemetry.io/collector v0.107.0 h1:C1Mng03iE73flGhEg795IFVlr3qhDLef5GESjIVtx5g= @@ -180,6 +180,8 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028 h1:+cNy6SZtPcJQH3LJVLOSmiC7MMxXNOb3PU/VUEz+EhU= golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028/go.mod h1:NDW/Ps6MPRej6fsCIbMTohpP40sJ/P/vI1MoTEGwX90= +gonum.org/v1/gonum v0.15.0 h1:2lYxjRbTYyxkJxlhC+LvJIx3SsANPdRybu1tGj9/OrQ= +gonum.org/v1/gonum v0.15.0/go.mod h1:xzZVBJBtS+Mz4q0Yl2LJTk+OxOg4jiXZ7qBoM0uISGo= google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 h1:BwIjyKYGsK9dMCBOorzRri8MQwmi7mT9rGHsCEinZkA= google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094/go.mod h1:Ue6ibwXGpU+dqIcODieyLOcgj7z8+IcskoNIgZxtrFY= google.golang.org/grpc v1.65.0 h1:bs/cUb4lp1G5iImFFd3u5ixQzweKizoZJAwBNLR42lc= diff --git a/desktopexporter/internal/server/server.go b/desktopexporter/internal/server/server.go index de6a5cc..1ec25df 100644 --- a/desktopexporter/internal/server/server.go +++ b/desktopexporter/internal/server/server.go @@ -63,11 +63,11 @@ func (s *Server) Close() error { func (s *Server) Handler(serveFromFS bool) http.Handler { router := http.NewServeMux() - router.HandleFunc("/api/traces", s.tracesHandler) - router.HandleFunc("/api/traces/{id}", s.traceIDHandler) - router.HandleFunc("/api/sampleData", s.sampleDataHandler) - router.HandleFunc("/api/clearData", s.clearTracesHandler) - router.HandleFunc("/traces/{id}", indexHandler) + router.HandleFunc("GET /api/traces", s.tracesHandler) + router.HandleFunc("GET /api/traces/{id}", s.traceIDHandler) + router.HandleFunc("GET /api/sampleData", s.sampleDataHandler) + router.HandleFunc("GET /api/clearData", s.clearTracesHandler) + router.HandleFunc("GET /traces/{id}", indexHandler) if serveFromFS { router.Handle("/", http.FileServer(http.Dir("./static/"))) diff --git a/desktopexporter/internal/server/server_test.go b/desktopexporter/internal/server/server_test.go index 1559580..efece4d 100644 --- a/desktopexporter/internal/server/server_test.go +++ b/desktopexporter/internal/server/server_test.go @@ -1,7 +1,6 @@ package server import ( - "bytes" "context" "encoding/json" "fmt" @@ -16,8 +15,18 @@ import ( "github.com/stretchr/testify/assert" ) -func setupWithTrace(t *testing.T) (*Server, func(*testing.T)) { - s := NewServer("localhost:8000") +func setupEmpty() (*httptest.Server, func()) { + server := NewServer("localhost:8000") + testServer := httptest.NewServer(server.Handler(false)) + + return testServer, func() { + testServer.Close() + server.Store.Close() + } +} + +func setupWithTrace(t *testing.T) (*httptest.Server, func(*testing.T)) { + server := NewServer("localhost:8000") testSpanData := telemetry.SpanData{ TraceID: "1234567890", TraceState: "", @@ -44,18 +53,31 @@ func setupWithTrace(t *testing.T) (*Server, func(*testing.T)) { StatusMessage: "", } - if err := s.Store.AddSpans(context.Background(), []telemetry.SpanData{testSpanData}); err != nil { - t.Fatalf("could not create test span: %v", err) - } - return s, func(t *testing.T) { - s.Store.Close() + err := server.Store.AddSpans(context.Background(), []telemetry.SpanData{testSpanData}) + assert.Nilf(t, err, "could not create test span: %v", err) + + testServer := httptest.NewServer(server.Handler(false)) + + return testServer, func(t *testing.T) { + testServer.Close() + server.Store.Close() } } func TestTracesHandler(t *testing.T) { t.Run("Traces Handler (Empty)", func(t *testing.T) { - s := NewServer("localhost:8000") - defer s.Store.Close() + testServer, teardown := setupEmpty() + defer teardown() + + res, err := http.Get(fmt.Sprintf("%s%s", testServer.URL, "/api/traces")) + assert.Nilf(t, err, "could not send GET request %v", err) + defer res.Body.Close() + assert.Equal(t, http.StatusOK, res.StatusCode) + + b, err := io.ReadAll(res.Body) + assert.Nilf(t, err, "could not read response body: %v", err) + + // Init summaries struct with some data to be overwritten testSummaries := telemetry.TraceSummaries{ TraceSummaries: []telemetry.TraceSummary{ { @@ -69,62 +91,28 @@ func TestTracesHandler(t *testing.T) { }, }, } - - req, err := http.NewRequest("GET", "/api/traces", nil) - if err != nil { - t.Fatalf("could not create request: %v", err) - } - - rec := httptest.NewRecorder() - s.tracesHandler(rec, req) - res := rec.Result() - defer res.Body.Close() - - b, err := io.ReadAll(res.Body) - if err != nil { - t.Fatalf("could not read response: %v", err) - } - - if res.StatusCode != http.StatusOK { - t.Errorf("expected status OK; got %v", res.Status) - } - err = json.Unmarshal(b, &testSummaries) - if err != nil { - t.Fatalf("could not unmarshal bytes to trace summaries: %v", err.Error()) - } + assert.Nilf(t, err, "could not unmarshal bytes to trace summaries: %v", err) assert.Len(t, testSummaries.TraceSummaries, 0) }) t.Run("Traces Handler (Not Empty)", func(t *testing.T) { - testSummaries := telemetry.TraceSummaries{} - s, teardown := setupWithTrace(t) + testServer, teardown := setupWithTrace(t) defer teardown(t) - req, err := http.NewRequest("GET", "/api/traces", nil) - if err != nil { - t.Fatalf("could not create request: %v", err) - } - - rec := httptest.NewRecorder() - s.tracesHandler(rec, req) - res := rec.Result() + res, err := http.Get(fmt.Sprintf("%s%s", testServer.URL, "/api/traces")) + assert.Nilf(t, err, "could not send GET request: %v", err) defer res.Body.Close() - b, err := io.ReadAll(res.Body) - if err != nil { - t.Fatalf("could not read response: %v", err) - } + assert.Equal(t, http.StatusOK, res.StatusCode) - if res.StatusCode != http.StatusOK { - t.Errorf("expected status OK; got %v", res.Status) - } + b, err := io.ReadAll(res.Body) + assert.Nilf(t, err, "could not read response body: %v", err) + testSummaries := telemetry.TraceSummaries{} err = json.Unmarshal(b, &testSummaries) - if err != nil { - t.Fatalf("could not unmarshal bytes to trace summaries: %v", err.Error()) - } + assert.Nilf(t, err, "could not unmarshal bytes to trace summaries: %v", err) assert.Equal(t, "1234567890", testSummaries.TraceSummaries[0].TraceID) assert.Equal(t, true, testSummaries.TraceSummaries[0].HasRootSpan) @@ -135,45 +123,30 @@ func TestTracesHandler(t *testing.T) { } func TestTraceIDHandler(t *testing.T) { - s, teardown := setupWithTrace(t) + testServer, teardown := setupWithTrace(t) defer teardown(t) - srv := httptest.NewServer(s.Handler(false)) - defer srv.Close() - t.Run("Trace ID Handler (Not Found)", func(t *testing.T) { - res, err := http.Get(fmt.Sprintf("%s%s", srv.URL, "/api/traces/987654321")) - if err != nil { - t.Fatalf("could not send GET request: %v", err) - } + res, err := http.Get(fmt.Sprintf("%s%s", testServer.URL, "/api/traces/987654321")) + assert.Nilf(t, err, "could not send GET request: %v", err) defer res.Body.Close() - if res.StatusCode != http.StatusBadRequest { - t.Errorf("expected status 400 Bad Request; got %v", res.Status) - } + assert.Equal(t, http.StatusBadRequest, res.StatusCode) }) t.Run("Traces ID Handler (ID Found)", func(t *testing.T) { - res, err := http.Get(fmt.Sprintf("%s%s", srv.URL, "/api/traces/1234567890")) - if err != nil { - t.Fatalf("could not send GET request: %v", err) - } + res, err := http.Get(fmt.Sprintf("%s%s", testServer.URL, "/api/traces/1234567890")) + assert.Nilf(t, err, "could not send GET request: %v", err) defer res.Body.Close() - if res.StatusCode != http.StatusOK { - t.Fatalf("expected status OK; got %v", res.Status) - } + assert.Equal(t, http.StatusOK, res.StatusCode) b, err := io.ReadAll(res.Body) - if err != nil { - t.Fatalf("could not read response: %v", err) - } + assert.Nilf(t, err, "could not read response body: %v", err) testTrace := telemetry.TraceData{} err = json.Unmarshal(b, &testTrace) - if err != nil { - t.Fatalf("could not unmarshal bytes to trace summaries: %v", err.Error()) - } + assert.Nilf(t, err, "could not unmarshal bytes to trace data: %v", err) assert.Equal(t, "1234567890", testTrace.TraceID) assert.Equal(t, "12345", testTrace.Spans[0].SpanID) @@ -184,91 +157,57 @@ func TestTraceIDHandler(t *testing.T) { } func TestClearTracesHandler(t *testing.T) { - s, teardown := setupWithTrace(t) + testServer, teardown := setupWithTrace(t) defer teardown(t) - testSummaries := telemetry.TraceSummaries{} - - // Clear traces - req, err := http.NewRequest("GET", "/api/clearData", nil) - if err != nil { - t.Fatalf("could not create request: %v", err) - } - - rec := httptest.NewRecorder() - s.clearTracesHandler(rec, req) - res := rec.Result() + // Clear dat data + res, err := http.Get(fmt.Sprintf("%s%s", testServer.URL, "/api/clearData")) + assert.Nilf(t, err, "could not send GET request: %v", err) defer res.Body.Close() - if res.StatusCode != http.StatusOK { - t.Errorf("expected status OK; got %v", res.Status) - } + assert.Equal(t, http.StatusOK, res.StatusCode) // Get trace summaries - req, err = http.NewRequest("GET", "/api/traces", nil) - if err != nil { - t.Fatalf("could not create request: %v", err) - } + res, err = http.Get(fmt.Sprintf("%s%s", testServer.URL, "/api/traces")) + assert.Nilf(t, err, "could not send GET request: %v", err) - rec = httptest.NewRecorder() - s.tracesHandler(rec, req) - res = rec.Result() + assert.Equal(t, http.StatusOK, res.StatusCode) b, err := io.ReadAll(res.Body) - if err != nil { - t.Fatalf("could not read response: %v", err) - } - - if res.StatusCode != http.StatusOK { - t.Errorf("expected status OK; got %v", res.Status) - } + assert.Nilf(t, err, "could not read response body: %v", err) + testSummaries := telemetry.TraceSummaries{} err = json.Unmarshal(b, &testSummaries) - if err != nil { - t.Fatalf("could not unmarshal bytes to trace summaries: %v", err.Error()) - } + assert.Nilf(t, err, "could not unmarshal bytes to trace summaries: %v", err) // Check that there are no traces in store assert.Len(t, testSummaries.TraceSummaries, 0) } func TestSampleHandler(t *testing.T) { - s := NewServer("localhost:8000") - defer s.Store.Close() + testServer, teardown := setupEmpty() + defer teardown() + + // Populate sample data + res, err := http.Get(fmt.Sprintf("%s%s", testServer.URL, "/api/sampleData")) + assert.Nilf(t, err, "could not send GET request: %v", err) + defer res.Body.Close() - srv := httptest.NewServer(s.Handler(false)) - defer srv.Close() + assert.Equal(t, http.StatusOK, res.StatusCode) t.Run("Sample Data Handler (Traces)", func(t *testing.T) { - res, err := http.Get(fmt.Sprintf("%s%s", srv.URL, "/api/sampleData")) - if err != nil { - t.Fatalf("could not send GET request: %v", err) - } + res, err := http.Get(fmt.Sprintf("%s%s", testServer.URL, "/api/traces/42957c7c2fca940a0d32a0cdd38c06a4")) + assert.Nilf(t, err, "could not send GET request: %v", err) defer res.Body.Close() - if res.StatusCode != http.StatusOK { - t.Errorf("expected status OK; got %v", res.Status) - } - - res, err = http.Get(fmt.Sprintf("%s%s", srv.URL, "/api/traces/42957c7c2fca940a0d32a0cdd38c06a4")) - if err != nil { - t.Fatalf("could not send GET request: %v", err) - } - - if res.StatusCode != http.StatusOK { - t.Fatalf("expected status OK; got %v", res.Status) - } + assert.Equal(t, http.StatusOK, res.StatusCode) b, err := io.ReadAll(res.Body) - if err != nil { - t.Fatalf("could not read response: %v", err) - } + assert.Nilf(t, err, "could not read response body: %v", err) testTrace := telemetry.TraceData{} err = json.Unmarshal(b, &testTrace) - if err != nil { - t.Fatalf("could not unmarshal bytes to trace summaries: %v", err.Error()) - } + assert.Nilf(t, err, "could not unmarshal bytes to trace data: %v", err) assert.Equal(t, "42957c7c2fca940a0d32a0cdd38c06a4", testTrace.TraceID) assert.Equal(t, "37fd1349bf83d330", testTrace.Spans[0].SpanID) @@ -277,45 +216,3 @@ func TestSampleHandler(t *testing.T) { assert.Equal(t, 3, len(testTrace.Spans)) }) } - -func TestRouting(t *testing.T) { - // No need to start s, as we only need the Handler method - s := NewServer("localhost:8000") - defer s.Store.Close() - - testTable := []struct { - name string - route string - expected string - }{ - {"Traces Handler", "/api/traces", `{"traceSummaries":[]}`}, - {"Sample Data Handler", "/api/sampleData", ``}, - {"Clear Traces Handler", "/api/clearData", ``}, - } - - srv := httptest.NewServer(s.Handler(false)) - defer srv.Close() - - for _, tc := range testTable { - t.Run(tc.name, func(t *testing.T) { - res, err := http.Get(fmt.Sprintf("%s%s", srv.URL, tc.route)) - if err != nil { - t.Fatalf("could not send GET request: %v", err) - } - defer res.Body.Close() - - if res.StatusCode != http.StatusOK { - t.Errorf("expected status OK; got %v", res.Status) - } - - b, err := io.ReadAll(res.Body) - if err != nil { - t.Fatalf("could not read response: %v", err) - } - - if val := string(bytes.TrimSpace(b)); val != tc.expected { - t.Fatalf("expected %s; got %v", tc.expected, val) - } - }) - } -}