From f5061f53dbc41742732ac77044765c60f0f76b7f Mon Sep 17 00:00:00 2001 From: Michael McGuinness Date: Mon, 12 May 2025 15:11:50 +0000 Subject: [PATCH] Merged in feature/timeout (pull request #138) Remove race conditions from -race in tests * timeout * raceconditions * nobuildgp * build * perf --- internal/cognitoauth/jwks_test.go | 4 ++-- internal/serviceconfig/logger/testlogger.go | 6 ++++++ internal/test/aws.go | 7 ------- internal/test/container.go | 12 +++++++++--- internal/test/database.go | 7 ------- internal/test/ecosystem.go | 14 ++++++++++++-- internal/test/mockserver.go | 20 ++++++-------------- internal/test/mockserver_test.go | 2 +- scripts/tests.yml | 4 +++- 9 files changed, 39 insertions(+), 37 deletions(-) diff --git a/internal/cognitoauth/jwks_test.go b/internal/cognitoauth/jwks_test.go index 7b3bb3d9..610854c4 100644 --- a/internal/cognitoauth/jwks_test.go +++ b/internal/cognitoauth/jwks_test.go @@ -273,7 +273,7 @@ func TestFetchJWKS(t *testing.T) { t.Parallel() // Create a server that sleeps longer than the client timeout slowServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - time.Sleep(1100 * time.Millisecond) + time.Sleep(50 * time.Millisecond) _, err := w.Write([]byte(jwksJSON)) if err != nil { t.Errorf("Failed to write response: %v", err) @@ -281,7 +281,7 @@ func TestFetchJWKS(t *testing.T) { })) defer slowServer.Close() - _, err := fetchJWKSWithTimeout(slowServer.URL, time.Second) + _, err := fetchJWKSWithTimeout(slowServer.URL, 10*time.Millisecond) if err == nil { t.Errorf("fetchJWKS() with timeout should return error, got nil") } diff --git a/internal/serviceconfig/logger/testlogger.go b/internal/serviceconfig/logger/testlogger.go index 27166f73..06302065 100644 --- a/internal/serviceconfig/logger/testlogger.go +++ b/internal/serviceconfig/logger/testlogger.go @@ -3,12 +3,14 @@ package logger import ( "context" "log/slog" + "sync" "testing" ) type TestLogger struct { T *testing.T Logs []map[string]any + mu sync.Mutex } func (l *TestLogger) Handle(ctx context.Context, r slog.Record) error { @@ -17,7 +19,11 @@ func (l *TestLogger) Handle(ctx context.Context, r slog.Record) error { attrs[a.Key] = a.Value.Any() return true }) + + l.mu.Lock() + defer l.mu.Unlock() l.Logs = append(l.Logs, attrs) + return nil } diff --git a/internal/test/aws.go b/internal/test/aws.go index 6d4849d9..0ae3750b 100644 --- a/internal/test/aws.go +++ b/internal/test/aws.go @@ -5,7 +5,6 @@ import ( "io" "net/http" "strconv" - "sync" "testing" "queryorchestration/internal/serviceconfig/objectstore" @@ -31,8 +30,6 @@ const ( awsPort = 4566 ) -var awsLock sync.Mutex - func CreateAWSContainer(t testing.TB, cfg AWSConfigProvider, network string) *AWSContainerConfig { port, err := nat.NewPort("tcp", strconv.Itoa(awsPort)) require.NoError(t, err) @@ -79,8 +76,6 @@ func CreateAWSContainer(t testing.TB, cfg AWSConfigProvider, network string) *AW }, } - awsLock.Lock() - container, err := testcontainers.GenericContainer(t.Context(), testcontainers.GenericContainerRequest{ ContainerRequest: req, Started: true, @@ -93,8 +88,6 @@ func CreateAWSContainer(t testing.TB, cfg AWSConfigProvider, network string) *AW mappedPort, err := container.MappedPort(t.Context(), port) require.NoError(t, err) - awsLock.Unlock() - awsAddress := fmt.Sprintf("http://%s:%s", host, mappedPort.Port()) return &AWSContainerConfig{ diff --git a/internal/test/container.go b/internal/test/container.go index c89fbfde..9e391789 100644 --- a/internal/test/container.go +++ b/internal/test/container.go @@ -42,12 +42,14 @@ const ( dockerfilePath = "build/Dockerfile" ) -var ( +type ContainerLock struct { imageBuild sync.Once -) +} + +var containerLock = &ContainerLock{} func buildImage(t testing.TB, ctx context.Context) { - imageBuild.Do(func() { + containerLock.imageBuild.Do(func() { dockerContextPath, err := os.Getwd() require.NoError(t, err, "Failed to get current directory") @@ -181,6 +183,10 @@ func createContainer(t testing.TB, ctx context.Context, network string, cfg *con } func PrintContainerLogs(t testing.TB, container testcontainers.Container) { + if container == nil { + return + } + logs, err := container.Logs(t.Context()) require.NoError(t, err) defer func() { diff --git a/internal/test/database.go b/internal/test/database.go index 7d227b56..9ff7cae8 100644 --- a/internal/test/database.go +++ b/internal/test/database.go @@ -3,7 +3,6 @@ package test import ( "fmt" "strconv" - "sync" "testing" db "queryorchestration/internal/database" @@ -24,8 +23,6 @@ const ( dbPort = 5432 ) -var dbLock sync.Mutex - func CreateDB(t testing.TB, cfg serviceconfig.ConfigProvider, network string, dcfg *CreateDatabaseConfig) testcontainers.Container { port, err := nat.NewPort("tcp", strconv.Itoa(dbPort)) require.NoError(t, err) @@ -59,8 +56,6 @@ func CreateDB(t testing.TB, cfg serviceconfig.ConfigProvider, network string, dc }, } - dbLock.Lock() - container, err := testcontainers.GenericContainer(t.Context(), testcontainers.GenericContainerRequest{ ContainerRequest: req, Started: true, @@ -73,8 +68,6 @@ func CreateDB(t testing.TB, cfg serviceconfig.ConfigProvider, network string, dc mappedPort, err := container.MappedPort(t.Context(), port) require.NoError(t, err) - dbLock.Unlock() - cfg.SetDBHost(host) cfg.SetDBPort(mappedPort.Int()) diff --git a/internal/test/ecosystem.go b/internal/test/ecosystem.go index bea85da8..b8dae807 100644 --- a/internal/test/ecosystem.go +++ b/internal/test/ecosystem.go @@ -109,7 +109,7 @@ type Dependencies struct { QueueURLs map[RunnerName]string AWSConfig *AWSContainerConfig DBConfig testcontainers.Container - MockServer MockServer + MockServer *MockServer Network string } @@ -129,10 +129,20 @@ func CreateFullDependencies(t testing.TB, ctx context.Context, cfg FullDependenc SetQueueClient(t, ctx, cfg, deps.AWSConfig.ExternalEndpoint) + var lock sync.Mutex + for _, runner := range runners { wg.Add(1) go func() { - deps.QueueURLs[runner.Name] = CreateQueue(t, ctx, cfg, runner.Name) + + queue := CreateQueue(t, ctx, cfg, runner.Name) + + lock.Lock() + + deps.QueueURLs[runner.Name] = queue + + lock.Unlock() + wg.Done() }() } diff --git a/internal/test/mockserver.go b/internal/test/mockserver.go index a466e77e..3d6fe03c 100644 --- a/internal/test/mockserver.go +++ b/internal/test/mockserver.go @@ -10,7 +10,6 @@ import ( "log/slog" "net/http" "strconv" - "sync" "testing" "time" @@ -58,9 +57,7 @@ const ( mockServerPort = 1080 ) -var mockServerLock sync.Mutex - -func CreateMockServer(t testing.TB, network string) MockServer { +func CreateMockServer(t testing.TB, network string) *MockServer { port, err := nat.NewPort("tcp", strconv.Itoa(mockServerPort)) require.NoError(t, err) @@ -90,7 +87,6 @@ func CreateMockServer(t testing.TB, network string) MockServer { }, } - mockServerLock.Lock() container, err := testcontainers.GenericContainer(t.Context(), testcontainers.GenericContainerRequest{ ContainerRequest: req, Started: true, @@ -104,19 +100,15 @@ func CreateMockServer(t testing.TB, network string) MockServer { externalPort, err := container.MappedPort(t.Context(), port) require.NoError(t, err) - mockServerLock.Unlock() - - server := MockServer{ + return &MockServer{ Client: &http.Client{}, Internal: Address(fmt.Sprintf("http://%s:%d", mockServerAlias, port.Int())), External: Address(fmt.Sprintf("http://%s:%d", host, externalPort.Int())), Container: container, } - - return server } -func CreateMockExpectation(t testing.TB, server MockServer, expectation MockExpectation) { +func CreateMockExpectation(t testing.TB, server *MockServer, expectation MockExpectation) { jsonData, err := json.Marshal(expectation) require.NoError(t, err) @@ -135,7 +127,7 @@ func CreateMockExpectation(t testing.TB, server MockServer, expectation MockExpe } } -func CreateDetectDocumentTextExpectation(t testing.TB, mockServer MockServer, body string) MockExpectation { +func CreateDetectDocumentTextExpectation(t testing.TB, mockServer *MockServer, body string) MockExpectation { childId := uuid.NewString() expectation := MockExpectation{ Request: MockRequest{ @@ -182,7 +174,7 @@ func CreateDetectDocumentTextExpectation(t testing.TB, mockServer MockServer, bo return expectation } -func WaitForMockEndpoint(t testing.TB, server MockServer, request MockRequest) MockRequest { +func WaitForMockEndpoint(t testing.TB, server *MockServer, request MockRequest) MockRequest { t.Helper() verificationRequest := map[string]any{ @@ -236,7 +228,7 @@ func WaitForMockEndpoint(t testing.TB, server MockServer, request MockRequest) M } } -func retrieveMatchingRequest(t testing.TB, server MockServer, request MockRequest) MockRequest { +func retrieveMatchingRequest(t testing.TB, server *MockServer, request MockRequest) MockRequest { retrieveURL := fmt.Sprintf("%s/mockserver/retrieve?type=REQUESTS", server.External) req, err := http.NewRequest("PUT", retrieveURL, nil) diff --git a/internal/test/mockserver_test.go b/internal/test/mockserver_test.go index 44281633..e5048797 100644 --- a/internal/test/mockserver_test.go +++ b/internal/test/mockserver_test.go @@ -17,7 +17,7 @@ func TestCreateDetectDocumentTextExpectation(t *testing.T) { })) defer ts.Close() - server := MockServer{ + server := &MockServer{ Client: ts.Client(), External: Address(ts.URL), } diff --git a/scripts/tests.yml b/scripts/tests.yml index 8a42bcfd..a7d17d61 100644 --- a/scripts/tests.yml +++ b/scripts/tests.yml @@ -49,7 +49,9 @@ tasks: vars: NUM_RESULTS: 5 cmds: - - GOMAXPROCS={{.TEST_PARALLEL}} go test -parallel {{.CPU_COUNT}} -json ./... + - | + GOMAXPROCS={{.TEST_PARALLEL}} go test -parallel {{.CPU_COUNT}} -count=1 -json ./... \ + > {{.TEST_FILE}} - | echo -e "🐢 TOP {{.NUM_RESULTS}} SLOWEST INDIVIDUAL PACKAGE RUNS:" jq -r 'select(.Action == "pass" and .Test == null) | "\(.Elapsed)s \(.Package)"' "{{.TEST_FILE}}" | \