Simplify pass + code-review fixes on today's changes

cmd/chargen/golden_test.go

@@ -4,15 +4,15 @@ import "testing"
 
 // TestGoldenCareerScoutSeed7 pins chargen's rendered character sheet for a
 // seeded career run, byte for byte — see cmd/worldgen/golden_test.go for why.
+// It also checks the same run against the README's sample block, sharing the
+// one subprocess invocation rather than re-running the command a second time.
 // Run `go test ./cmd/chargen/... -update` to regenerate after a deliberate
 // rules change.
 func TestGoldenCareerScoutSeed7(t *testing.T) {
-	command.Run(t, "-career", "scout", "-seed", "7").AssertGolden(t, "careerScoutSeed7")
-}
-
-// TestReadmeSampleUpToDate fails if the README's chargen sample ever diverges
-// from a fresh run of the exact invocation it shows.
-func TestReadmeSampleUpToDate(t *testing.T) {
 	got := command.Run(t, "-career", "scout", "-seed", "7")
-	got.AssertReadmeUpToDate(t, "../../README.md", "$ go run ./cmd/chargen -career scout -seed 7")
+
+	t.Run("golden", func(t *testing.T) { got.AssertGolden(t, "careerScoutSeed7") })
+	t.Run("readme", func(t *testing.T) {
+		got.AssertReadmeUpToDate(t, "../../README.md", "$ go run ./cmd/chargen -career scout -seed 7")
+	})
 }

cmd/sectorgen/golden_test.go

@@ -8,25 +8,29 @@ import (
 )
 
 // TestGoldenSeed42 pins sectorgen's default subsector listing for a seeded
-// run, byte for byte — see cmd/worldgen/golden_test.go for why. Run
-// `go test ./cmd/sectorgen/... -update` to regenerate after a deliberate
-// rules change.
+// run, byte for byte — see cmd/worldgen/golden_test.go for why. It also
+// checks the same run against the README's sample block, sharing the one
+// subprocess invocation rather than re-running the command a second time. The
+// README pipes the command through `head -3`, so only the first three lines
+// are shown — comparing the whole stdout against that block would fail on
+// line count alone, not content, so the readme check is a prefix comparison
+// instead of full equality (the one command among the four goldens where
+// that distinction matters). Run `go test ./cmd/sectorgen/... -update` to
+// regenerate after a deliberate rules change.
 func TestGoldenSeed42(t *testing.T) {
-	command.Run(t, "-seed", "42").AssertGolden(t, "seed42")
-}
-
-// TestReadmeSampleUpToDate fails if the README's sectorgen sample ever
-// diverges from a fresh run of the exact invocation it shows. The README
-// pipes the command through `head -3`, so only the first three lines are
-// shown — comparing the whole stdout against that block would fail on line
-// count alone, not content, so this checks a prefix instead of full equality
-// (the one command among the four goldens where that distinction matters).
-func TestReadmeSampleUpToDate(t *testing.T) {
 	got := command.Run(t, "-seed", "42")
-	want := clitest.ReadmeBlock(t, "../../README.md", "$ go run ./cmd/sectorgen -seed 42 | head -3")
 
-	if !strings.HasPrefix(got.Stdout, want) {
-		t.Errorf("README sample is stale.\n--- README shows (head -3) ---\n%s--- actual output's first lines ---\n%.*s",
-			want, len(want), got.Stdout)
-	}
+	t.Run("golden", func(t *testing.T) { got.AssertGolden(t, "seed42") })
+	t.Run("readme", func(t *testing.T) {
+		want := clitest.ReadmeBlock(t, "../../README.md", "$ go run ./cmd/sectorgen -seed 42 | head -3")
+
+		if !strings.HasPrefix(got.Stdout, want) {
+			t.Errorf(
+				"README sample is stale.\n--- README shows (head -3) ---\n%s--- actual output's first lines ---\n%.*s",
+				want,
+				len(want),
+				got.Stdout,
+			)
+		}
+	})
 }

cmd/shipgen/golden_test.go

@@ -15,21 +15,21 @@ func murphyReadmeArgs() []string {
 
 // TestGoldenReadmeDesign pins shipgen's rendered ship card for the README's
 // own design flags, byte for byte — see cmd/worldgen/golden_test.go for why.
-// Run `go test ./cmd/shipgen/... -update` to regenerate after a deliberate
-// rules change.
+// It also checks the same run against the README's sample block, sharing the
+// one subprocess invocation rather than re-running the command a second
+// time; that readme check caught a real instance of drift on first write —
+// the README was missing the Defenses/Fuel/problem lines the command
+// actually prints. Run `go test ./cmd/shipgen/... -update` to regenerate
+// after a deliberate rules change.
 func TestGoldenReadmeDesign(t *testing.T) {
-	command.Run(t, murphyReadmeArgs()...).AssertGolden(t, "readmeDesign")
-}
-
-// TestReadmeSampleUpToDate fails if the README's shipgen sample ever
-// diverges from a fresh run of the exact invocation it shows. It caught a
-// real instance of this on first write: the README was missing the
-// Defenses/Fuel/problem lines the command actually prints.
-func TestReadmeSampleUpToDate(t *testing.T) {
 	got := command.Run(t, murphyReadmeArgs()...)
-	got.AssertReadmeUpToDate(
-		t,
-		"../../README.md",
-		`$ go run ./cmd/shipgen -hull A -tl 12 -config S -maneuver A -jump A -weapon "beamlaser:T1:orbit" -defense blackglobe`,
-	)
+
+	t.Run("golden", func(t *testing.T) { got.AssertGolden(t, "readmeDesign") })
+	t.Run("readme", func(t *testing.T) {
+		got.AssertReadmeUpToDate(
+			t,
+			"../../README.md",
+			`$ go run ./cmd/shipgen -hull A -tl 12 -config S -maneuver A -jump A -weapon "beamlaser:T1:orbit" -defense blackglobe`,
+		)
+	})
 }

cmd/worldgen/golden_test.go

@@ -10,19 +10,18 @@ import (
 // TestGoldenN3Seed42 pins worldgen's rendered stdout for a seeded run,
 // byte for byte, closing the #321 hazard: nothing else in this package would
 // notice a reordered field or a shifted dice stream that changed every
-// generated UWP without changing the exit code or the seed report. Run
-// `go test ./cmd/worldgen/... -update` to regenerate after a deliberate
+// generated UWP without changing the exit code or the seed report. It also
+// checks the same run against the README's sample block, sharing the one
+// subprocess invocation rather than re-running the command a second time.
+// Run `go test ./cmd/worldgen/... -update` to regenerate after a deliberate
 // rules change — eyeball the diff before committing it.
 func TestGoldenN3Seed42(t *testing.T) {
-	command.Run(t, "-n", "3", "-seed", "42").AssertGolden(t, "n3seed42")
-}
-
-// TestReadmeSampleUpToDate fails if the README's "Sample output" block for
-// this command ever diverges from a fresh run of the exact invocation it
-// shows.
-func TestReadmeSampleUpToDate(t *testing.T) {
 	got := command.Run(t, "-n", "3", "-seed", "42")
-	got.AssertReadmeUpToDate(t, "../../README.md", "$ go run ./cmd/worldgen -n 3 -seed 42")
+
+	t.Run("golden", func(t *testing.T) { got.AssertGolden(t, "n3seed42") })
+	t.Run("readme", func(t *testing.T) {
+		got.AssertReadmeUpToDate(t, "../../README.md", "$ go run ./cmd/worldgen -n 3 -seed 42")
+	})
 }
 
 // TestGoldenN3Seed42JSON pins -format json's rendered output, byte for byte —

cmd/worldgen/main.go

@@ -74,6 +74,16 @@ func main() {
 			// guard rather than bypassing it via string(p.Starport) directly.
 			uwpStr := p.String()
 
+			// TradeClassifications returns a nil slice when no code matches —
+			// a real, valid outcome, not an edge case to special-case away —
+			// so it is coerced to an empty slice here to render "[]" rather
+			// than "null"; a code-less world otherwise breaks any JSON
+			// consumer that iterates tradeCodes without a null check.
+			jsonTC := tc
+			if jsonTC == nil {
+				jsonTC = []tradecode.Code{}
+			}
+
 			err := enc.Encode(jsonWorld{
 				UWP:           uwpStr,
 				Starport:      uwpStr[:1],
@@ -84,9 +94,15 @@ func main() {
 				Government:    p.Government,
 				Law:           p.Law,
 				TechLevel:     p.TechLevel,
-				TradeCodes:    tc,
+				TradeCodes:    jsonTC,
 			})
 			if err != nil {
+				// Flush what already rendered before exiting: out is only
+				// otherwise flushed once, after the loop, so an unflushed
+				// buffer here would silently discard every earlier record
+				// this run already generated.
+				_ = out.Flush()
+
 				cli.Fatalf("writing world %d: %v", i, err)
 			}
 

internal/dice/dice.go

@@ -96,20 +96,6 @@ func NewScripted(faces ...int) *Roller {
 	})
 }
 
-// Fixed returns a Roller where every die shows face, forever. It pins one
-// variable in a property sweep — e.g. holding a die at 6 while another
-// Roller varies the rest — where NewScripted's exact, finite script would be
-// the wrong shape.
-//
-// It panics if face is not a real die face in 1..6.
-func Fixed(face int) *Roller {
-	if face < 1 || face > 6 {
-		panic(fmt.Sprintf("dice: Fixed face %d is not a die face (want 1..6)", face))
-	}
-
-	return NewSource(func() int { return face })
-}
-
 // Seed reports the seed the Roller was built from and whether it has one. A
 // Roller from NewSource or NewScripted draws from a supplied sequence rather
 // than a seeded generator, so it reports ok false.

internal/dice/dice_test.go

@@ -57,34 +57,6 @@ func TestScriptedPanicsWhenExhausted(t *testing.T) {
 	r.Die()
 }
 
-// TestFixedRejectsNonFaces mirrors TestScriptedRejectsNonFaces: Fixed
-// validates its face eagerly, at construction, not at first roll.
-func TestFixedRejectsNonFaces(t *testing.T) {
-	for _, bad := range []int{0, 7, -1} {
-		func() {
-			defer func() {
-				if recover() == nil {
-					t.Errorf("Fixed(%d) accepted a non-die face", bad)
-				}
-			}()
-
-			Fixed(bad)
-		}()
-	}
-}
-
-// TestFixedRepeatsForever distinguishes Fixed from NewScripted: unlike a
-// script, it never exhausts.
-func TestFixedRepeatsForever(t *testing.T) {
-	r := Fixed(6)
-
-	for range 100 {
-		if got := r.Die(); got != 6 {
-			t.Fatalf("Die() = %d, want 6", got)
-		}
-	}
-}
-
 func TestDie(t *testing.T) {
 	r := scripted(4)
 	if got := r.Die(); got != 4 {

internal/shipgen/design_test.go

@@ -50,11 +50,15 @@ func TestDesignMurphy(t *testing.T) {
 		t.Errorf("drive costs = %d/%d/%d, want 4,000,000/10,000,000/4,000,000",
 			s.Maneuver.Cost, s.Jump.Cost, s.Power.Cost)
 	}
-	// Core cost is the sum of the components just checked above, not a
-	// separately hand-added total — so it can't drift from them while still
-	// passing. (The book's MCr 70.3 full-ship figure additionally includes the
-	// deferred staterooms, turret, and sensors.)
-	if want := s.Hull.Cost + s.Maneuver.Cost + s.Jump.Cost + s.Power.Cost + s.Fuel.Cost; s.Cost != want {
+	// Core cost is the sum of the components just checked above. Fuel is
+	// pinned by its own formula (TestFuelMurphy: 22t x Cr500 + KCr100 scoop +
+	// MCr1 purifier), not read back off s.Fuel.Cost — reading it back would
+	// make this assertion tautological with respect to design.go's own
+	// ship.Cost formula and unable to catch a wrong fuel price. (The book's
+	// MCr 70.3 full-ship figure additionally includes the deferred
+	// staterooms, turret, and sensors.)
+	const fuelCost = 22*500 + 100_000 + 1_000_000
+	if want := s.Hull.Cost + s.Maneuver.Cost + s.Jump.Cost + s.Power.Cost + fuelCost; s.Cost != want {
 		t.Errorf("core cost = %d, want %d (Hull+Maneuver+Jump+Power+Fuel)", s.Cost, want)
 	}
 }

internal/shipgen/generate.go

@@ -27,21 +27,16 @@ func ConfigByLetter(letter string) (Config, bool) {
 
 // StructureByName returns the Structure for a name and whether it was found,
 // matching case-, space-, and hyphen-insensitively against the structure's CLI
-// short name (see StructureNames), its display name, or any of its aliases —
-// so "plate", "Frame-and-Plate", and "frame-plate" all resolve to FramePlate.
+// short name (see StructureNames), its display name, or its alias — so
+// "plate", "Frame-and-Plate", and "frame-plate" all resolve to FramePlate.
 func StructureByName(name string) (Structure, bool) {
 	key := squash(name)
 
 	for i, d := range structureData {
-		if squash(d.cliName) == key || squash(d.display) == key {
+		if squash(d.cliName) == key || squash(d.display) == key ||
+			(d.alias != "" && squash(d.alias) == key) {
 			return Structure(i), true
 		}
-
-		for _, alias := range d.aliases {
-			if squash(alias) == key {
-				return Structure(i), true
-			}
-		}
 	}
 
 	return 0, false

internal/shipgen/shipgen.go

@@ -83,16 +83,16 @@ const (
 // never be reached, since StructureByName squashes hyphens out of its input
 // before the lookup.
 var structureData = [...]struct {
-	display string   // rendered form, e.g. in a ship card
-	cliName string   // -structure flag's short form, e.g. "plate"
-	aliases []string // other accepted -structure spellings, squashed at lookup
+	display string // rendered form, e.g. in a ship card
+	cliName string // -structure flag's short form, e.g. "plate"
+	alias   string // another accepted -structure spelling, squashed at lookup ("" if none)
 }{
-	FramePlate: {"Frame-and-Plate", "plate", []string{"frameplate"}},
-	Shell:      {"Shell", "shell", nil},
-	Polymer:    {"Polymer", "polymer", nil},
-	FeNi:       {"FeNi", "feni", nil},
-	Organic:    {"Organic", "organic", nil},
-	Charged:    {"Charged", "charged", nil},
+	FramePlate: {"Frame-and-Plate", "plate", "frameplate"},
+	Shell:      {"Shell", "shell", ""},
+	Polymer:    {"Polymer", "polymer", ""},
+	FeNi:       {"FeNi", "feni", ""},
+	Organic:    {"Organic", "organic", ""},
+	Charged:    {"Charged", "charged", ""},
 }
 
 func (s Structure) String() string {