Test release launcher behavior without fixed command spellings

2026-10-08 17:13:07 +00:00
parent 05a5bbb3e2
commit cd58b2bcfc
2 changed files with 158 additions and 46 deletions
--- a/docs/roadmap/remediation.md
+++ b/docs/roadmap/remediation.md
@@ -1,6 +1,6 @@
 # ReleaseKit adoption review findings

-Status: two remediation items pending; implementation not changed by this review.
+Status: R-01 complete; R-02 pending.

 Date: 2026-10-08.

@@ -31,7 +31,7 @@ gates, new public contracts, or additional publisher machinery.

 **Location:** [scripts/release_test.go](../../scripts/release_test.go),
 `TestReleaseLauncherDelegatesPublication`, `assertNativeBootstrap`
-(lines 222–260), and `fakeGo` (lines 363–397).
+and `fakeGo` as they existed at the reviewed revision.

 The assertion compares the entire build argument vector, including flag order
 and the absolute spelling of the output path. It also requires the two exact
@@ -56,15 +56,15 @@ those selected behaviors without elevating implementation details.

 Required work:

- [ ] Assert the required build settings, selected package, and resolved output
+- [x] Assert the required build settings, selected package, and resolved output
  destination without requiring one incidental flag ordering or path spelling.
- [ ] Make host-query fixture handling and assertions reflect the requested
+- [x] Make host-query fixture handling and assertions reflect the requested
  host values rather than requiring separate subprocess query forms. Keep the
  fixture small; a complete Go command emulator is unnecessary.
- [ ] Check the publication command, option values, version operand, and
+- [x] Check the publication command, option values, version operand, and
  argument boundaries without fixing the order of independent options. Retain
  the upstream requirement that options precede the version.
- [ ] Preserve checks for wrong native target, missing readonly mode, incorrect
+- [x] Preserve checks for wrong native target, missing readonly mode, incorrect
  package/output, workspace or toolchain mistakes, failed bootstrap, and
  publication status/stream propagation.

@@ -72,6 +72,20 @@ Completion evidence: focused launcher checks still detect the selected failures
 and accept an equivalent bootstrap form. This is a test correction, not a
 requirement to refactor the working launcher or add a release-time check.

+**Completed 2026-10-08:** bootstrap checks now inspect required options and
+resolve output paths; publication checks use the standard flag parser to preserve
+option-order freedom and operand boundaries. The small executable double returns
+each requested host value and accepts either output-option spelling. A fixture
+launcher with combined host queries, reordered options, and a relative output
+passed. Negative cases still reject incorrect target, workspace/toolchain,
+directory, readonly mode, package pin, output, publication paths, and operands;
+existing status/stream and bootstrap-failure checks passed.
+
+Validation: offline `go test -mod=readonly -count=1 ./scripts` and
+`go test -mod=readonly -count=1 ./...` both passed with `GOWORK=off`,
+`GOTOOLCHAIN=local`, `GOPROXY=off`, `GONOPROXY=none`, and `GOSUMDB=off`.
+The production launcher, profile, and module selection were not changed.
+
 ## R-02: clarify optional diagnostic validation in the runbook

 **Priority:** low. **Category:** potentially strengthened prohibition.
--- a/scripts/release_test.go
+++ b/scripts/release_test.go
@@ -5,7 +5,9 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"flag"
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -74,20 +76,78 @@ func TestReleaseLauncherDelegatesPublication(t *testing.T) {
 				t.Fatalf("stderr = %q, want publisher error output", result.stderr)
 			}

-			wantArgs := []string{
-				"publish",
-				"--repo", fixture.repoRoot,
-				"--config", filepath.Join(fixture.repoRoot, "releasekit.json"),
-				releaseVersion,
-			}
-			if got := readLines(t, fixture.publishLog); !equalStrings(got, wantArgs) {
-				t.Fatalf("publisher args = %#v, want %#v", got, wantArgs)
+			if err := checkPublication(readLines(t, fixture.publishLog), fixture.repoRoot); err != nil {
+				t.Fatal(err)
 			}
 			assertNativeBootstrap(t, fixture)
 		})
 	}
 }

+func TestReleaseLauncherAcceptsEquivalentCommands(t *testing.T) {
+	shell := requirePOSIXShell(t)
+	fixture := newFixture(t)
+	// Change only the fixture script: the production launcher need not be refactored.
+	writeFile(t, fixture.launcher, `#!/bin/sh
+set -eu
+version=$1
+root=$(CDPATH= cd "$(dirname "$0")/.." && pwd)
+CDPATH= cd "$root"
+mkdir -p .releasekit/bin
+set -- $(GOWORK=off GOTOOLCHAIN=local go env GOHOSTOS GOHOSTARCH)
+GOOS="$1" GOARCH="$2" GOWORK=off GOTOOLCHAIN=local go build \
+    -o=.releasekit/bin/releasekit -mod readonly \
+    gitea.maximumdirect.net/eric/ReleaseKit/cmd/releasekit
+exec .releasekit/bin/releasekit publish \
+    --config="$root/releasekit.json" --repo "$root" "$version"
+`, 0o755)
+	result := runLauncher(t, shell, fixture, []string{releaseVersion}, nil)
+	if result.status != 0 {
+		t.Fatalf("equivalent launcher failed: status=%d stderr=%q", result.status, result.stderr)
+	}
+	if err := checkPublication(readLines(t, fixture.publishLog), fixture.repoRoot); err != nil {
+		t.Fatal(err)
+	}
+	assertNativeBootstrap(t, fixture)
+}
+
+func TestLauncherChecksRejectContractViolations(t *testing.T) {
+	root := filepath.Join(t.TempDir(), "repository with spaces")
+	output := filepath.Join(root, ".releasekit", "bin", "releasekit")
+	for _, test := range []struct {
+		name   string
+		change func(*goCall)
+	}{
+		{"wrong target", func(c *goCall) { c.goarch = "wrong" }},
+		{"workspace enabled", func(c *goCall) { c.gowork = "auto" }},
+		{"toolchain download", func(c *goCall) { c.toolchain = "auto" }},
+		{"wrong directory", func(c *goCall) { c.cwd = filepath.Dir(root) }},
+		{"missing readonly", func(c *goCall) { c.args = []string{"build", "-o", output, releasePackage} }},
+		{"wrong output", func(c *goCall) { c.args[3] = filepath.Join(root, "wrong") }},
+		{"independent package pin", func(c *goCall) { c.args[4] += "@v0.2.0" }},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			call := goCall{cwd: root, goos: "linux", goarch: "amd64", gowork: "off", toolchain: "local",
+				args: []string{"build", "-mod=readonly", "-o", output, releasePackage}}
+			test.change(&call)
+			if err := checkNativeBootstrap([]goCall{call}, root, "linux", "amd64"); err == nil {
+				t.Fatal("invalid bootstrap accepted")
+			}
+		})
+	}
+	for _, args := range [][]string{
+		{"check", "--repo", root, "--config", filepath.Join(root, "releasekit.json"), releaseVersion},
+		{"publish", "--repo", "wrong", "--config", filepath.Join(root, "releasekit.json"), releaseVersion},
+		{"publish", "--repo", root, "--config", "wrong", releaseVersion},
+		{"publish", "--repo", root, "--config", filepath.Join(root, "releasekit.json"), "wrong-version"},
+		{"publish", releaseVersion, "--repo", root, "--config", filepath.Join(root, "releasekit.json")},
+	} {
+		if err := checkPublication(args, root); err == nil {
+			t.Errorf("invalid publication accepted: %q", args)
+		}
+	}
+}
+
 func TestReleaseLauncherStopsAfterBootstrapFailure(t *testing.T) {
 	shell := requirePOSIXShell(t)
 	fixture := newFixture(t)
@@ -221,44 +281,78 @@ func fixtureEnvironment(fixture fixture, overrides map[string]string) []string {

 func assertNativeBootstrap(t *testing.T, fixture fixture) {
 	t.Helper()
-	calls := readGoCalls(t, fixture.goLog)
-	var foundOS, foundArch, foundBuild bool
+	if err := checkNativeBootstrap(readGoCalls(t, fixture.goLog), fixture.repoRoot, "linux", "amd64"); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func checkNativeBootstrap(calls []goCall, root, hostOS, hostArch string) error {
+	foundBuild := false
 	for _, call := range calls {
-		if call.cwd != fixture.repoRoot {
-			t.Fatalf("Go ran in %q, want repository root %q", call.cwd, fixture.repoRoot)
+		if call.cwd != root {
+			return fmt.Errorf("Go ran in %q, want repository root %q", call.cwd, root)
 		}
-		if call.args[0] == "env" {
-			if call.gowork != "off" || call.toolchain != "local" {
-				t.Fatalf("Go env settings = GOWORK=%q GOTOOLCHAIN=%q", call.gowork, call.toolchain)
-			}
-			switch strings.Join(call.args, " ") {
-			case "env GOHOSTOS":
-				foundOS = true
-			case "env GOHOSTARCH":
-				foundArch = true
-			}
+		if call.gowork != "off" || call.toolchain != "local" {
+			return fmt.Errorf("Go settings = GOWORK=%q GOTOOLCHAIN=%q", call.gowork, call.toolchain)
 		}
-		if call.args[0] == "build" {
+		if len(call.args) > 0 && call.args[0] == "build" {
 			foundBuild = true
-			want := []string{
-				"build", "-mod=readonly", "-o",
-				filepath.Join(fixture.repoRoot, ".releasekit", "bin", "releasekit"),
-				releasePackage,
+			if call.args[len(call.args)-1] != releasePackage {
+				return fmt.Errorf("build does not select module-owned package %q: %q", releasePackage, call.args)
 			}
-			if !equalStrings(call.args, want) {
-				t.Fatalf("build args = %#v, want %#v", call.args, want)
+			if argumentValue(call.args[1:], "-mod") != "readonly" {
+				return fmt.Errorf("build does not use readonly module metadata: %q", call.args)
 			}
-			if call.goos != "linux" || call.goarch != "amd64" {
-				t.Fatalf("build target = %s/%s, want linux/amd64", call.goos, call.goarch)
+			output := argumentValue(call.args[1:], "-o")
+			if !filepath.IsAbs(output) {
+				output = filepath.Join(call.cwd, output)
 			}
-			if call.gowork != "off" || call.toolchain != "local" {
-				t.Fatalf("build settings = GOWORK=%q GOTOOLCHAIN=%q", call.gowork, call.toolchain)
+			if filepath.Clean(output) != filepath.Join(root, ".releasekit", "bin", "releasekit") {
+				return fmt.Errorf("incorrect build output %q", output)
+			}
+			if call.goos != hostOS || call.goarch != hostArch {
+				return fmt.Errorf("build target = %s/%s, want %s/%s", call.goos, call.goarch, hostOS, hostArch)
 			}
 		}
 	}
-	if !foundOS || !foundArch || !foundBuild {
-		t.Fatalf("host queries and build not all observed: os=%t arch=%t build=%t", foundOS, foundArch, foundBuild)
+	if !foundBuild {
+		return errors.New("no native build observed")
 	}
+	return nil
+}
+
+// Inspect only the selected build options, leaving other Go flags to Go itself.
+func argumentValue(args []string, name string) string {
+	value := ""
+	for i := 0; i < len(args); i++ {
+		if args[i] == name && i+1 < len(args) {
+			i++
+			value = args[i]
+		} else if supplied, ok := strings.CutPrefix(args[i], name+"="); ok {
+			value = supplied
+		}
+	}
+	return value
+}
+
+func checkPublication(args []string, root string) error {
+	if len(args) == 0 || args[0] != "publish" {
+		return fmt.Errorf("incorrect publication command: %q", args)
+	}
+	flags := flag.NewFlagSet("publish", flag.ContinueOnError)
+	flags.SetOutput(io.Discard)
+	repo := flags.String("repo", "", "")
+	config := flags.String("config", "", "")
+	if err := flags.Parse(args[1:]); err != nil {
+		return err
+	}
+	if *repo != root || *config != filepath.Join(root, "releasekit.json") {
+		return fmt.Errorf("incorrect publication paths: repo=%q config=%q", *repo, *config)
+	}
+	if !equalStrings(flags.Args(), []string{releaseVersion}) {
+		return fmt.Errorf("incorrect publication operands: %q", flags.Args())
+	}
+	return nil
 }

 type goCall struct {
@@ -367,11 +461,14 @@ for arg do printf '\t%s' "$arg" >> "$GO_LOG"; done
 printf '\n' >> "$GO_LOG"

 if [ "$1" = env ]; then
-    case "$2" in
-        GOHOSTOS) printf '%s\n' "$FAKE_GOHOSTOS" ;;
-        GOHOSTARCH) printf '%s\n' "$FAKE_GOHOSTARCH" ;;
-        *) exit 90 ;;
-    esac
+    shift
+    for name do
+        case "$name" in
+            GOHOSTOS) printf '%s\n' "$FAKE_GOHOSTOS" ;;
+            GOHOSTARCH) printf '%s\n' "$FAKE_GOHOSTARCH" ;;
+            *) exit 90 ;;
+        esac
+    done
    exit 0
 fi

@@ -386,6 +483,7 @@ if [ "$1" = build ]; then
            continue
        fi
        if [ "$arg" = -o ]; then expect_output=1; fi
+        case "$arg" in -o=*) output=${arg#-o=} ;; esac
    done
    [ -n "$output" ] || exit 91
    cat "$FAKE_PUBLISHER_SOURCE" > "$output"