From cd58b2bcfcc846b4dfd92350e092cd9478283649 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 8 Oct 2026 17:13:07 +0000 Subject: [PATCH] Test release launcher behavior without fixed command spellings --- docs/roadmap/remediation.md | 26 ++++-- scripts/release_test.go | 178 ++++++++++++++++++++++++++++-------- 2 files changed, 158 insertions(+), 46 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 047748b..b6ed1b1 100644 --- 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. diff --git a/scripts/release_test.go b/scripts/release_test.go index ad55682..868ca51 100644 --- 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"