From 059a79525adf1bdd11bd38a1b202bf4cca8e8913 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 11 Oct 2026 00:07:25 +0000 Subject: [PATCH] Synchronize delivery test fixtures and record the concurrency audit --- cmd/releasekit/assets_test.go | 24 +- cmd/releasekit/committed_identity_test.go | 14 +- cmd/releasekit/local_module_test.go | 6 +- cmd/releasekit/main_test.go | 232 ++++++++++-------- .../multi_module_woodpecker_test.go | 112 +++++---- .../releasekit_delivery_workflow_test.go | 24 +- cmd/releasekit/woodpecker_test.go | 20 +- delivery_reads_linux_test.go | 5 +- delivery_test.go | 170 ++++++++----- docs/roadmap/audit.md | 152 ++++++++++++ docs/roadmap/promptkit-bug-report.md | 4 +- docs/roadmap/race.md | 12 +- internal/gitea/assets_test.go | 17 +- internal/gitea/release_test.go | 140 ++++++----- performance_delivery_test.go | 5 +- repository_test.go | 2 +- source_inspection_test.go | 4 +- 17 files changed, 615 insertions(+), 328 deletions(-) create mode 100644 docs/roadmap/audit.md diff --git a/cmd/releasekit/assets_test.go b/cmd/releasekit/assets_test.go index 7a6b5dc..25443d1 100644 --- a/cmd/releasekit/assets_test.go +++ b/cmd/releasekit/assets_test.go @@ -37,7 +37,7 @@ func TestBuildAndPrebuiltDeliveryUseSameDirectoryBase(t *testing.T) { } } writeRepositoryFile(t, repository, ".gitignore", "dist/\n") - state := &deliveryServerState{tag: tag} + state := &deliveryServerState{data: deliveryServerData{tag: tag}} server := newDeliveryServer(t, state) defer server.Close() settings := cli.Settings{Product: "Example Product", ModuleDir: tt.module, @@ -47,7 +47,7 @@ func TestBuildAndPrebuiltDeliveryUseSameDirectoryBase(t *testing.T) { writeDeliveryProfile(t, repository, settings) cliGitRun(t, repository, "add", ".") cliGitRun(t, repository, "commit", "-m", "Prepare module source and output exclusion") - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", tag) output, resolved := "dist", filepath.Join(moduleDirectory, "dist") if tt.absolute { @@ -73,12 +73,12 @@ func TestBuildAndPrebuiltDeliveryUseSameDirectoryBase(t *testing.T) { args := []string{"deliver", "--repo", invocationRepository, "--assets", output, "v1.2.3"} stdout.Reset() stderr.Reset() - if status := run(context.Background(), args, &stdout, &stderr); status != 0 || state.releaseCreates != 1 || state.assetUploads != 2 { - t.Fatalf("delivery status=%d, creates=%d, uploads=%d, stderr=%s", status, state.releaseCreates, state.assetUploads, stderr.String()) + if status := run(context.Background(), args, &stdout, &stderr); status != 0 || state.snapshot().releaseCreates != 1 || state.snapshot().assetUploads != 2 { + t.Fatalf("delivery status=%d, creates=%d, uploads=%d, stderr=%s", status, state.snapshot().releaseCreates, state.snapshot().assetUploads, stderr.String()) } if tt.alias { - if status := run(context.Background(), []string{"deliver", "--repo", repository, "--assets", output, "v1.2.3"}, &stdout, &stderr); status != 0 || state.assetUploads != 2 { - t.Fatalf("physical-path retry status=%d, uploads=%d, stderr=%s", status, state.assetUploads, stderr.String()) + if status := run(context.Background(), []string{"deliver", "--repo", repository, "--assets", output, "v1.2.3"}, &stdout, &stderr); status != 0 || state.snapshot().assetUploads != 2 { + t.Fatalf("physical-path retry status=%d, uploads=%d, stderr=%s", status, state.snapshot().assetUploads, stderr.String()) } } if tt.module != "" { @@ -86,17 +86,17 @@ func TestBuildAndPrebuiltDeliveryUseSameDirectoryBase(t *testing.T) { wrongModule := settings wrongModule.ModuleDir = "" invocation := cli.Invocation{Repository: repository, AssetsDir: resolved, Version: "v1.2.3"} - requestsBefore := state.apiRequests - if status := deliverWithSettings(context.Background(), invocation, wrongModule, &stdout, &stderr); status != 1 || state.apiRequests != requestsBefore { - t.Fatalf("wrong module status=%d, requests=%d->%d, stderr=%s", status, requestsBefore, state.apiRequests, stderr.String()) + requestsBefore := state.snapshot().apiRequests + if status := deliverWithSettings(context.Background(), invocation, wrongModule, &stdout, &stderr); status != 1 || state.snapshot().apiRequests != requestsBefore { + t.Fatalf("wrong module status=%d, requests=%d->%d, stderr=%s", status, requestsBefore, state.snapshot().apiRequests, stderr.String()) } } if err := os.WriteFile(receipt.Artifacts[0].Path, []byte("changed"), 0o600); err != nil { t.Fatal(err) } - requestsBefore := state.apiRequests - if status := run(context.Background(), args, &stdout, &stderr); status != 1 || state.apiRequests != requestsBefore { - t.Fatalf("changed content status=%d, requests=%d->%d, stderr=%s", status, requestsBefore, state.apiRequests, stderr.String()) + requestsBefore := state.snapshot().apiRequests + if status := run(context.Background(), args, &stdout, &stderr); status != 1 || state.snapshot().apiRequests != requestsBefore { + t.Fatalf("changed content status=%d, requests=%d->%d, stderr=%s", status, requestsBefore, state.snapshot().apiRequests, stderr.String()) } }) } diff --git a/cmd/releasekit/committed_identity_test.go b/cmd/releasekit/committed_identity_test.go index 47e7b7c..4c0ac4f 100644 --- a/cmd/releasekit/committed_identity_test.go +++ b/cmd/releasekit/committed_identity_test.go @@ -36,15 +36,15 @@ func TestPageOnlyDeliveryUsesCommittedModuleInSparseCheckout(t *testing.T) { writeRepositoryFile(t, repository, module+"/docs/releases/v1.2.3.md", "# Example Product v1.2.3\n\nNested release note.\n") cliGitRun(t, repository, "add", ".") cliGitRun(t, repository, "commit", "-m", "Add sparse release module") - state := &deliveryServerState{tag: module + "/v1.2.3"} + state := &deliveryServerState{data: deliveryServerData{tag: module + "/v1.2.3"}} server := newDeliveryServer(t, state) defer server.Close() writeDeliveryProfile(t, repository, cli.Settings{Product: "Example Product", ModuleDir: module, Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}, }) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") - cliGitRun(t, repository, "tag", state.tag) - cliGitRun(t, repository, "checkout", "--detach", state.tag) + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) + cliGitRun(t, repository, "tag", state.snapshot().tag) + cliGitRun(t, repository, "checkout", "--detach", state.snapshot().tag) cliGitRun(t, repository, "sparse-checkout", "set", "--cone", "docs") if _, err := os.Stat(filepath.Join(repository, filepath.FromSlash(module))); !errors.Is(err, os.ErrNotExist) { t.Fatalf("module fixture is materialized: %v", err) @@ -61,11 +61,11 @@ func TestPageOnlyDeliveryUsesCommittedModuleInSparseCheckout(t *testing.T) { t.Setenv("PATH", gitOnly) for _, args := range [][]string{ {"deliver", "--repo", repository, "v1.2.3"}, - {"ci-deliver", "--repo", repository, "--prepared", "--expected-module", module, "--commit", state.commit, state.tag}, + {"ci-deliver", "--repo", repository, "--prepared", "--expected-module", module, "--commit", state.snapshot().commit, state.snapshot().tag}, } { var stdout, stderr bytes.Buffer - if status := run(context.Background(), args, &stdout, &stderr); status != 0 || state.releaseCreates != 1 || state.assetUploads != 0 { - t.Fatalf("%s sparse delivery status=%d creates=%d uploads=%d stderr=%s", args[0], status, state.releaseCreates, state.assetUploads, stderr.String()) + if status := run(context.Background(), args, &stdout, &stderr); status != 0 || state.snapshot().releaseCreates != 1 || state.snapshot().assetUploads != 0 { + t.Fatalf("%s sparse delivery status=%d creates=%d uploads=%d stderr=%s", args[0], status, state.snapshot().releaseCreates, state.snapshot().assetUploads, stderr.String()) } } } diff --git a/cmd/releasekit/local_module_test.go b/cmd/releasekit/local_module_test.go index 3f08759..4de8bc4 100644 --- a/cmd/releasekit/local_module_test.go +++ b/cmd/releasekit/local_module_test.go @@ -34,11 +34,11 @@ func TestMainModuleWithoutDomainSupportsCheckBuildAndPageDelivery(t *testing.T) } settings.Build = nil writeDeliveryProfile(t, repository, settings) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "v1.2.3") t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token") var stdout, stderr bytes.Buffer - if status := run(context.Background(), []string{"deliver", "--repo", repository, "v1.2.3"}, &stdout, &stderr); status != 0 || state.releaseCreates != 1 || state.assetUploads != 0 { - t.Fatalf("page delivery status=%d creates=%d uploads=%d stderr=%s", status, state.releaseCreates, state.assetUploads, stderr.String()) + if status := run(context.Background(), []string{"deliver", "--repo", repository, "v1.2.3"}, &stdout, &stderr); status != 0 || state.snapshot().releaseCreates != 1 || state.snapshot().assetUploads != 0 { + t.Fatalf("page delivery status=%d creates=%d uploads=%d stderr=%s", status, state.snapshot().releaseCreates, state.snapshot().assetUploads, stderr.String()) } } diff --git a/cmd/releasekit/main_test.go b/cmd/releasekit/main_test.go index ee243ef..18d38bf 100644 --- a/cmd/releasekit/main_test.go +++ b/cmd/releasekit/main_test.go @@ -15,6 +15,7 @@ import ( "path/filepath" "runtime" "strings" + "sync" "testing" releasekit "gitea.maximumdirect.net/eric/ReleaseKit" @@ -641,7 +642,7 @@ func TestDeliverPageOnlyCreatesAndReusesRelease(t *testing.T) { Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}, } writeDeliveryProfile(t, repository, settings) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "v1.2.3") // Historical lightweight tags are valid delivery inputs. t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token") for attempt := 0; attempt < 2; attempt++ { @@ -654,11 +655,11 @@ func TestDeliverPageOnlyCreatesAndReusesRelease(t *testing.T) { t.Fatalf("delivery output = %q", stdout.String()) } } - if state.releaseCreates != 1 || state.assetUploads != 0 || state.assetListReads != 0 { - t.Fatalf("page-only calls = creates:%d asset uploads:%d asset reads:%d", state.releaseCreates, state.assetUploads, state.assetListReads) + if state.snapshot().releaseCreates != 1 || state.snapshot().assetUploads != 0 || state.snapshot().assetListReads != 0 { + t.Fatalf("page-only calls = creates:%d asset uploads:%d asset reads:%d", state.snapshot().releaseCreates, state.snapshot().assetUploads, state.snapshot().assetListReads) } - if state.noteBody != "# Example Product v1.2.3\n\nRelease note.\n" { - t.Fatalf("hosted note body = %q", state.noteBody) + if state.snapshot().noteBody != "# Example Product v1.2.3\n\nRelease note.\n" { + t.Fatalf("hosted note body = %q", state.snapshot().noteBody) } } @@ -675,7 +676,7 @@ func TestDeliverBuildsTaggedArtifactsAndReusesAssets(t *testing.T) { Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}, } writeDeliveryProfile(t, repository, settings) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "v1.2.3") cliGitRun(t, repository, "checkout", "--detach", "v1.2.3") t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token") @@ -689,14 +690,15 @@ func TestDeliverBuildsTaggedArtifactsAndReusesAssets(t *testing.T) { t.Fatalf("second delivery did not report matching assets: %q", stdout.String()) } } - if state.releaseCreates != 1 || state.assetUploads != 2 { - t.Fatalf("delivery calls = creates:%d asset uploads:%d", state.releaseCreates, state.assetUploads) + observed := state.snapshot() + if observed.releaseCreates != 1 || observed.assetUploads != 2 { + t.Fatalf("delivery calls = creates:%d asset uploads:%d", observed.releaseCreates, observed.assetUploads) } - if _, ok := state.assets["helper-v1.2.3-linux-amd64"]; !ok { - t.Fatalf("binary was not uploaded: %#v", state.assets) + if _, ok := observed.assets["helper-v1.2.3-linux-amd64"]; !ok { + t.Fatalf("binary was not uploaded: %#v", observed.assets) } - if _, ok := state.assets["SHA256SUMS"]; !ok { - t.Fatalf("checksum manifest was not uploaded: %#v", state.assets) + if _, ok := observed.assets["SHA256SUMS"]; !ok { + t.Fatalf("checksum manifest was not uploaded: %#v", observed.assets) } if entries, err := os.ReadDir(temporaryRoot); err != nil || len(entries) != 0 { t.Fatalf("facade-owned build output remains after delivery: entries=%v error=%v", entries, err) @@ -717,7 +719,7 @@ func TestCIDeliverLoadsTaggedProfileAndSkipsUnavailablePageOnlyDependencies(t *t Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}, }) expectedCommit := pushTaggedCLIRepository(t, repository) - state.commit = expectedCommit + state.update(func(state *deliveryServerData) { state.commit = expectedCommit }) writeDeliveryProfileAt(t, repository, "profiles/ci-delivery.json", cli.Settings{Product: "Later Branch Profile"}) cliGitRun(t, repository, "checkout", "--detach", expectedCommit) cliGitRun(t, repository, "remote", "remove", "origin") @@ -730,8 +732,8 @@ func TestCIDeliverLoadsTaggedProfileAndSkipsUnavailablePageOnlyDependencies(t *t if status := run(context.Background(), args, &stdout, &stderr); status != 0 { t.Fatalf("run() status=%d stdout=%q stderr=%q", status, stdout.String(), stderr.String()) } - if state.releaseCreates != 1 || state.apiRequests == 0 { - t.Fatalf("page delivery did not reach the server: creates=%d requests=%d", state.releaseCreates, state.apiRequests) + if state.snapshot().releaseCreates != 1 || state.snapshot().apiRequests == 0 { + t.Fatalf("page delivery did not reach the server: creates=%d requests=%d", state.snapshot().releaseCreates, state.snapshot().apiRequests) } if got := cliGitOutput(t, repository, "rev-parse", "HEAD"); got != expectedCommit { t.Fatalf("prepared HEAD = %s, want %s", got, expectedCommit) @@ -752,7 +754,7 @@ func TestCIDeliverRejectsWrongCommitBeforeGoOrHostedRequests(t *testing.T) { Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}, }) expectedCommit := pushTaggedCLIRepository(t, repository) - state.commit = expectedCommit + state.update(func(state *deliveryServerData) { state.commit = expectedCommit }) shimDir := t.TempDir() markerPath := filepath.Join(shimDir, "go-invoked") @@ -775,8 +777,8 @@ func TestCIDeliverRejectsWrongCommitBeforeGoOrHostedRequests(t *testing.T) { if _, err := os.Stat(markerPath); !os.IsNotExist(err) { t.Fatalf("Go was invoked before commit agreement: %v", err) } - if state.apiRequests != 0 { - t.Fatalf("hosted API requests = %d, want 0", state.apiRequests) + if state.snapshot().apiRequests != 0 { + t.Fatalf("hosted API requests = %d, want 0", state.snapshot().apiRequests) } } @@ -803,7 +805,7 @@ func TestCIDeliverRejectsExpectedModuleMismatchBeforeDependencyPreparation(t *te Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}, }) expectedCommit := pushTaggedCLIRepository(t, repository) - state.commit = expectedCommit + state.update(func(state *deliveryServerData) { state.commit = expectedCommit }) shimDir := t.TempDir() markerPath := filepath.Join(shimDir, "go-invoked") goShim := "#!/bin/sh\n: > \"$RELEASEKIT_GO_MARKER\"\nexit 99\n" @@ -824,8 +826,8 @@ func TestCIDeliverRejectsExpectedModuleMismatchBeforeDependencyPreparation(t *te if _, err := os.Stat(markerPath); !os.IsNotExist(err) { t.Fatalf("Go was invoked before profile agreement: %v", err) } - if state.apiRequests != 0 { - t.Fatalf("hosted API requests = %d, want 0", state.apiRequests) + if state.snapshot().apiRequests != 0 { + t.Fatalf("hosted API requests = %d, want 0", state.snapshot().apiRequests) } } @@ -840,18 +842,18 @@ func TestCIDeliverPreparesDependenciesAndBuildsConfiguredTargets(t *testing.T) { Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}, }) expectedCommit := pushTaggedCLIRepository(t, repository) - state.commit = expectedCommit + state.update(func(state *deliveryServerData) { state.commit = expectedCommit }) t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token") var stdout, stderr bytes.Buffer args := []string{"ci-deliver", "--repo", repository, "--commit", expectedCommit, "v1.2.3"} if status := run(context.Background(), args, &stdout, &stderr); status != 0 { t.Fatalf("run() status=%d stdout=%q stderr=%q", status, stdout.String(), stderr.String()) } - if state.releaseCreates != 1 || state.assetUploads != 2 { - t.Fatalf("delivery counts = creates:%d uploads:%d", state.releaseCreates, state.assetUploads) + if state.snapshot().releaseCreates != 1 || state.snapshot().assetUploads != 2 { + t.Fatalf("delivery counts = creates:%d uploads:%d", state.snapshot().releaseCreates, state.snapshot().assetUploads) } - if _, ok := state.assets["helper-v1.2.3-"+runtime.GOOS+"-"+runtime.GOARCH]; !ok { - t.Fatalf("binary not uploaded: %#v", state.assets) + if _, ok := state.snapshot().assets["helper-v1.2.3-"+runtime.GOOS+"-"+runtime.GOARCH]; !ok { + t.Fatalf("binary not uploaded: %#v", state.snapshot().assets) } } @@ -866,7 +868,7 @@ func TestCIDeliverValidatesCredentialsBeforePreparingDependencies(t *testing.T) Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}, }) expectedCommit := pushTaggedCLIRepository(t, repository) - state.commit = expectedCommit + state.update(func(state *deliveryServerData) { state.commit = expectedCommit }) shimDir := t.TempDir() markerPath := filepath.Join(shimDir, "go-invoked") @@ -888,8 +890,8 @@ func TestCIDeliverValidatesCredentialsBeforePreparingDependencies(t *testing.T) if _, err := os.Stat(markerPath); !os.IsNotExist(err) { t.Fatalf("Go was invoked before credential validation: %v", err) } - if state.apiRequests != 0 { - t.Fatalf("hosted API requests = %d, want 0", state.apiRequests) + if state.snapshot().apiRequests != 0 { + t.Fatalf("hosted API requests = %d, want 0", state.snapshot().apiRequests) } } @@ -904,7 +906,7 @@ func TestDeliverRejectsInvalidTaggedSourceBeforeManagedBuild(t *testing.T) { Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}, } writeDeliveryProfile(t, repository, settings) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "v1.2.3") if err := os.WriteFile(filepath.Join(repository, "uncommitted.txt"), []byte("dirty"), 0600); err != nil { t.Fatal(err) @@ -912,8 +914,8 @@ func TestDeliverRejectsInvalidTaggedSourceBeforeManagedBuild(t *testing.T) { t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token") var stdout, stderr bytes.Buffer status := run(context.Background(), []string{"deliver", "--repo", repository, "v1.2.3"}, &stdout, &stderr) - if status != 1 || state.apiRequests != 0 || strings.Contains(stderr.String(), "building ") { - t.Fatalf("invalid-source delivery status=%d requests=%d stdout=%q stderr=%q", status, state.apiRequests, stdout.String(), stderr.String()) + if status != 1 || state.snapshot().apiRequests != 0 || strings.Contains(stderr.String(), "building ") { + t.Fatalf("invalid-source delivery status=%d requests=%d stdout=%q stderr=%q", status, state.snapshot().apiRequests, stdout.String(), stderr.String()) } } @@ -958,16 +960,18 @@ func TestDeliverFailureReportsRecoveryForPartialConflictAndUnknownStates(t *test Build: &build, Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}, }) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "v1.2.3") assetsDir, _ := writeDeliveryReceipt(t, repository, build) switch test.name { case "partial": - state.failAssetName = "helper-v1.2.3-linux-amd64" + state.update(func(state *deliveryServerData) { state.failAssetName = "helper-v1.2.3-linux-amd64" }) case "conflict": - state.assets["helper-v1.2.3-linux-amd64"] = []byte("different hosted bytes") + state.update(func(state *deliveryServerData) { + state.assets["helper-v1.2.3-linux-amd64"] = []byte("different hosted bytes") + }) case "unknown": - state.ambiguousAssetName = "helper-v1.2.3-linux-amd64" + state.update(func(state *deliveryServerData) { state.ambiguousAssetName = "helper-v1.2.3-linux-amd64" }) } t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token") var stdout, stderr bytes.Buffer @@ -995,17 +999,17 @@ func TestDeliverFailureReportsRecoveryForPartialConflictAndUnknownStates(t *test func TestDeliverRejectsWrongHostedTagBeforeReleaseWrite(t *testing.T) { repository := newCLIRepository(t) - state := &deliveryServerState{remoteCommit: strings.Repeat("0", 40)} + state := &deliveryServerState{data: deliveryServerData{remoteCommit: strings.Repeat("0", 40)}} server := newDeliveryServer(t, state) defer server.Close() writeDeliveryProfile(t, repository, cli.Settings{Product: "Example Product", Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}}) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "v1.2.3") t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token") var stdout, stderr bytes.Buffer status := run(context.Background(), []string{"deliver", "--repo", repository, "v1.2.3"}, &stdout, &stderr) - if status != 1 || state.releaseCreates != 0 || state.assetUploads != 0 { - t.Fatalf("wrong hosted tag status=%d creates=%d uploads=%d stdout=%q stderr=%q", status, state.releaseCreates, state.assetUploads, stdout.String(), stderr.String()) + if status != 1 || state.snapshot().releaseCreates != 0 || state.snapshot().assetUploads != 0 { + t.Fatalf("wrong hosted tag status=%d creates=%d uploads=%d stdout=%q stderr=%q", status, state.snapshot().releaseCreates, state.snapshot().assetUploads, stdout.String(), stderr.String()) } } @@ -1015,13 +1019,13 @@ func TestDeliverRequiresCredentialBeforeRemoteRequests(t *testing.T) { server := newDeliveryServer(t, state) defer server.Close() writeDeliveryProfile(t, repository, cli.Settings{Product: "Example Product", Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example", TokenEnv: "RELEASEKIT_TEST_DELIVERY_TOKEN"}}) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "v1.2.3") t.Setenv("RELEASEKIT_TEST_DELIVERY_TOKEN", "") var stdout, stderr bytes.Buffer status := run(context.Background(), []string{"deliver", "--repo", repository, "v1.2.3"}, &stdout, &stderr) - if status != 2 || state.apiRequests != 0 || !strings.Contains(stderr.String(), "RELEASEKIT_TEST_DELIVERY_TOKEN") { - t.Fatalf("missing credential status=%d requests=%d stdout=%q stderr=%q", status, state.apiRequests, stdout.String(), stderr.String()) + if status != 2 || state.snapshot().apiRequests != 0 || !strings.Contains(stderr.String(), "RELEASEKIT_TEST_DELIVERY_TOKEN") { + t.Fatalf("missing credential status=%d requests=%d stdout=%q stderr=%q", status, state.snapshot().apiRequests, stdout.String(), stderr.String()) } } @@ -1031,7 +1035,7 @@ func TestDeliverRejectsCheckoutThatDiffersFromLocalTag(t *testing.T) { server := newDeliveryServer(t, state) defer server.Close() writeDeliveryProfile(t, repository, cli.Settings{Product: "Example Product", Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}}) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "v1.2.3") if err := os.WriteFile(filepath.Join(repository, "later.txt"), []byte("later commit"), 0o600); err != nil { t.Fatal(err) @@ -1041,11 +1045,11 @@ func TestDeliverRejectsCheckoutThatDiffersFromLocalTag(t *testing.T) { t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token") var stdout, stderr bytes.Buffer status := run(context.Background(), []string{"deliver", "--repo", repository, "v1.2.3"}, &stdout, &stderr) - if status != 1 || state.apiRequests != 0 || state.releaseCreates != 0 { - t.Fatalf("mismatched checkout status=%d requests=%d creates=%d stdout=%q stderr=%q", status, state.apiRequests, state.releaseCreates, stdout.String(), stderr.String()) + if status != 1 || state.snapshot().apiRequests != 0 || state.snapshot().releaseCreates != 0 { + t.Fatalf("mismatched checkout status=%d requests=%d creates=%d stdout=%q stderr=%q", status, state.snapshot().apiRequests, state.snapshot().releaseCreates, stdout.String(), stderr.String()) } - if got := cliGitOutput(t, repository, "rev-parse", "refs/tags/v1.2.3"); got != state.commit { - t.Fatalf("delivery moved tag from %s to %s", state.commit, got) + if got := cliGitOutput(t, repository, "rev-parse", "refs/tags/v1.2.3"); got != state.snapshot().commit { + t.Fatalf("delivery moved tag from %s to %s", state.snapshot().commit, got) } } @@ -1059,7 +1063,7 @@ func TestDeliverRejectsTamperedAssetsAndStaleReceiptBeforeRemoteWrites(t *testin current := cli.BuildSettings{MainPackage: ".", BinaryName: "helper", Targets: []string{"linux/amd64"}} settings := cli.Settings{Product: "Example Product", Build: ¤t, Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}} writeDeliveryProfile(t, repository, settings) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "v1.2.3") receiptSettings := current if mode == "stale settings" { @@ -1085,16 +1089,16 @@ func TestDeliverRejectsTamperedAssetsAndStaleReceiptBeforeRemoteWrites(t *testin var stdout, stderr bytes.Buffer status := run(context.Background(), []string{"deliver", "--repo", repository, "--assets", assetsDir, "v1.2.3"}, &stdout, &stderr) if mode == "valid receipt" { - if status != 0 || state.releaseCreates != 1 || state.assetUploads != 2 { - t.Fatalf("valid receipt status=%d creates=%d uploads=%d stdout=%q stderr=%q", status, state.releaseCreates, state.assetUploads, stdout.String(), stderr.String()) + if status != 0 || state.snapshot().releaseCreates != 1 || state.snapshot().assetUploads != 2 { + t.Fatalf("valid receipt status=%d creates=%d uploads=%d stdout=%q stderr=%q", status, state.snapshot().releaseCreates, state.snapshot().assetUploads, stdout.String(), stderr.String()) } if _, err := os.Stat(artifactPath); err != nil { t.Fatalf("prebuilt artifact was not retained: %v", err) } return } - if status != 1 || state.apiRequests != 0 || state.releaseCreates != 0 || state.assetUploads != 0 { - t.Fatalf("%s receipt status=%d requests=%d stdout=%q stderr=%q", mode, status, state.apiRequests, stdout.String(), stderr.String()) + if status != 1 || state.snapshot().apiRequests != 0 || state.snapshot().releaseCreates != 0 || state.snapshot().assetUploads != 0 { + t.Fatalf("%s receipt status=%d requests=%d stdout=%q stderr=%q", mode, status, state.snapshot().apiRequests, stdout.String(), stderr.String()) } if mode == "tampered" || mode == "missing artifact" { if strings.Contains(stderr.String(), "load verified build receipt") { @@ -1136,6 +1140,30 @@ func writeDeliveryReceipt(t *testing.T, repository string, settings cli.BuildSet } type deliveryServerState struct { + mu sync.Mutex + data deliveryServerData +} + +func (state *deliveryServerState) update(change func(*deliveryServerData)) { + state.mu.Lock() + defer state.mu.Unlock() + change(&state.data) +} + +func (state *deliveryServerState) snapshot() deliveryServerData { + state.mu.Lock() + defer state.mu.Unlock() + result := state.data + + result.assets = make(map[string][]byte, len(state.data.assets)) + for key, value := range state.data.assets { + result.assets[key] = append([]byte(nil), value...) + } + + return result +} + +type deliveryServerData struct { tag string malformedCreate bool failAssetListing bool @@ -1156,36 +1184,38 @@ type deliveryServerState struct { func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Server { t.Helper() - if state.tag == "" { - state.tag = "v1.2.3" + data := &state.data + if data.tag == "" { + data.tag = "v1.2.3" } - if state.assets == nil { - state.assets = make(map[string][]byte) + if data.assets == nil { + data.assets = make(map[string][]byte) } - serverURL := "" handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + state.mu.Lock() + defer state.mu.Unlock() if strings.HasPrefix(r.URL.Path, "/api/") { - state.apiRequests++ + data.apiRequests++ if r.Header.Get("Authorization") != "token fixture-token" { t.Errorf("authorization = %q", r.Header.Get("Authorization")) } } switch r.URL.Path { - case "/api/v1/repos/owner/example/tags/" + state.tag: - if state.commit == "" { + case "/api/v1/repos/owner/example/tags/" + data.tag: + if data.commit == "" { t.Error("tag source commit was not initialized") } - commit := state.remoteCommit + commit := data.remoteCommit if commit == "" { - commit = state.commit + commit = data.commit } - _, _ = fmt.Fprintf(w, `{"name":%q,"commit":{"sha":%q}}`, state.tag, commit) - case "/api/v1/repos/owner/example/releases/tags/" + state.tag: - if !state.pageExists { + _, _ = fmt.Fprintf(w, `{"name":%q,"commit":{"sha":%q}}`, data.tag, commit) + case "/api/v1/repos/owner/example/releases/tags/" + data.tag: + if !data.pageExists { http.NotFound(w, r) return } - writeDeliveryRelease(w, serverURL, state) + writeDeliveryRelease(w, serverURL(r), data) case "/api/v1/repos/owner/example/releases": if r.Method != http.MethodPost { t.Errorf("release request method = %s", r.Method) @@ -1201,26 +1231,26 @@ func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Serve if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { t.Errorf("decode create payload: %v", err) } - if payload.Tag != state.tag || payload.Target != state.commit { + if payload.Tag != data.tag || payload.Target != data.commit { t.Errorf("release source payload = %#v", payload) } - state.pageExists, state.releaseCreates, state.noteBody = true, state.releaseCreates+1, payload.Body - if state.malformedCreate { + data.pageExists, data.releaseCreates, data.noteBody = true, data.releaseCreates+1, payload.Body + if data.malformedCreate { w.WriteHeader(http.StatusCreated) - _ = json.NewEncoder(w).Encode(map[string]any{"id": 10, "tag_name": state.tag, "name": "inconsistent response", "body": payload.Body}) + _ = json.NewEncoder(w).Encode(map[string]any{"id": 10, "tag_name": data.tag, "name": "inconsistent response", "body": payload.Body}) return } - writeDeliveryRelease(w, serverURL, state) + writeDeliveryRelease(w, serverURL(r), data) case "/api/v1/repos/owner/example/releases/10/assets": if r.Method == http.MethodGet { - state.assetListReads++ - if state.failAssetListing { + data.assetListReads++ + if data.failAssetListing { http.Error(w, "unavailable", 503) return } - list := make([]map[string]any, 0, len(state.assets)) - for name, content := range state.assets { - list = append(list, map[string]any{"id": len(list) + 1, "name": name, "size": len(content), "browser_download_url": serverURL + "/files/" + name}) + list := make([]map[string]any, 0, len(data.assets)) + for name, content := range data.assets { + list = append(list, map[string]any{"id": len(list) + 1, "name": name, "size": len(content), "browser_download_url": serverURL(r) + "/files/" + name}) } _ = json.NewEncoder(w).Encode(list) return @@ -1242,28 +1272,28 @@ func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Serve if err != nil { t.Errorf("read asset bytes: %v", err) } - if _, exists := state.assets[name]; exists { + if _, exists := data.assets[name]; exists { t.Errorf("asset %q uploaded more than once", name) } - if name == state.failAssetName { + if name == data.failAssetName { http.Error(w, "upload failed", http.StatusBadRequest) return } - if name == state.ambiguousAssetName { + if name == data.ambiguousAssetName { http.Error(w, "upload outcome ambiguous", http.StatusInternalServerError) return } - state.assets[name] = content - state.assetUploads++ - _ = json.NewEncoder(w).Encode(map[string]any{"id": state.assetUploads, "name": name, "size": len(content)}) + data.assets[name] = content + data.assetUploads++ + _ = json.NewEncoder(w).Encode(map[string]any{"id": data.assetUploads, "name": name, "size": len(content)}) default: if strings.HasPrefix(r.URL.Path, "/files/") { - if state.failAssetDownloads { + if data.failAssetDownloads { http.Error(w, "unavailable", 503) return } name := strings.TrimPrefix(r.URL.Path, "/files/") - content, ok := state.assets[name] + content, ok := data.assets[name] if !ok { http.NotFound(w, r) return @@ -1276,17 +1306,17 @@ func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Serve } }) server := httptest.NewServer(handler) - serverURL = server.URL return server } -func writeDeliveryRelease(w http.ResponseWriter, serverURL string, state *deliveryServerState) { +// writeDeliveryRelease reads data while the request handler holds state.mu. +func writeDeliveryRelease(w http.ResponseWriter, baseURL string, data *deliveryServerData) { w.Header().Set("Content-Type", "application/json") - product := state.product + product := data.product if product == "" { product = "Example Product" } - _ = json.NewEncoder(w).Encode(map[string]any{"id": 10, "html_url": serverURL + "/releases/10", "tag_name": state.tag, "target_commitish": state.commit, "name": product + " v1.2.3", "body": state.noteBody, "draft": false, "prerelease": false}) + _ = json.NewEncoder(w).Encode(map[string]any{"id": 10, "html_url": baseURL + "/releases/10", "tag_name": data.tag, "target_commitish": data.commit, "name": product + " v1.2.3", "body": data.noteBody, "draft": false, "prerelease": false}) } func writeDeliveryProfile(t *testing.T, repository string, settings cli.Settings) { @@ -1417,7 +1447,7 @@ func TestDeliverRejectsReceiptBuiltFromDirtySourceAfterCleanup(t *testing.T) { defer server.Close() settings := cli.Settings{Product: "Example Product", Build: &cli.BuildSettings{MainPackage: ".", BinaryName: "helper", Targets: []string{"linux/amd64"}}, Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}} writeDeliveryProfile(t, repository, settings) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "v1.2.3") mainPath := filepath.Join(repository, "main.go") original, err := os.ReadFile(mainPath) @@ -1444,26 +1474,26 @@ func TestDeliverRejectsReceiptBuiltFromDirtySourceAfterCleanup(t *testing.T) { stdout.Reset() stderr.Reset() status := run(context.Background(), []string{"deliver", "--repo", repository, "--assets", assets, "v1.2.3"}, &stdout, &stderr) - if status != 1 || state.apiRequests != 0 || !strings.Contains(stderr.String(), "build source") { - t.Fatalf("status=%d requests=%d %s", status, state.apiRequests, stderr.String()) + if status != 1 || state.snapshot().apiRequests != 0 || !strings.Contains(stderr.String(), "build source") { + t.Fatalf("status=%d requests=%d %s", status, state.snapshot().apiRequests, stderr.String()) } } func TestRecoveredPageCreatePreservesPartialDeliveryOnInspectionFailure(t *testing.T) { repository := newCLIRepository(t) - state := &deliveryServerState{malformedCreate: true, failAssetListing: true} + state := &deliveryServerState{data: deliveryServerData{malformedCreate: true, failAssetListing: true}} server := newDeliveryServer(t, state) defer server.Close() build := cli.BuildSettings{MainPackage: ".", BinaryName: "helper", Targets: []string{"linux/amd64"}} writeDeliveryProfile(t, repository, cli.Settings{Product: "Example Product", Build: &build, Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}}) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "v1.2.3") assets, _ := writeDeliveryReceipt(t, repository, build) t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token") var stdout, stderr bytes.Buffer status := run(context.Background(), []string{"deliver", "--repo", repository, "--assets", assets, "v1.2.3"}, &stdout, &stderr) - if status != 1 || !strings.Contains(stderr.String(), "state=partial") || state.releaseCreates != 1 || state.assetListReads != 1 || state.assetUploads != 0 { - t.Fatalf("status=%d state=%#v %s", status, state, stderr.String()) + if status != 1 || !strings.Contains(stderr.String(), "state=partial") || state.snapshot().releaseCreates != 1 || state.snapshot().assetListReads != 1 || state.snapshot().assetUploads != 0 { + t.Fatalf("status=%d state=%#v %s", status, state.snapshot(), stderr.String()) } } @@ -1471,12 +1501,12 @@ func TestUnreadableAssetsDoNotClaimAnUnknownWrite(t *testing.T) { for _, existingPage := range []bool{false, true} { t.Run(fmt.Sprintf("existing-page-%t", existingPage), func(t *testing.T) { repository := newCLIRepository(t) - state := &deliveryServerState{pageExists: existingPage, noteBody: "# Example Product v1.2.3\n\nRelease note.\n", failAssetDownloads: true, assets: map[string][]byte{"helper-v1.2.3-linux-amd64": []byte("fixture artifact")}} + state := &deliveryServerState{data: deliveryServerData{pageExists: existingPage, noteBody: "# Example Product v1.2.3\n\nRelease note.\n", failAssetDownloads: true, assets: map[string][]byte{"helper-v1.2.3-linux-amd64": []byte("fixture artifact")}}} server := newDeliveryServer(t, state) defer server.Close() build := cli.BuildSettings{MainPackage: ".", BinaryName: "helper", Targets: []string{"linux/amd64"}} writeDeliveryProfile(t, repository, cli.Settings{Product: "Example Product", Build: &build, Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}}) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "v1.2.3") assets, _ := writeDeliveryReceipt(t, repository, build) t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token") @@ -1488,8 +1518,8 @@ func TestUnreadableAssetsDoNotClaimAnUnknownWrite(t *testing.T) { expectedState = "not_attempted" expectedCreates = 0 } - if status != 1 || !strings.Contains(stderr.String(), "state="+expectedState) || state.assetUploads != 0 || state.releaseCreates != expectedCreates { - t.Fatalf("status=%d creates=%d uploads=%d %s", status, state.releaseCreates, state.assetUploads, stderr.String()) + if status != 1 || !strings.Contains(stderr.String(), "state="+expectedState) || state.snapshot().assetUploads != 0 || state.snapshot().releaseCreates != expectedCreates { + t.Fatalf("status=%d creates=%d uploads=%d %s", status, state.snapshot().releaseCreates, state.snapshot().assetUploads, stderr.String()) } if existingPage && !strings.Contains(stderr.String(), "recovery: fix the local or preflight failure") { t.Fatalf("not-attempted recovery guidance missing: %s", stderr.String()) @@ -1532,7 +1562,7 @@ func TestTemporaryDeliveryBuildDoesNotRequireWritableRepositoryParent(t *testing server := newDeliveryServer(t, state) defer server.Close() writeDeliveryProfile(t, repository, cli.Settings{Product: "Example Product", Build: &cli.BuildSettings{MainPackage: ".", BinaryName: "helper", Targets: []string{"linux/amd64"}}, Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}}) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "v1.2.3") parent := t.TempDir() moved := filepath.Join(parent, "repo") diff --git a/cmd/releasekit/multi_module_woodpecker_test.go b/cmd/releasekit/multi_module_woodpecker_test.go index 43ff222..b8f57db 100644 --- a/cmd/releasekit/multi_module_woodpecker_test.go +++ b/cmd/releasekit/multi_module_woodpecker_test.go @@ -12,6 +12,7 @@ import ( "os/exec" "path/filepath" "strings" + "sync" "testing" "gitea.maximumdirect.net/eric/ReleaseKit/cmd/releasekit/internal/cli" @@ -43,18 +44,18 @@ func TestMultiModuleWoodpeckerDelivery(t *testing.T) { repository := newWorkflowRepository(t, example) writeMappedProfile(t, repository, "releasekit-ckconfig.json", cli.Settings{ Product: "Example Consumer Integration", ModuleDir: "integrations/ckconfig", - Delivery: workflowDeliverySettings(delivery.URL), + Delivery: workflowDeliverySettings(delivery.snapshot().URL), }) writeRepositoryFile(t, repository, "integrations/ckconfig/docs/releases/v1.2.3.md", "# Example Consumer Integration v1.2.3\n\nTagged integration note.\n") writeRepositoryFile(t, repository, "integrations/ckconfig/go.mod", workflowModuleFile("example.com/multi-module-consumer/integrations/ckconfig", "v0.2.197")) commit := tagAndPushWorkflowRepository(t, repository, tag) - delivery.commit = commit + delivery.update(func(delivery *mappedDeliveryData) { delivery.commit = commit }) env := multiModuleWorkflowEnvironment(t, repository, commit, tag, toolBinary, proxy) if output := runWoodpeckerBlock(t, command, repository, env); !strings.Contains(output, "state complete") || !strings.Contains(output, "tool-version=v0.2.197") { t.Fatalf("tagged page delivery output = %q", output) } - if delivery.creates != 1 || delivery.requests == 0 || delivery.tag != tag { - t.Fatalf("page delivery fixture = %#v", delivery) + if delivery.snapshot().creates != 1 || delivery.snapshot().requests == 0 || delivery.snapshot().tag != tag { + t.Fatalf("page delivery fixture = %#v", delivery.snapshot()) } cliGitRun(t, repository, "checkout", "main") @@ -68,8 +69,8 @@ func TestMultiModuleWoodpeckerDelivery(t *testing.T) { if !strings.Contains(output, "tool-version=v0.2.197") || strings.Contains(output, "tool-version=v0.2.198") { t.Fatalf("tag retry did not use its committed tool version: %q", output) } - if delivery.creates != 1 || delivery.tag != tag { - t.Fatalf("tag retry did not reuse the tagged mapping: %#v", delivery) + if delivery.snapshot().creates != 1 || delivery.snapshot().tag != tag { + t.Fatalf("tag retry did not reuse the tagged mapping: %#v", delivery.snapshot()) } }) @@ -80,25 +81,26 @@ func TestMultiModuleWoodpeckerDelivery(t *testing.T) { writeMappedProfile(t, repository, "releasekit-core-v2.json", cli.Settings{ Product: "Example Consumer Core v2", ModuleDir: "integrations/core/v2", Build: &cli.BuildSettings{MainPackage: ".", BinaryName: "core", Targets: []string{"linux/amd64"}}, - Delivery: workflowDeliverySettings(delivery.URL), + Delivery: workflowDeliverySettings(delivery.snapshot().URL), }) writeRepositoryFile(t, repository, "integrations/core/v2/docs/releases/v2.1.0.md", "# Example Consumer Core v2 v2.1.0\n\nTagged core note.\n") prepareWorkflowModuleSum(t, repository, "integrations/core/v2", proxy) commit := tagAndPushWorkflowRepository(t, repository, tag) - delivery.commit = commit + delivery.update(func(delivery *mappedDeliveryData) { delivery.commit = commit }) env := multiModuleWorkflowEnvironment(t, repository, commit, tag, toolBinary, proxy) output := runWoodpeckerBlock(t, command, repository, env) if !strings.Contains(output, "tool-version=v0.2.197") || !strings.Contains(output, "state complete") { t.Fatalf("binary delivery output = %q", output) } - if delivery.creates != 1 || delivery.uploads != 2 || delivery.tag != tag { - t.Fatalf("binary delivery fixture = %#v", delivery) + observed := delivery.snapshot() + if observed.creates != 1 || observed.uploads != 2 || observed.tag != tag { + t.Fatalf("binary delivery fixture = %#v", observed) } - if _, ok := delivery.assets["core-v2.1.0-linux-amd64"]; !ok { - t.Fatalf("binary asset missing: %#v", delivery.assets) + if _, ok := observed.assets["core-v2.1.0-linux-amd64"]; !ok { + t.Fatalf("binary asset missing: %#v", observed.assets) } - if _, ok := delivery.assets["SHA256SUMS"]; !ok { - t.Fatalf("checksum asset missing: %#v", delivery.assets) + if _, ok := observed.assets["SHA256SUMS"]; !ok { + t.Fatalf("checksum asset missing: %#v", observed.assets) } }) @@ -109,21 +111,21 @@ func TestMultiModuleWoodpeckerDelivery(t *testing.T) { writeRepositoryFile(t, repository, "release-modules.tsv", strings.ReplaceAll(readWorkflowFile(t, repository, "release-modules.tsv"), "integrations/core/v2\treleasekit-core-v2.json", "integrations/core\treleasekit-core-v2.json")) writeMappedProfile(t, repository, "releasekit-core-v2.json", cli.Settings{ Product: "Example Consumer Core v2", ModuleDir: "integrations/core", - Delivery: workflowDeliverySettings(delivery.URL), + Delivery: workflowDeliverySettings(delivery.snapshot().URL), }) writeRepositoryFile(t, repository, "integrations/core/go.mod", workflowModuleFile("example.com/multi-module-consumer/integrations/core/v2", "v0.2.197")) writeRepositoryFile(t, repository, "integrations/core/main.go", readWorkflowFile(t, example, "integrations/core/v2/main.go")) writeRepositoryFile(t, repository, "integrations/core/docs/releases/v2.1.0.md", "# Example Consumer Core v2 v2.1.0\n\nTagged core note.\n") prepareWorkflowModuleSum(t, repository, "integrations/core", proxy) commit := tagAndPushWorkflowRepository(t, repository, tag) - delivery.commit = commit + delivery.update(func(delivery *mappedDeliveryData) { delivery.commit = commit }) env := multiModuleWorkflowEnvironment(t, repository, commit, tag, toolBinary, proxy) output := runWoodpeckerBlock(t, command, repository, env) if !strings.Contains(output, "tool-version=v0.2.197") || !strings.Contains(output, "state complete") { t.Fatalf("virtual-major delivery output = %q", output) } - if delivery.creates != 1 || delivery.tag != tag { - t.Fatalf("virtual-major delivery fixture = %#v", delivery) + if delivery.snapshot().creates != 1 || delivery.snapshot().tag != tag { + t.Fatalf("virtual-major delivery fixture = %#v", delivery.snapshot()) } }) @@ -138,24 +140,48 @@ func TestMultiModuleWoodpeckerDelivery(t *testing.T) { writeRepositoryFile(t, repository, "release-modules.tsv", mapping) writeMappedProfile(t, repository, "releasekit-core-v2.json", cli.Settings{ Product: "Example Consumer Core v2", ModuleDir: "integrations/core/v2", - Delivery: workflowDeliverySettings(delivery.URL), + Delivery: workflowDeliverySettings(delivery.snapshot().URL), }) writeRepositoryFile(t, repository, "integrations/core/v2/docs/releases/v2.1.0.md", "# Example Consumer Core v2 v2.1.0\n\nMismatched mapped note.\n") commit := tagAndPushWorkflowRepository(t, repository, tag) - delivery.commit = commit + delivery.update(func(delivery *mappedDeliveryData) { delivery.commit = commit }) env := multiModuleWorkflowEnvironment(t, repository, commit, tag, toolBinary, proxy) if output, err := executeWoodpeckerBlock(command, repository, env); err == nil { t.Fatalf("inconsistent mapping succeeded: %s", output) } else if !strings.Contains(output, tag) || !strings.Contains(output, "integrations/core/v2.1.0") { t.Fatalf("inconsistent mapping did not fail the exact tag comparison: %s", output) } - if delivery.requests != 0 { - t.Fatalf("exact event tag mismatch reached hosted delivery: %#v", delivery) + if delivery.snapshot().requests != 0 { + t.Fatalf("exact event tag mismatch reached hosted delivery: %#v", delivery.snapshot()) } }) } type mappedDeliveryFixture struct { + mu sync.Mutex + data mappedDeliveryData +} + +func (state *mappedDeliveryFixture) update(change func(*mappedDeliveryData)) { + state.mu.Lock() + defer state.mu.Unlock() + change(&state.data) +} + +func (state *mappedDeliveryFixture) snapshot() mappedDeliveryData { + state.mu.Lock() + defer state.mu.Unlock() + result := state.data + + result.assets = make(map[string][]byte, len(state.data.assets)) + for key, value := range state.data.assets { + result.assets[key] = append([]byte(nil), value...) + } + + return result +} + +type mappedDeliveryData struct { URL string tag string commit string @@ -170,9 +196,12 @@ type mappedDeliveryFixture struct { func newMappedDeliveryFixture(t *testing.T, tag string) *mappedDeliveryFixture { t.Helper() - state := &mappedDeliveryFixture{tag: tag, assets: make(map[string][]byte)} - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - state.requests++ + state := &mappedDeliveryFixture{data: mappedDeliveryData{tag: tag, assets: make(map[string][]byte)}} + data := &state.data + server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + state.mu.Lock() + defer state.mu.Unlock() + data.requests++ if r.Header.Get("Authorization") != "token fixture-token" { t.Errorf("delivery authorization = %q", r.Header.Get("Authorization")) } @@ -180,13 +209,13 @@ func newMappedDeliveryFixture(t *testing.T, tag string) *mappedDeliveryFixture { escapedTag := url.PathEscape(tag) switch r.URL.EscapedPath() { case apiRoot + "tags/" + escapedTag: - _ = json.NewEncoder(w).Encode(map[string]any{"name": tag, "commit": map[string]string{"sha": state.commit}}) + _ = json.NewEncoder(w).Encode(map[string]any{"name": tag, "commit": map[string]string{"sha": data.commit}}) case apiRoot + "releases/tags/" + escapedTag: - if !state.page { + if !data.page { http.NotFound(w, r) return } - _ = json.NewEncoder(w).Encode(map[string]any{"id": 10, "html_url": serverURL(r) + "/releases/10", "tag_name": tag, "target_commitish": state.commit, "name": state.name, "body": state.body, "draft": false, "prerelease": false}) + _ = json.NewEncoder(w).Encode(map[string]any{"id": 10, "html_url": serverURL(r) + "/releases/10", "tag_name": tag, "target_commitish": data.commit, "name": data.name, "body": data.body, "draft": false, "prerelease": false}) case apiRoot + "releases": var payload struct { Tag string `json:"tag_name"` @@ -197,18 +226,18 @@ func newMappedDeliveryFixture(t *testing.T, tag string) *mappedDeliveryFixture { if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { t.Errorf("decode release create: %v", err) } - if payload.Tag != tag || payload.Commit != state.commit { + if payload.Tag != tag || payload.Commit != data.commit { t.Errorf("release payload = %#v", payload) } - state.page = true - state.name = payload.Name - state.body = payload.Body - state.creates++ - _ = json.NewEncoder(w).Encode(map[string]any{"id": 10, "html_url": serverURL(r) + "/releases/10", "tag_name": tag, "target_commitish": state.commit, "name": payload.Name, "body": payload.Body, "draft": false, "prerelease": false}) + data.page = true + data.name = payload.Name + data.body = payload.Body + data.creates++ + _ = json.NewEncoder(w).Encode(map[string]any{"id": 10, "html_url": serverURL(r) + "/releases/10", "tag_name": tag, "target_commitish": data.commit, "name": payload.Name, "body": payload.Body, "draft": false, "prerelease": false}) case apiRoot + "releases/10/assets": if r.Method == http.MethodGet { - listing := make([]map[string]any, 0, len(state.assets)) - for name, content := range state.assets { + listing := make([]map[string]any, 0, len(data.assets)) + for name, content := range data.assets { listing = append(listing, map[string]any{"id": len(listing) + 1, "name": name, "size": len(content), "browser_download_url": serverURL(r) + "/files/" + name}) } _ = json.NewEncoder(w).Encode(listing) @@ -231,13 +260,13 @@ func newMappedDeliveryFixture(t *testing.T, tag string) *mappedDeliveryFixture { if err != nil { t.Errorf("read asset bytes: %v", err) } - state.assets[name] = content - state.uploads++ - _ = json.NewEncoder(w).Encode(map[string]any{"id": state.uploads, "name": name, "size": len(content)}) + data.assets[name] = content + data.uploads++ + _ = json.NewEncoder(w).Encode(map[string]any{"id": data.uploads, "name": name, "size": len(content)}) default: if strings.HasPrefix(r.URL.Path, "/files/") { name := strings.TrimPrefix(r.URL.Path, "/files/") - if content, ok := state.assets[name]; ok { + if content, ok := data.assets[name]; ok { _, _ = w.Write(content) return } @@ -248,8 +277,9 @@ func newMappedDeliveryFixture(t *testing.T, tag string) *mappedDeliveryFixture { http.NotFound(w, r) } })) + data.URL = "http://" + server.Listener.Addr().String() + server.Start() t.Cleanup(server.Close) - state.URL = server.URL return state } diff --git a/cmd/releasekit/releasekit_delivery_workflow_test.go b/cmd/releasekit/releasekit_delivery_workflow_test.go index be0bdb5..9410845 100644 --- a/cmd/releasekit/releasekit_delivery_workflow_test.go +++ b/cmd/releasekit/releasekit_delivery_workflow_test.go @@ -50,14 +50,14 @@ func TestReleaseKitSourceDeliveryWorkflowFromTaggedCheckout(t *testing.T) { cliGitRun(t, repository, "add", "docs/releases/"+version+".md") cliGitRun(t, repository, "commit", "-m", "prepare source-only delivery") - state := &deliveryServerState{product: "ReleaseKit"} + state := &deliveryServerState{data: deliveryServerData{product: "ReleaseKit"}} server := newDeliveryServer(t, state) defer server.Close() writeDeliveryProfile(t, repository, cli.Settings{ Product: "ReleaseKit", Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}, }) - state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + state.update(func(state *deliveryServerData) { state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") }) cliGitRun(t, repository, "tag", "-a", version, "-m", "ReleaseKit v1.2.3") remote := filepath.Join(t.TempDir(), "origin.git") @@ -78,7 +78,7 @@ func TestReleaseKitSourceDeliveryWorkflowFromTaggedCheckout(t *testing.T) { } env := map[string]string{ "CI_WORKSPACE": repository, - "CI_COMMIT_SHA": state.commit, + "CI_COMMIT_SHA": state.snapshot().commit, "CI_COMMIT_TAG": version, "RELEASEKIT_GITEA_TOKEN": "fixture-token", "GOTOOLCHAIN": "local", @@ -87,14 +87,14 @@ func TestReleaseKitSourceDeliveryWorkflowFromTaggedCheckout(t *testing.T) { } command := strings.ReplaceAll(block, "$${", "${") - state.remoteCommit = strings.Repeat("f", 40) + state.update(func(state *deliveryServerData) { state.remoteCommit = strings.Repeat("f", 40) }) if output, err := executeWoodpeckerBlock(command, repository, env); err == nil { t.Fatalf("delivery with a mismatched hosted tag succeeded: %s", output) } - if state.releaseCreates != 0 || cliGitOutput(t, repository, "rev-parse", "refs/tags/"+version+"^{commit}") != state.commit { + if state.snapshot().releaseCreates != 0 || cliGitOutput(t, repository, "rev-parse", "refs/tags/"+version+"^{commit}") != state.snapshot().commit { t.Fatal("delivery failure changed hosted release state or source tag") } - state.remoteCommit = "" + state.update(func(state *deliveryServerData) { state.remoteCommit = "" }) for attempt := 0; attempt < 2; attempt++ { output := runWoodpeckerBlock(t, command, repository, env) @@ -102,14 +102,14 @@ func TestReleaseKitSourceDeliveryWorkflowFromTaggedCheckout(t *testing.T) { t.Fatalf("delivery attempt %d output = %q", attempt+1, output) } } - if state.releaseCreates != 1 || state.assetUploads != 0 || state.assetListReads != 0 { - t.Fatalf("page-only workflow calls = creates:%d uploads:%d asset reads:%d", state.releaseCreates, state.assetUploads, state.assetListReads) + if state.snapshot().releaseCreates != 1 || state.snapshot().assetUploads != 0 || state.snapshot().assetListReads != 0 { + t.Fatalf("page-only workflow calls = creates:%d uploads:%d asset reads:%d", state.snapshot().releaseCreates, state.snapshot().assetUploads, state.snapshot().assetListReads) } - if state.noteBody != note { - t.Fatalf("hosted note body = %q, want complete note %q", state.noteBody, note) + if state.snapshot().noteBody != note { + t.Fatalf("hosted note body = %q, want complete note %q", state.snapshot().noteBody, note) } - if got := cliGitOutput(t, repository, "rev-parse", "refs/tags/"+version+"^{commit}"); got != state.commit { - t.Fatalf("source tag moved after delivery: %s, want %s", got, state.commit) + if got := cliGitOutput(t, repository, "rev-parse", "refs/tags/"+version+"^{commit}"); got != state.snapshot().commit { + t.Fatalf("source tag moved after delivery: %s, want %s", got, state.snapshot().commit) } if status := cliGitOutput(t, repository, "status", "--porcelain"); status != "" { t.Fatalf("tagged checkout became dirty after delivery: %s", status) diff --git a/cmd/releasekit/woodpecker_test.go b/cmd/releasekit/woodpecker_test.go index 47dc3cc..9a9fb04 100644 --- a/cmd/releasekit/woodpecker_test.go +++ b/cmd/releasekit/woodpecker_test.go @@ -42,7 +42,7 @@ func TestWoodpeckerTagDeliveryTemplate(t *testing.T) { Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}, }) commit := pushTaggedCLIRepository(t, repository) - state.commit = commit + state.update(func(state *deliveryServerData) { state.commit = commit }) goDir, goLog := installGoBootstrapShim(t) env := templateEnvironment(repository, commit, goDir, goLog) @@ -51,8 +51,8 @@ func TestWoodpeckerTagDeliveryTemplate(t *testing.T) { if !strings.Contains(output, "state complete") { t.Fatalf("delivery output=%q", output) } - if state.releaseCreates != 1 || state.assetUploads != 0 { - t.Fatalf("source-only calls = creates:%d uploads:%d", state.releaseCreates, state.assetUploads) + if state.snapshot().releaseCreates != 1 || state.snapshot().assetUploads != 0 { + t.Fatalf("source-only calls = creates:%d uploads:%d", state.snapshot().releaseCreates, state.snapshot().assetUploads) } assertModuleBootstrap(t, repository, goLog, moduleBefore) }) @@ -73,7 +73,7 @@ func TestWoodpeckerTagDeliveryTemplate(t *testing.T) { Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}, }) commit := pushTaggedCLIRepository(t, repository) - state.commit = commit + state.update(func(state *deliveryServerData) { state.commit = commit }) goDir, goLog := installGoBootstrapShim(t) env := templateEnvironment(repository, commit, goDir, goLog) @@ -88,14 +88,14 @@ func TestWoodpeckerTagDeliveryTemplate(t *testing.T) { t.Fatalf("second delivery did not reuse matching assets: %q", output) } } - if state.releaseCreates != 1 || state.assetUploads != 2 { - t.Fatalf("binary calls = creates:%d uploads:%d", state.releaseCreates, state.assetUploads) + if state.snapshot().releaseCreates != 1 || state.snapshot().assetUploads != 2 { + t.Fatalf("binary calls = creates:%d uploads:%d", state.snapshot().releaseCreates, state.snapshot().assetUploads) } - if _, ok := state.assets["helper-v1.2.3-"+runtime.GOOS+"-"+runtime.GOARCH]; !ok { - t.Fatalf("binary asset was not delivered: %#v", state.assets) + if _, ok := state.snapshot().assets["helper-v1.2.3-"+runtime.GOOS+"-"+runtime.GOARCH]; !ok { + t.Fatalf("binary asset was not delivered: %#v", state.snapshot().assets) } - if _, ok := state.assets["SHA256SUMS"]; !ok { - t.Fatalf("checksum manifest was not delivered: %#v", state.assets) + if _, ok := state.snapshot().assets["SHA256SUMS"]; !ok { + t.Fatalf("checksum manifest was not delivered: %#v", state.snapshot().assets) } assertModuleBootstrap(t, repository, goLog, moduleBefore, sumBefore) }) diff --git a/delivery_reads_linux_test.go b/delivery_reads_linux_test.go index 5df8d74..2d9b9fb 100644 --- a/delivery_reads_linux_test.go +++ b/delivery_reads_linux_test.go @@ -24,9 +24,10 @@ func TestDeliverReadsLocalBinaryOnceForVerificationAndOnceForUpload(t *testing.T } state := &deliveryBenchmarkServer{commit: build.CandidateCommit, assets: expected} state.reset(false) - server := httptest.NewServer(http.HandlerFunc(state.serveHTTP)) + server := httptest.NewUnstartedServer(http.HandlerFunc(state.serveHTTP)) + state.baseURL = "http://" + server.Listener.Addr().String() + server.Start() defer server.Close() - state.baseURL = server.URL result, err := releasekit.Deliver(t.Context(), releasekit.DeliveryRequest{ RepositoryDir: repository, Product: "Sample App", Version: build.Version, Destination: facadeDeliveryDestination(server.URL), Prebuilt: &build, diff --git a/delivery_test.go b/delivery_test.go index f684c86..ace7799 100644 --- a/delivery_test.go +++ b/delivery_test.go @@ -13,6 +13,7 @@ import ( "path/filepath" "runtime" "strings" + "sync" "testing" releasekit "gitea.maximumdirect.net/eric/ReleaseKit" @@ -41,11 +42,12 @@ func TestDeliverManagedBuildUploadsArtifactsAndCleansTemporaryOutput(t *testing. if err != nil { t.Fatalf("Deliver() error = %v; result = %#v", err, result) } - if result.State != releasekit.DeliveryComplete || result.CandidateCommit != remote.commit { + observed := remote.snapshot() + if result.State != releasekit.DeliveryComplete || result.CandidateCommit != observed.commit { t.Fatalf("delivery result = %#v", result) } - if len(result.Uploaded) != 2 || len(remote.assets) != 2 { - t.Fatalf("uploaded = %#v; hosted assets = %v", result.Uploaded, mapKeys(remote.assets)) + if len(result.Uploaded) != 2 || len(observed.assets) != 2 { + t.Fatalf("uploaded = %#v; hosted assets = %v", result.Uploaded, mapKeys(observed.assets)) } if !strings.Contains(progress.String(), "checking fixture version output") { t.Fatalf("build progress = %q", progress.String()) @@ -79,7 +81,8 @@ func TestDeliverBinaryConsumerWithBuildinfoAndSelectedTargets(t *testing.T) { if err != nil { t.Fatalf("Deliver() error = %v; result = %#v", err, result) } - if result.State != releasekit.DeliveryComplete || result.CandidateCommit != remote.commit { + observed := remote.snapshot() + if result.State != releasekit.DeliveryComplete || result.CandidateCommit != observed.commit { t.Fatalf("delivery result = %#v", result) } wantAssets := []string{ @@ -87,12 +90,12 @@ func TestDeliverBinaryConsumerWithBuildinfoAndSelectedTargets(t *testing.T) { "fixture-v1.2.3-linux-arm64", "SHA256SUMS", } - if len(result.Uploaded) != len(wantAssets) || len(remote.assets) != len(wantAssets) { - t.Fatalf("uploaded = %#v; hosted assets = %v", result.Uploaded, mapKeys(remote.assets)) + if len(result.Uploaded) != len(wantAssets) || len(observed.assets) != len(wantAssets) { + t.Fatalf("uploaded = %#v; hosted assets = %v", result.Uploaded, mapKeys(observed.assets)) } for _, name := range wantAssets { - if _, ok := remote.assets[name]; !ok { - t.Errorf("hosted assets omit %q: %v", name, mapKeys(remote.assets)) + if _, ok := observed.assets[name]; !ok { + t.Errorf("hosted assets omit %q: %v", name, mapKeys(observed.assets)) } } if !strings.Contains(progress.String(), "checking fixture version output") { @@ -174,8 +177,8 @@ func TestDeliverRejectsSameSizePrebuiltTamperingBeforeHostedWrites(t *testing.T) if err == nil || !strings.Contains(err.Error(), "verify build assets") || result.State != releasekit.DeliveryNotAttempted { t.Fatalf("Deliver() = %#v, %v; want local asset verification failure", result, err) } - if remote.requests != 0 { - t.Fatalf("hosted requests = %d, want none before local verification", remote.requests) + if remote.snapshot().requests != 0 { + t.Fatalf("hosted requests = %d, want none before local verification", remote.snapshot().requests) } } @@ -217,8 +220,8 @@ func TestDeliverPageOnlySkipsBuildAndHostedAssets(t *testing.T) { if err != nil || result.State != releasekit.DeliveryComplete { t.Fatalf("Deliver() = %#v, %v", result, err) } - if len(remote.assets) != 0 || len(result.Uploaded) != 0 || len(result.Pending) != 0 { - t.Fatalf("page-only delivery included assets: result=%#v assets=%v", result, mapKeys(remote.assets)) + if len(remote.snapshot().assets) != 0 || len(result.Uploaded) != 0 || len(result.Pending) != 0 { + t.Fatalf("page-only delivery included assets: result=%#v assets=%v", result, mapKeys(remote.snapshot().assets)) } if progress.Len() != 0 { t.Fatalf("page-only delivery emitted build progress: %q", progress.String()) @@ -242,8 +245,8 @@ func TestDeliverUsesCommittedModuleIdentityForSeparatePages(t *testing.T) { ExpectedTag: "integrations/wrong/v1.2.3", Destination: facadeDeliveryDestination(server.URL), }) - if err == nil || wrongTag.Tag != "v1.2.3" || remote.requests != 0 { - t.Fatalf("delivery with inconsistent expected tag = %#v, %v; hosted requests %d", wrongTag, err, remote.requests) + if err == nil || wrongTag.Tag != "v1.2.3" || remote.snapshot().requests != 0 { + t.Fatalf("delivery with inconsistent expected tag = %#v, %v; hosted requests %d", wrongTag, err, remote.snapshot().requests) } rootResult, err := releasekit.Deliver(context.Background(), releasekit.DeliveryRequest{ @@ -258,8 +261,10 @@ func TestDeliverUsesCommittedModuleIdentityForSeparatePages(t *testing.T) { } nestedTag := "integrations/ckconfig/v1.2.3" - remote.tag = nestedTag - remote.noteBody = "# Fixture v1.2.3\n\nNested delivery fixture.\n" + remote.update(func(remote *facadeDeliveryData) { + remote.tag = nestedTag + remote.noteBody = "# Fixture v1.2.3\n\nNested delivery fixture.\n" + }) nestedResult, err := releasekit.Deliver(context.Background(), releasekit.DeliveryRequest{ RepositoryDir: repository, ModuleDir: "integrations/ckconfig", @@ -271,8 +276,8 @@ func TestDeliverUsesCommittedModuleIdentityForSeparatePages(t *testing.T) { if err != nil || nestedResult.State != releasekit.DeliveryComplete || nestedResult.Tag != nestedTag || nestedResult.ModulePath != "example.com/ckconfig" { t.Fatalf("nested delivery = %#v, %v", nestedResult, err) } - if len(remote.pages) != 2 || !remote.pages["v1.2.3"] || !remote.pages[nestedTag] { - t.Fatalf("created hosted pages = %#v", remote.pages) + if len(remote.snapshot().pages) != 2 || !remote.snapshot().pages["v1.2.3"] || !remote.snapshot().pages[nestedTag] { + t.Fatalf("created hosted pages = %#v", remote.snapshot().pages) } } @@ -293,21 +298,21 @@ func TestDeliverRejectsPrebuiltArtifactsForAnotherModuleBeforeHostedRequests(t * } server, remote := newFacadeDeliveryServer(t, repository) defer server.Close() - remote.tag = "integrations/ckconfig/v1.2.3" + remote.update(func(remote *facadeDeliveryData) { remote.tag = "integrations/ckconfig/v1.2.3" }) result, err := releasekit.Deliver(context.Background(), releasekit.DeliveryRequest{ RepositoryDir: repository, ModuleDir: "integrations/ckconfig", Product: "Fixture", Version: "v1.2.3", - ExpectedTag: remote.tag, + ExpectedTag: remote.snapshot().tag, Destination: facadeDeliveryDestination(server.URL), Prebuilt: &prebuilt, }) - if err == nil || !strings.Contains(err.Error(), "selected tagged module") || result.Tag != remote.tag { + if err == nil || !strings.Contains(err.Error(), "selected tagged module") || result.Tag != remote.snapshot().tag { t.Fatalf("nested delivery with root artifacts = %#v, %v", result, err) } - if remote.requests != 0 { - t.Fatalf("hosted requests before prebuilt identity rejection = %d", remote.requests) + if remote.snapshot().requests != 0 { + t.Fatalf("hosted requests before prebuilt identity rejection = %d", remote.snapshot().requests) } } @@ -351,14 +356,14 @@ func TestDeliverRejectsDirtyTaggedSourceBeforeBuildOrHostedWrites(t *testing.T) }, Output: &progress, }) - if err == nil || result.State != releasekit.DeliveryNotAttempted || result.CandidateCommit != remote.commit { + if err == nil || result.State != releasekit.DeliveryNotAttempted || result.CandidateCommit != remote.snapshot().commit { t.Fatalf("Deliver() = %#v, %v; want preflight failure for known commit", result, err) } if progress.Len() != 0 { t.Fatalf("build started before source eligibility passed: %q", progress.String()) } - if remote.requests != 0 { - t.Fatalf("hosted requests = %d, want none", remote.requests) + if remote.snapshot().requests != 0 { + t.Fatalf("hosted requests = %d, want none", remote.snapshot().requests) } } @@ -392,8 +397,8 @@ func TestDeliverRejectsCheckoutAwayFromTagBeforeBuildOrHostedWrites(t *testing.T if progress.Len() != 0 { t.Fatalf("build started before tag/HEAD agreement: %q", progress.String()) } - if remote.requests != 0 { - t.Fatalf("hosted requests = %d, want none", remote.requests) + if remote.snapshot().requests != 0 { + t.Fatalf("hosted requests = %d, want none", remote.snapshot().requests) } } @@ -417,14 +422,14 @@ func TestDeliverRejectsInvalidCommittedNoteBeforeBuildOrHostedWrites(t *testing. }, Output: &progress, }) - if err == nil || result.State != releasekit.DeliveryNotAttempted || result.CandidateCommit != remote.commit { + if err == nil || result.State != releasekit.DeliveryNotAttempted || result.CandidateCommit != remote.snapshot().commit { t.Fatalf("Deliver() = %#v, %v; want committed-note preflight failure", result, err) } if progress.Len() != 0 { t.Fatalf("build started before committed-note validation: %q", progress.String()) } - if remote.requests != 0 { - t.Fatalf("hosted requests = %d, want none", remote.requests) + if remote.snapshot().requests != 0 { + t.Fatalf("hosted requests = %d, want none", remote.snapshot().requests) } } @@ -458,8 +463,8 @@ func TestDeliverManagedBuildRejectsCheckoutMutationBeforeHostedWrites(t *testing if err == nil || !strings.Contains(err.Error(), "build source") || result.State != releasekit.DeliveryNotAttempted { t.Fatalf("Deliver() = %#v, %v; want build-source eligibility failure", result, err) } - if remote.requests != 0 { - t.Fatalf("hosted requests = %d, want none", remote.requests) + if remote.snapshot().requests != 0 { + t.Fatalf("hosted requests = %d, want none", remote.snapshot().requests) } if entries, err := os.ReadDir(temporaryRoot); err != nil || len(entries) != 0 { t.Fatalf("ineligible managed build output remains: entries=%v error=%v", entries, err) @@ -484,11 +489,11 @@ func TestDeliverManagedBuildFailureCleansTemporaryOutputBeforeHostedWrites(t *te Targets: []releasekit.Target{{OS: runtime.GOOS, Arch: runtime.GOARCH}}, }, }) - if err == nil || result.State != releasekit.DeliveryNotAttempted || result.CandidateCommit != remote.commit { + if err == nil || result.State != releasekit.DeliveryNotAttempted || result.CandidateCommit != remote.snapshot().commit { t.Fatalf("Deliver() = %#v, %v; want managed build failure before hosted writes", result, err) } - if remote.requests != 0 { - t.Fatalf("hosted requests = %d, want none", remote.requests) + if remote.snapshot().requests != 0 { + t.Fatalf("hosted requests = %d, want none", remote.snapshot().requests) } if entries, err := os.ReadDir(temporaryRoot); err != nil || len(entries) != 0 { t.Fatalf("failed managed build output remains: entries=%v error=%v", entries, err) @@ -520,8 +525,8 @@ func TestDeliverManagedBuildCancellationCleansTemporaryOutput(t *testing.T) { if !errors.Is(err, context.Canceled) || result.State != releasekit.DeliveryNotAttempted { t.Fatalf("Deliver() = %#v, %v; want cancellation before hosted writes", result, err) } - if remote.requests != 0 { - t.Fatalf("hosted requests = %d, want none", remote.requests) + if remote.snapshot().requests != 0 { + t.Fatalf("hosted requests = %d, want none", remote.snapshot().requests) } if entries, err := os.ReadDir(temporaryRoot); err != nil || len(entries) != 0 { t.Fatalf("canceled managed build output remains: entries=%v error=%v", entries, err) @@ -544,8 +549,8 @@ func TestDeliverRejectsBothBuildSourcesBeforeHostedWrites(t *testing.T) { if err == nil || result.State != releasekit.DeliveryNotAttempted || !strings.Contains(err.Error(), "both") { t.Fatalf("Deliver() = %#v, %v; want invalid build-source combination", result, err) } - if remote.requests != 0 { - t.Fatalf("hosted requests = %d, want none", remote.requests) + if remote.snapshot().requests != 0 { + t.Fatalf("hosted requests = %d, want none", remote.snapshot().requests) } } @@ -582,6 +587,35 @@ func (writer deliveryCancelWriter) Write(content []byte) (int, error) { } type facadeDeliveryServer struct { + mu sync.Mutex + data facadeDeliveryData +} + +func (state *facadeDeliveryServer) update(change func(*facadeDeliveryData)) { + state.mu.Lock() + defer state.mu.Unlock() + change(&state.data) +} + +func (state *facadeDeliveryServer) snapshot() facadeDeliveryData { + state.mu.Lock() + defer state.mu.Unlock() + result := state.data + + result.assets = make(map[string][]byte, len(state.data.assets)) + for key, value := range state.data.assets { + result.assets[key] = append([]byte(nil), value...) + } + + result.pages = make(map[string]bool, len(state.data.pages)) + for key, value := range state.data.pages { + result.pages[key] = value + } + + return result +} + +type facadeDeliveryData struct { t *testing.T commit string tag string @@ -595,24 +629,28 @@ type facadeDeliveryServer struct { func newFacadeDeliveryServer(t *testing.T, repository string) (*httptest.Server, *facadeDeliveryServer) { t.Helper() - state := &facadeDeliveryServer{t: t, commit: buildGit(t, repository, "rev-parse", "HEAD"), tag: "v1.2.3", version: "v1.2.3", noteBody: "# Fixture v1.2.3\n\nDelivery fixture.\n", pages: make(map[string]bool), assets: make(map[string][]byte)} - server := httptest.NewServer(http.HandlerFunc(state.serveHTTP)) - state.url = server.URL + state := &facadeDeliveryServer{data: facadeDeliveryData{t: t, commit: buildGit(t, repository, "rev-parse", "HEAD"), tag: "v1.2.3", version: "v1.2.3", noteBody: "# Fixture v1.2.3\n\nDelivery fixture.\n", pages: make(map[string]bool), assets: make(map[string][]byte)}} + server := httptest.NewUnstartedServer(http.HandlerFunc(state.serveHTTP)) + state.data.url = "http://" + server.Listener.Addr().String() + server.Start() return server, state } func (state *facadeDeliveryServer) serveHTTP(w http.ResponseWriter, r *http.Request) { + state.mu.Lock() + defer state.mu.Unlock() + data := &state.data if strings.HasPrefix(r.URL.Path, "/api/") { - state.requests++ + data.requests++ if r.Header.Get("Authorization") != "token fixture-token" { - state.t.Errorf("authorization = %q", r.Header.Get("Authorization")) + data.t.Errorf("authorization = %q", r.Header.Get("Authorization")) } } switch r.URL.EscapedPath() { - case "/api/v1/repos/owner/example/tags/" + url.PathEscape(state.tag): - _, _ = fmt.Fprintf(w, `{"name":%q,"commit":{"sha":%q}}`, state.tag, state.commit) - case "/api/v1/repos/owner/example/releases/tags/" + url.PathEscape(state.tag): - if !state.pages[state.tag] { + case "/api/v1/repos/owner/example/tags/" + url.PathEscape(data.tag): + _, _ = fmt.Fprintf(w, `{"name":%q,"commit":{"sha":%q}}`, data.tag, data.commit) + case "/api/v1/repos/owner/example/releases/tags/" + url.PathEscape(data.tag): + if !data.pages[data.tag] { http.NotFound(w, r) return } @@ -625,20 +663,20 @@ func (state *facadeDeliveryServer) serveHTTP(w http.ResponseWriter, r *http.Requ Body string `json:"body"` } if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { - state.t.Errorf("decode release payload: %v", err) + data.t.Errorf("decode release payload: %v", err) http.Error(w, "invalid payload", http.StatusBadRequest) return } - if payload.Tag != state.tag || payload.Target != state.commit || payload.Name != "Fixture "+state.version { - state.t.Errorf("release payload = %#v", payload) + if payload.Tag != data.tag || payload.Target != data.commit || payload.Name != "Fixture "+data.version { + data.t.Errorf("release payload = %#v", payload) } - state.pages[state.tag] = true + data.pages[data.tag] = true state.writeRelease(w) case "/api/v1/repos/owner/example/releases/10/assets": if r.Method == http.MethodGet { - list := make([]map[string]any, 0, len(state.assets)) - for name, content := range state.assets { - list = append(list, map[string]any{"id": len(list) + 1, "name": name, "size": len(content), "browser_download_url": state.url + "/files/" + name}) + list := make([]map[string]any, 0, len(data.assets)) + for name, content := range data.assets { + list = append(list, map[string]any{"id": len(list) + 1, "name": name, "size": len(content), "browser_download_url": data.url + "/files/" + name}) } _ = json.NewEncoder(w).Encode(list) return @@ -646,22 +684,22 @@ func (state *facadeDeliveryServer) serveHTTP(w http.ResponseWriter, r *http.Requ name := r.URL.Query().Get("name") file, _, err := r.FormFile("attachment") if err != nil { - state.t.Errorf("read upload %q: %v", name, err) + data.t.Errorf("read upload %q: %v", name, err) http.Error(w, "invalid multipart upload", http.StatusBadRequest) return } content, err := io.ReadAll(file) _ = file.Close() if err != nil { - state.t.Errorf("read upload %q: %v", name, err) + data.t.Errorf("read upload %q: %v", name, err) } - state.assets[name] = content + data.assets[name] = content w.WriteHeader(http.StatusCreated) - _ = json.NewEncoder(w).Encode(map[string]any{"id": len(state.assets), "name": name, "size": len(content)}) + _ = json.NewEncoder(w).Encode(map[string]any{"id": len(data.assets), "name": name, "size": len(content)}) default: if strings.HasPrefix(r.URL.Path, "/files/") { name := strings.TrimPrefix(r.URL.Path, "/files/") - content, ok := state.assets[name] + content, ok := data.assets[name] if !ok { http.NotFound(w, r) return @@ -669,17 +707,19 @@ func (state *facadeDeliveryServer) serveHTTP(w http.ResponseWriter, r *http.Requ _, _ = w.Write(content) return } - state.t.Errorf("unexpected delivery request %s %s", r.Method, r.URL) + data.t.Errorf("unexpected delivery request %s %s", r.Method, r.URL) http.NotFound(w, r) } } func (state *facadeDeliveryServer) writeRelease(w http.ResponseWriter) { + // The request handler holds state.mu. + data := &state.data w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(map[string]any{ - "id": 10, "html_url": state.url + "/releases/10", "tag_name": state.tag, - "target_commitish": state.commit, "name": "Fixture " + state.version, - "body": state.noteBody, "draft": false, "prerelease": false, + "id": 10, "html_url": data.url + "/releases/10", "tag_name": data.tag, + "target_commitish": data.commit, "name": "Fixture " + data.version, + "body": data.noteBody, "draft": false, "prerelease": false, }) } diff --git a/docs/roadmap/audit.md b/docs/roadmap/audit.md new file mode 100644 index 0000000..eb85dbe --- /dev/null +++ b/docs/roadmap/audit.md @@ -0,0 +1,152 @@ +# Targeted concurrency audit + +Status: complete; all five finding groups corrected and verified. + +## Scope and baseline + +Reviewed ReleaseKit at `6176971`, including the +[PromptKit report](promptkit-bug-report.md) and +[fixture synchronization roadmap](race.md). The audit covered shared-state +ownership in HTTP fixtures, background goroutines, cancellation callbacks, +subprocess-driven tests, output capture, and production operation callbacks. + +Code discovery used the codebase graph, with source inspection and filesystem +searches where the graph omitted recent files. Reviewed current source and +fixture consumers, including assertion and diagnostic reads. The full offline +race-enabled baseline used Go `go1.26.8 linux/amd64`: + +```sh +GOTOOLCHAIN=local GOWORK=off GOPROXY=off GOSUMDB=off \ + go test -race -mod=readonly -count=1 ./... +``` + +The recorded baseline failed in +`TestReleaseKitSourceDeliveryWorkflowFromTaggedCheckout`. It reported four race +diagnostics involving three fields: `commit`, `remoteCommit`, and +`releaseCreates`. Other packages passed. An earlier redirected invocation +returned unsuccessfully without diagnostics and supplied no usable baseline; +the complete results above came from a direct invocation. + +The additional findings below were established by source inspection. Their +absence from this baseline's diagnostics reflects its exercised schedules. + +## Findings and corrections + +### 1. CLI delivery fixtures share mutable state without synchronization + +Status: corrected; original reported defect and related fixture accesses. + +The HTTP handlers in [main_test.go](../../cmd/releasekit/main_test.go) read +configuration and update page state, counters, note content, and assets while +tests configure and inspect that state. The +[mapped workflow fixture](../../cmd/releasekit/multi_module_woodpecker_test.go) +has the same ownership pattern. Subprocess completion supplies no shared-state +synchronization with the parent process's HTTP goroutines. + +**Correction:** Each fixture owns a mutex and a separate plain state value. +Handlers serialize state access; test configuration uses locked updates, and +assertions and diagnostics use locked snapshots. Snapshots copy asset maps and +byte slices so inspection retains independent values. All consumers use that +boundary, including source-only, binary, sparse-checkout, alias, failure, +ordinary Woodpecker, and mapped-module tests. Response helpers run under the +handler's lock. The CLI fixture derives response URLs from the request; mapped +fixture configuration is initialized before its server starts. + +### 2. The public delivery fixture has the same unsynchronized ownership + +Status: corrected; additional finding. + +The fixture in [delivery_test.go](../../delivery_test.go) shares request counts, +tags, notes, release-page maps, and asset maps between handlers and test code. +The module-page test also changes the selected tag and note between deliveries. +Its consumers include [repository resolution](../../repository_test.go) and +[partial-clone inspection](../../source_inspection_test.go) tests. + +**Correction:** Added a fixture-local mutex, locked updates, and independent +snapshots, including copied page and asset maps. Tag and note changes occur in +one locked update. The server URL is initialized before serving requests. +Applied the same startup ordering to the existing synchronized benchmark server +and its [artifact-read regression test](../../delivery_reads_linux_test.go). +Binary assertion groups reuse their captured snapshot to avoid repeated asset +copies. + +### 3. Gitea adapter fixtures expose unsynchronized counters and flags + +Status: corrected; additional finding. + +[Release fixtures](../../internal/gitea/release_test.go) share creation and +lookup counts, creation flags, and cancellation observations between handlers +and assertions. Lost-acknowledgment scenarios also carry state across separate +HTTP connections. The +[external attachment fixture](../../internal/gitea/assets_test.go) shares its +download counter with the test goroutine. + +**Correction:** Used standard-library atomic counters and flags for these +independent scalar observations. Handler updates and assertion/diagnostic reads +use their atomic methods. Existing exact counts, conflict handling, recovery, +and external-origin credential assertions remain in place. + +### 4. HTTP assertions invoke fatal methods from request goroutines + +Status: corrected; additional finding. + +Gitea release and asset handlers, including their assertion helpers, used +`testing.T.Fatal` and `Fatalf`. Those methods terminate the calling goroutine +and belong to the test goroutine. In a handler, they can abandon request +processing and produce misleading transport failures. + +**Correction:** Handler assertions report through `Error` or `Errorf` and +return from the affected handler or helper. Fatal assertions remain in the test +goroutine. The fixtures still mark every violated assertion as a test failure. + +### 5. A release fixture omits server lifetime cleanup + +Status: corrected; additional finding. + +`newReleaseServer` returned an active HTTP server without registering cleanup, +and several callers omitted closure. This left listeners and serving goroutines +alive after their owning test finished. + +**Correction:** The helper registers `server.Close` with `t.Cleanup`, giving +server shutdown one owner for all its consumers. + +## Production and other reviewed paths + +The audit found no confirmed production concurrency defect in these paths: + +| Boundary | Ownership and synchronization evidence | +| --- | --- | +| [Git blob reader](../../internal/git/blob_reader.go) | A mutex serializes reads, close, and terminal state. A cancellation callback closes process pipes, and the read operation joins a started callback before continuing. Process diagnostics are inspected after child wait. | +| [Process adapter](../../internal/process/process.go) | Process handles are initialized before publication; cancellation uses a context. `sync.Once` protects child wait and its result. Ordinary command completion joins output-copy workers before returning captures. | +| [Go output capture](../../internal/gotool/gotool.go) | One mutex protects combined output, forwarding, and writer errors. Capture ownership transfers after the process finishes writing. | +| [Gitea upload](../../internal/gitea/upload.go) | The multipart worker owns the writer and file reader. An error channel transfers completion; request failure closes the pipe and joins the worker. | +| [Runtime metadata](../../internal/runtimeinfo/version.go) | The injected value is set at link time; fallback initialization uses `sync.OnceValue`. Existing concurrent accessor coverage uses atomics, channels, and a wait group. | +| [Go settings owner](../../internal/gotool/owner.go), candidate validation, and publication callbacks | Mutable owners and validation caches belong to one operation. Validation, product callbacks, and subsequent builds execute sequentially. Internal mutable owners remain behind the facade. | +| Background test workers | Reviewed build/process/dependency cancellation, Git-reader lifecycle, publication watchers, and destination-resolution workers. Results transfer through channels; the publication watcher has an explicit completion join. | +| Other HTTP fixtures | Immutable asset payloads remain read-only. The upload fixture already locks files and observations and returns copied POST names. The module-proxy fixture uses an atomic request counter. | +| Process environment | Tests changing environment settings run sequentially within their package; subprocess overlays belong to individual invocations. | + +These conclusions apply to the reviewed ownership paths. Race-enabled execution +and inspection supply evidence for this audit, rather than a guarantee about +every possible caller or schedule. Production files and public API contracts +were unchanged by the corrections. + +## Verification and completion + +The affected source-delivery, ordinary Woodpecker, and mapped-module workflows +passed with the race detector after fixture synchronization. The Gitea release +and asset checks also passed during focused verification. Existing assertions +continue to cover tag mismatch rejection, source preservation, complete notes, +idempotent delivery, asset counts, conflicts, and recovery. + +**Completed:** The final full offline race-enabled suite passed across all +packages with Go 1.26.8, including the original failing workflow and every +fixture consumer. The compiled public API/dependency check passed. The corrected +handlers, configuration updates, snapshots, diagnostics, counters, and server +startup/cleanup were reviewed against their ownership boundaries. Local +documentation links and fragments, Go formatting, and diff whitespace passed. +The linked roadmap and bug report record the completed remediation. + +The audit and corrections are local development work. Future publication follows +the existing [release procedure](../release.md), with the published v0.4.0 +contents preserved. diff --git a/docs/roadmap/promptkit-bug-report.md b/docs/roadmap/promptkit-bug-report.md index 98aa796..59a6b74 100644 --- a/docs/roadmap/promptkit-bug-report.md +++ b/docs/roadmap/promptkit-bug-report.md @@ -1,6 +1,8 @@ # PromptKit Bug Report: Race In Source-Delivery Test Fixture -Status: reported for upstream investigation; no implementation changes made. +Status: remediated in current source; focused and full offline race-enabled +verification passed. See the [concurrency audit](audit.md) for corrections and +additional findings. The original report below records the reviewed release. ## Reviewed Release And Environment diff --git a/docs/roadmap/race.md b/docs/roadmap/race.md index 30d8a32..a72871c 100644 --- a/docs/roadmap/race.md +++ b/docs/roadmap/race.md @@ -1,6 +1,9 @@ # Roadmap: synchronize delivery test fixtures -Status: planned; implementation has not started. +Status: complete; fixture synchronization and race-enabled verification passed. + +The [concurrency audit](audit.md) records the wider review, additional findings, +corrections, and final verification evidence. ## Objective and evidence @@ -101,3 +104,10 @@ This verification addresses the reported defect. Future release validation continues to follow the [release procedure](../release.md). The correction is suitable for a subsequent patch release; preparation and publication are separate work under that procedure, preserving the published v0.4.0 contents. + +**Completed:** Both delivery fixtures and all their consumers use mutex-protected +updates and independent snapshots. Existing mismatch, note, asset, retry, and +source-preservation assertions remain in place. Focused workflow checks and the +full offline race-enabled suite passed with Go 1.26.8; the compiled API/dependency +check passed. The audit also corrected public delivery fixture ownership, +Gitea scalar observations, HTTP assertion handling, and server cleanup. diff --git a/internal/gitea/assets_test.go b/internal/gitea/assets_test.go index b162f3a..60f26e6 100644 --- a/internal/gitea/assets_test.go +++ b/internal/gitea/assets_test.go @@ -47,7 +47,8 @@ func TestInspectAssetsAcceptsMatchingContentAndPreservesUnrelatedFiles(t *testin case "/files/notes": _, _ = w.Write([]byte("unrelated")) default: - t.Fatalf("unexpected path %s", r.URL.Path) + t.Errorf("unexpected path %s", r.URL.Path) + return } })) defer server.Close() @@ -107,7 +108,8 @@ func TestInspectAssetsAcceptsFullManifestWhenBinariesAreMissing(t *testing.T) { return } if r.URL.Path != "/files/manifest" { - t.Fatalf("unexpected path %s", r.URL.Path) + t.Errorf("unexpected path %s", r.URL.Path) + return } _, _ = w.Write(manifest.Content) })) @@ -237,7 +239,8 @@ func TestInspectAssetsFollowsSameOriginRedirectWithAuthorization(t *testing.T) { _, _ = w.Write(content) return } - t.Fatalf("unexpected path %s", r.URL.Path) + t.Errorf("unexpected path %s", r.URL.Path) + return })) defer server.Close() inspection, err := testClient(server).InspectAssets(context.Background(), testDestination(server.URL), 10, []ExpectedAsset{expected}) @@ -253,9 +256,9 @@ func TestInspectAssetsVerifiesExternalContentWithoutGiteaToken(t *testing.T) { for _, mode := range []string{"direct", "redirect", "conflicting"} { t.Run(mode, func(t *testing.T) { expected := assetFor("app.bin", []byte("matching bytes")) - externalGets := 0 + var externalGets atomic.Int64 external := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - externalGets++ + externalGets.Add(1) if r.Header.Get("Authorization") != "" { t.Error("Gitea token reached external host") } @@ -287,8 +290,8 @@ func TestInspectAssetsVerifiesExternalContentWithoutGiteaToken(t *testing.T) { if mode == "conflicting" { want = AssetConflicting } - if inspection.Assets[0].State != want || externalGets != 1 { - t.Fatalf("inspection=%#v downloads=%d", inspection, externalGets) + if inspection.Assets[0].State != want || externalGets.Load() != 1 { + t.Fatalf("inspection=%#v downloads=%d", inspection, externalGets.Load()) } }) } diff --git a/internal/gitea/release_test.go b/internal/gitea/release_test.go index 998535a..7bc65ac 100644 --- a/internal/gitea/release_test.go +++ b/internal/gitea/release_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "strings" + "sync/atomic" "testing" ) @@ -40,19 +41,23 @@ func TestEnsureReleaseEscapesSlashContainingTag(t *testing.T) { http.NotFound(w, r) case "/api/v1/repos/owner/project/releases": assertRequest(t, r, http.MethodPost, r.URL.Path) - state.posts++ + state.posts.Add(1) var payload createReleaseRequest if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { - t.Fatal(err) + t.Error(err) + return } if payload.TagName != tag || payload.Name != "Example v1.2.3" || payload.Body != testNote { - t.Fatalf("create payload = %#v", payload) + t.Errorf("create payload = %#v", payload) + return } if err := json.NewEncoder(w).Encode(releaseResponse{ID: 7, HTMLURL: "https://example.test/release", TagName: tag, TargetCommitish: testCommit, Name: payload.Name, Body: payload.Body}); err != nil { - t.Fatal(err) + t.Error(err) + return } default: - t.Fatalf("unexpected escaped request %s %s", r.Method, r.URL.EscapedPath()) + t.Errorf("unexpected escaped request %s %s", r.Method, r.URL.EscapedPath()) + return } })) defer server.Close() @@ -60,8 +65,8 @@ func TestEnsureReleaseEscapesSlashContainingTag(t *testing.T) { request := testRequest(server.URL) request.Tag = tag result, err := testClient(server).EnsureRelease(context.Background(), request) - if err != nil || !result.Created || state.posts != 1 { - t.Fatalf("nested tag release = %#v, posts %d, error %v", result, state.posts, err) + if err != nil || !result.Created || state.posts.Load() != 1 { + t.Fatalf("nested tag release = %#v, posts %d, error %v", result, state.posts.Load(), err) } } @@ -82,7 +87,7 @@ func TestEnsureReleaseConflictDoesNotEditExistingPage(t *testing.T) { if failureKind(t, err) != FailureConflict { t.Fatalf("error = %v", err) } - if got := state.posts; got != 0 { + if got := state.posts.Load(); got != 0 { t.Fatalf("release creates = %d, want 0", got) } } @@ -103,68 +108,68 @@ func TestEnsureReleaseUnauthorizedIsStructuredAndHidesToken(t *testing.T) { } func TestEnsureReleaseMalformedTagResponseStopsBeforeCreate(t *testing.T) { - posts := 0 + var posts atomic.Int64 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method == http.MethodPost { - posts++ + posts.Add(1) } assertRequest(t, r, http.MethodGet, "/api/v1/repos/owner/project/tags/v1.2.3") _, _ = w.Write([]byte("{")) })) defer server.Close() _, err := testClient(server).EnsureRelease(context.Background(), testRequest(server.URL)) - if failureKind(t, err) != FailureMalformed || posts != 0 { - t.Fatalf("error = %v, posts = %d", err, posts) + if failureKind(t, err) != FailureMalformed || posts.Load() != 0 { + t.Fatalf("error = %v, posts = %d", err, posts.Load()) } } func TestEnsureReleaseWrongHostedTagStopsBeforeCreate(t *testing.T) { - posts := 0 + var posts atomic.Int64 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assertRequest(t, r, http.MethodGet, "/api/v1/repos/owner/project/tags/v1.2.3") if r.Method == http.MethodPost { - posts++ + posts.Add(1) } _, _ = fmt.Fprintf(w, `{"commit":{"sha":"%040d"}}`, 7) })) defer server.Close() _, err := testClient(server).EnsureRelease(context.Background(), testRequest(server.URL)) - if failureKind(t, err) != FailureConflict || posts != 0 { - t.Fatalf("error = %v, posts = %d", err, posts) + if failureKind(t, err) != FailureConflict || posts.Load() != 0 { + t.Fatalf("error = %v, posts = %d", err, posts.Load()) } } func TestEnsureReleaseMissingHostedTagStopsBeforeCreate(t *testing.T) { - posts := 0 + var posts atomic.Int64 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assertRequest(t, r, http.MethodGet, "/api/v1/repos/owner/project/tags/v1.2.3") if r.Method == http.MethodPost { - posts++ + posts.Add(1) } http.NotFound(w, r) })) defer server.Close() _, err := testClient(server).EnsureRelease(context.Background(), testRequest(server.URL)) - if failureKind(t, err) != FailureNotFound || posts != 0 { - t.Fatalf("error = %v, posts = %d", err, posts) + if failureKind(t, err) != FailureNotFound || posts.Load() != 0 { + t.Fatalf("error = %v, posts = %d", err, posts.Load()) } } func TestEnsureReleaseHonorsContextCancellation(t *testing.T) { - called := false - server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { called = true })) + var called atomic.Bool + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { called.Store(true) })) defer server.Close() ctx, cancel := context.WithCancel(context.Background()) cancel() _, err := testClient(server).EnsureRelease(ctx, testRequest(server.URL)) - if !errors.Is(err, context.Canceled) || called { - t.Fatalf("error = %v, server called = %t", err, called) + if !errors.Is(err, context.Canceled) || called.Load() { + t.Fatalf("error = %v, server called = %t", err, called.Load()) } } func TestEnsureReleaseResolvesLostCreateAcknowledgmentWithOneLookup(t *testing.T) { - created := false - lookups := 0 + var created atomic.Bool + var lookups atomic.Int64 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assertAuth(t, r) switch r.URL.Path { @@ -173,8 +178,8 @@ func TestEnsureReleaseResolvesLostCreateAcknowledgmentWithOneLookup(t *testing.T writeTag(t, w, testCommit) case "/api/v1/repos/owner/project/releases/tags/v1.2.3": assertRequest(t, r, http.MethodGet, r.URL.Path) - lookups++ - if !created { + lookups.Add(1) + if !created.Load() { http.NotFound(w, r) return } @@ -183,23 +188,28 @@ func TestEnsureReleaseResolvesLostCreateAcknowledgmentWithOneLookup(t *testing.T assertRequest(t, r, http.MethodPost, r.URL.Path) var payload createReleaseRequest if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { - t.Fatal(err) + t.Error(err) + return } if payload.TagName != "v1.2.3" || payload.TargetCommitish != testCommit || payload.Name != "Example v1.2.3" || payload.Body != testNote || payload.Draft || payload.Prerelease { - t.Fatalf("create payload = %#v", payload) + t.Errorf("create payload = %#v", payload) + return } - created = true + created.Store(true) hijacker, ok := w.(http.Hijacker) if !ok { - t.Fatal("HTTP writer does not support hijacking") + t.Error("HTTP writer does not support hijacking") + return } connection, _, err := hijacker.Hijack() if err != nil { - t.Fatal(err) + t.Error(err) + return } _ = connection.Close() default: - t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + return } })) defer server.Close() @@ -207,13 +217,13 @@ func TestEnsureReleaseResolvesLostCreateAcknowledgmentWithOneLookup(t *testing.T if err != nil { t.Fatal(err) } - if result.Created || lookups != 2 { - t.Fatalf("result = %#v, release lookups = %d", result, lookups) + if result.Created || lookups.Load() != 2 { + t.Fatalf("result = %#v, release lookups = %d", result, lookups.Load()) } } func TestEnsureReleaseResolvesCreateConflictWithOneLookup(t *testing.T) { - lookups := 0 + var lookups atomic.Int64 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assertAuth(t, r) switch r.URL.Path { @@ -222,8 +232,8 @@ func TestEnsureReleaseResolvesCreateConflictWithOneLookup(t *testing.T) { writeTag(t, w, testCommit) case "/api/v1/repos/owner/project/releases/tags/v1.2.3": assertRequest(t, r, http.MethodGet, r.URL.Path) - lookups++ - if lookups == 1 { + lookups.Add(1) + if lookups.Load() == 1 { http.NotFound(w, r) return } @@ -232,7 +242,8 @@ func TestEnsureReleaseResolvesCreateConflictWithOneLookup(t *testing.T) { assertRequest(t, r, http.MethodPost, r.URL.Path) w.WriteHeader(http.StatusConflict) default: - t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + return } })) defer server.Close() @@ -240,13 +251,13 @@ func TestEnsureReleaseResolvesCreateConflictWithOneLookup(t *testing.T) { if err != nil { t.Fatal(err) } - if result.Created || result.Release.Title != "Created by another request" || lookups != 2 { - t.Fatalf("result = %#v, release lookups = %d", result, lookups) + if result.Created || result.Release.Title != "Created by another request" || lookups.Load() != 2 { + t.Fatalf("result = %#v, release lookups = %d", result, lookups.Load()) } } func TestEnsureReleaseReportsUnknownWhenAmbiguousCreateCannotBeInspected(t *testing.T) { - lookups := 0 + var lookups atomic.Int64 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assertAuth(t, r) switch r.URL.Path { @@ -255,8 +266,8 @@ func TestEnsureReleaseReportsUnknownWhenAmbiguousCreateCannotBeInspected(t *test writeTag(t, w, testCommit) case "/api/v1/repos/owner/project/releases/tags/v1.2.3": assertRequest(t, r, http.MethodGet, r.URL.Path) - lookups++ - if lookups == 1 { + lookups.Add(1) + if lookups.Load() == 1 { http.NotFound(w, r) return } @@ -265,13 +276,14 @@ func TestEnsureReleaseReportsUnknownWhenAmbiguousCreateCannotBeInspected(t *test assertRequest(t, r, http.MethodPost, r.URL.Path) w.WriteHeader(http.StatusInternalServerError) default: - t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + return } })) defer server.Close() _, err := testClient(server).EnsureRelease(context.Background(), testRequest(server.URL)) - if failureKind(t, err) != FailureUnknown || lookups != 2 { - t.Fatalf("error = %v, release lookups = %d", err, lookups) + if failureKind(t, err) != FailureUnknown || lookups.Load() != 2 { + t.Fatalf("error = %v, release lookups = %d", err, lookups.Load()) } } @@ -282,7 +294,7 @@ type releaseFixture struct { pre bool } -type fixtureState struct{ posts int } +type fixtureState struct{ posts atomic.Int64 } func newReleaseServer(t *testing.T, existing releaseFixture, createMissing bool) (*httptest.Server, *fixtureState) { t.Helper() @@ -302,13 +314,15 @@ func newReleaseServer(t *testing.T, existing releaseFixture, createMissing bool) writeRelease(t, w, existing) case "/api/v1/repos/owner/project/releases": assertRequest(t, r, http.MethodPost, r.URL.Path) - state.posts++ + state.posts.Add(1) var payload createReleaseRequest if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { - t.Fatal(err) + t.Error(err) + return } if payload.TagName != "v1.2.3" || payload.TargetCommitish != testCommit || payload.Name != "Example v1.2.3" || payload.Body != testNote || payload.Draft || payload.Prerelease { - t.Fatalf("create payload = %#v", payload) + t.Errorf("create payload = %#v", payload) + return } if !createMissing { w.WriteHeader(http.StatusConflict) @@ -316,10 +330,12 @@ func newReleaseServer(t *testing.T, existing releaseFixture, createMissing bool) } writeRelease(t, w, releaseFixture{title: payload.Name}) default: - t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + return } }) server := httptest.NewServer(handler) + t.Cleanup(server.Close) return server, state } @@ -346,7 +362,8 @@ func (fixture releaseFixture) bodyOrDefault() string { func assertRequest(t *testing.T, request *http.Request, method, path string) { t.Helper() if request.Method != method || request.URL.Path != path { - t.Fatalf("request = %s %s, want %s %s", request.Method, request.URL.Path, method, path) + t.Errorf("request = %s %s, want %s %s", request.Method, request.URL.Path, method, path) + return } assertAuth(t, request) } @@ -354,7 +371,8 @@ func assertRequest(t *testing.T, request *http.Request, method, path string) { func assertAuth(t *testing.T, request *http.Request) { t.Helper() if got := request.Header.Get("Authorization"); got != "token "+testToken { - t.Fatalf("Authorization = %q", got) + t.Errorf("Authorization = %q", got) + return } } @@ -382,14 +400,14 @@ func contains(value, substring string) bool { func TestEnsureReleaseReconcilesInconsistentSuccessfulCreate(t *testing.T) { for _, mode := range []string{"matching", "conflicting", "unavailable"} { t.Run(mode, func(t *testing.T) { - posts, lookups := 0, 0 + var posts, lookups atomic.Int64 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { case strings.Contains(r.URL.Path, "/tags/") && !strings.Contains(r.URL.Path, "/releases/"): writeTag(t, w, testCommit) case strings.Contains(r.URL.Path, "/releases/tags/"): - lookups++ - if posts == 0 { + lookups.Add(1) + if posts.Load() == 0 { http.NotFound(w, r) return } @@ -403,7 +421,7 @@ func TestEnsureReleaseReconcilesInconsistentSuccessfulCreate(t *testing.T) { } writeRelease(t, w, releaseFixture{title: "Example v1.2.3", body: body}) case r.Method == http.MethodPost: - posts++ + posts.Add(1) w.WriteHeader(http.StatusCreated) json.NewEncoder(w).Encode(releaseResponse{ID: 10, TagName: "v1.2.3", Name: "inconsistent title", Body: testNote}) default: @@ -412,8 +430,8 @@ func TestEnsureReleaseReconcilesInconsistentSuccessfulCreate(t *testing.T) { })) defer server.Close() result, err := testClient(server).EnsureRelease(context.Background(), testRequest(server.URL)) - if posts != 1 || lookups != 2 { - t.Fatalf("posts=%d lookups=%d", posts, lookups) + if posts.Load() != 1 || lookups.Load() != 2 { + t.Fatalf("posts=%d lookups=%d", posts.Load(), lookups.Load()) } switch mode { case "matching": diff --git a/performance_delivery_test.go b/performance_delivery_test.go index 1ad0596..16d9de7 100644 --- a/performance_delivery_test.go +++ b/performance_delivery_test.go @@ -47,8 +47,9 @@ type deliveryBenchmarkIdentity struct { func BenchmarkPerformanceFacadeDeliver(b *testing.B) { repository, build, expected := newDeliveryBenchmarkFixture(b) state := &deliveryBenchmarkServer{commit: gitText(b, repository, "rev-parse", "HEAD"), assets: expected} - server := httptest.NewServer(http.HandlerFunc(state.serveHTTP)) - state.baseURL = server.URL + server := httptest.NewUnstartedServer(http.HandlerFunc(state.serveHTTP)) + state.baseURL = "http://" + server.Listener.Addr().String() + server.Start() b.Cleanup(server.Close) request := releasekit.DeliveryRequest{ RepositoryDir: repository, diff --git a/repository_test.go b/repository_test.go index 1093376..5c9d4ea 100644 --- a/repository_test.go +++ b/repository_test.go @@ -40,7 +40,7 @@ func TestModuleSelectionFollowsSymlinkBeforeParentAcrossOperations(t *testing.T) buildGit(t, repository, "tag", wantTag) server, remote := newFacadeDeliveryServer(t, repository) defer server.Close() - remote.tag, remote.noteBody = wantTag, note + remote.update(func(remote *facadeDeliveryData) { remote.tag, remote.noteBody = wantTag, note }) delivered, err := releasekit.Deliver(t.Context(), releasekit.DeliveryRequest{ RepositoryDir: repository, ModuleDir: "link/..", Product: "Fixture", Version: version, ExpectedTag: wantTag, Destination: facadeDeliveryDestination(server.URL), diff --git a/source_inspection_test.go b/source_inspection_test.go index 393831f..4dd3448 100644 --- a/source_inspection_test.go +++ b/source_inspection_test.go @@ -40,8 +40,8 @@ func TestSourceOperationsRejectUnavailableNoteWithoutTransport(t *testing.T) { if checked.Commit != commit || checked.Tag != "v1.2.3" || delivered.CandidateCommit != commit || delivered.Tag != checked.Tag || delivered.State != releasekit.DeliveryNotAttempted { t.Fatalf("missing-note evidence: check=%#v, delivery=%#v", checked, delivered) } - if remote.requests != 0 { - t.Fatalf("missing note caused %d hosted requests", remote.requests) + if remote.snapshot().requests != 0 { + t.Fatalf("missing note caused %d hosted requests", remote.snapshot().requests) } assertInspectionStayedLocal(t, repository, attempts, before) }