diff --git a/integration/integration_azurerm_test.go b/integration/integration_azurerm_test.go index e36bde4..f3b4e35 100644 --- a/integration/integration_azurerm_test.go +++ b/integration/integration_azurerm_test.go @@ -244,8 +244,7 @@ func TestAPIDiscoveryAzureRM(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() scenario(t, "api/azurerm", tt.name) - gh := newMockGitHub(t, "testdata/azurerm", azurermPRs) - gh.conflicted = conflictedPRs + gh := newMockGitHub(t, "testdata/azurerm", azurermPRs, withConflicted(conflictedPRs)) tc := newMockTeamCity(t) res := runTCTest(t, azurermEnv(gh, tc), tt.args...) @@ -381,8 +380,7 @@ func TestASTDiscoveryAzureRM(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() scenario(t, "ast/azurerm", tt.name) - gh := newMockGitHub(t, "testdata/azurerm", azurermASTPRs) - gh.conflicted = conflictedPRs + gh := newMockGitHub(t, "testdata/azurerm", azurermASTPRs, withConflicted(conflictedPRs)) tc := newMockTeamCity(t) env := azurermEnv(gh, tc) env["TCTEST_LOCAL_REPO_PATH"] = cloneUpstream(t, azurermUpstream) @@ -426,8 +424,7 @@ func TestErrorMessages(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() scenario(t, "errors/azurerm", tt.name) - gh := newMockGitHub(t, "testdata/azurerm", azurermPRs) - gh.conflicted = conflictedPRs + gh := newMockGitHub(t, "testdata/azurerm", azurermPRs, withConflicted(conflictedPRs)) tc := newMockTeamCity(t) res := runTCTest(t, azurermEnv(gh, tc), tt.args...) @@ -529,8 +526,7 @@ func TestPrsCommand(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() scenario(t, "prs/azurerm", tt.name) - gh := newMockGitHub(t, "testdata/azurerm", azurermPRs) - gh.openPRs = openPRs + gh := newMockGitHub(t, "testdata/azurerm", azurermPRs, withOpenPRs(openPRs)) tc := newMockTeamCity(t) res := runTCTest(t, azurermEnv(gh, tc), tt.args...) diff --git a/integration/integration_test.go b/integration/integration_test.go index 4cd0b7e..a1a6c25 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -52,10 +52,11 @@ func TestMain(m *testing.M) { return 1 } - // build the real binary once + // build the real binary once, race-instrumented so a data race in the CLI surfaces as a test failure (the + // race runtime prints a report and exits non-zero, which the exit-code and output assertions catch) binPath = filepath.Join(tmp, "tctest") - build := exec.CommandContext(context.Background(), "go", "build", "-o", binPath, ".") //nolint:gosec // building the repo's own binary into a temp dir - build.Dir = ".." // tests run with CWD = the test package dir + build := exec.CommandContext(context.Background(), "go", "build", "-race", "-o", binPath, ".") //nolint:gosec // building the repo's own binary into a temp dir + build.Dir = ".." // tests run with CWD = the test package dir if out, err := build.CombinedOutput(); err != nil { fmt.Fprintf(os.Stderr, "integration test setup: building tctest: %v\n%s\n", err, out) return 1 @@ -186,12 +187,31 @@ type mockGitHub struct { conflicted map[int]bool // PRs served with mergeable=false (and, like real GitHub, a stale merge_commit_sha) } -func newMockGitHub(t *testing.T, fixtureDir string, prs []prDef) *mockGitHub { +// mockGitHubOption configures a mockGitHub before its server starts. The handler goroutines read these fields +// without locking, so they must be fully populated before httptest.NewServer spawns them — setting them on the +// returned struct afterwards is a data race the race detector flags even though the binary under test has not +// connected yet. +type mockGitHubOption func(*mockGitHub) + +// withConflicted marks the given PRs as mergeable=false. +func withConflicted(conflicted map[int]bool) mockGitHubOption { + return func(m *mockGitHub) { m.conflicted = conflicted } +} + +// withOpenPRs sets the list served by GET /repos/{o}/{r}/pulls for the prs command. +func withOpenPRs(openPRs []listPR) mockGitHubOption { + return func(m *mockGitHub) { m.openPRs = openPRs } +} + +func newMockGitHub(t *testing.T, fixtureDir string, prs []prDef, opts ...mockGitHubOption) *mockGitHub { t.Helper() m := &mockGitHub{fixture: fixtureDir, prs: map[int]prDef{}} for _, pr := range prs { m.prs[pr.number] = pr } + for _, opt := range opts { + opt(m) + } m.srv = httptest.NewServer(http.HandlerFunc(m.handle)) t.Cleanup(m.srv.Close) return m diff --git a/makefile b/makefile index eeae186..44ac2d9 100644 --- a/makefile +++ b/makefile @@ -120,9 +120,9 @@ depscheck: ## Check that go.mod/go.sum and vendor/ are in sync (echo; echo "golangci-lint version mismatch: .tools/go.mod has $$modv but .tools/.custom-gcl.yml has $$gclv - update .custom-gcl.yml to match."; exit 1) ##@ Testing -test: build ## Run unit and integration tests - go test $$(go list ./... | grep -v '/integration$$') -timeout ${TEST_TIMEOUT} - @set -o pipefail; go test ./integration/ -v -timeout ${TEST_TIMEOUT} | grep -vE "^=== |--- PASS|^PASS$$" +test: build ## Run unit and integration tests under the race detector + go test -race $$(go list ./... | grep -v '/integration$$') -timeout ${TEST_TIMEOUT} + @set -o pipefail; go test -race ./integration/ -v -timeout ${TEST_TIMEOUT} | grep -vE "^=== |--- PASS|^PASS$$" check-all: build test lint actionlint yamllint shellcheck depscheck ## Run build + test + all linters + depscheck