amadan

carlos / platform Public

zdd-4 third fix wave: the R17 publication tests assert R19's publish-before-exec

Per the controller's ruling on concern 1: stagedFake now always keeps
ExecBackend's before-exec hook contract (the opt-in flag is gone), so no
test can pass because the fake skips the hook. Two R17 tests asserted
what R19 reverses and are retired, because third-wave tests already
assert R19's behaviour exactly:
- TestWorkerGenerationsFileIsWrittenBeforeTheFirstSpawn ("no hosts at
  the first spawn"), covered by TestWorkerSpawnPublishesItsBuildBeforeTheExec;
- TestWorkerGenerationsWriteFailureDoesNotBlockTheSpawnAndTheSweepRetries,
  covered by TestWorkerSpawnRefusedWhenItsBuildCannotBePublished.
TestWorkerGenerationsListOnlyVersionStoreBuilds' comment now says such
builds are listed by path; its assertion is unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude c0a23eab0b605cd91f937fb228deef5264d97b03 parent 85711f5
2 files changed, +11 −75
  • internal/activator/worker_generations_test.go +2 −62
  • internal/activator/worker_staged_test.go +9 −13
diff --git a/internal/activator/worker_generations_test.go b/internal/activator/worker_generations_test.go
index e276c8355..d5e54f7d5 100644
--- a/internal/activator/worker_generations_test.go
+++ b/internal/activator/worker_generations_test.go
@@ -46,31 +46,6 @@ func stringOf(m map[string][]string) string {
return b.String()
}
-// Ruling R17: the file exists, even empty, before any worker runs.
-func TestWorkerGenerationsFileIsWrittenBeforeTheFirstSpawn(t *testing.T) {
- a, b, _, _, _ := stagedActivator(t)
- path := release.GenerationsPath(t.TempDir())
- a.GenerationsFile = path
- seen := "spawn never ran"
- b.workerStartHook = func(registry.Route) {
- g, err := release.ReadGenerations(path)
- switch {
- case err != nil:
- seen = "unreadable: " + err.Error()
- case g == nil:
- seen = "absent"
- default:
- seen = "hosts=" + stringOf(g.Hosts)
- }
- }
- if err := a.WakeWorker(alwaysOnWorkerRoute("bot.x.internal")); err != nil {
- t.Fatalf("WakeWorker: %v", err)
- }
- if seen != "hosts=" {
- t.Fatalf("at the first spawn the generations file was %s; want it written, with no hosts", seen)
- }
-}
-
// Ruling R17: the file names a worker's captured build after its wake, keeps
// naming it across a crash recycle while live/<host> names another build,
// follows a converge, and drops it once the worker stops.
@@ -113,8 +88,8 @@ func TestWorkerGenerationsFileFollowsTheCapturedBuild(t *testing.T) {
}
// A captured build with no version, or a version name the store could never
-// hold, came from a link outside the version store: nothing restages it, so
-// it is not listed, and it never makes the file unwritable.
+// hold, is not listed by version, which would make the file unwritable; it
+// is listed by its staged path instead (ruling R19 item 2).
func TestWorkerGenerationsListOnlyVersionStoreBuilds(t *testing.T) {
a, _, _, _, ver := stagedActivator(t)
path := release.GenerationsPath(t.TempDir())
@@ -136,39 +111,6 @@ func TestWorkerGenerationsListOnlyVersionStoreBuilds(t *testing.T) {
}
}
-// Ruling R17: a write that fails never blocks a spawn, and the sweep retries
-// it.
-func TestWorkerGenerationsWriteFailureDoesNotBlockTheSpawnAndTheSweepRetries(t *testing.T) {
- a, b, _, _, _ := stagedActivator(t)
- data := filepath.Join(t.TempDir(), "data")
- if err := os.WriteFile(data, []byte("a file where the data dir should be"), 0o644); err != nil {
- t.Fatal(err)
- }
- path := release.GenerationsPath(data)
- a.GenerationsFile = path
- host := "bot.x.internal"
- if err := a.WakeWorker(alwaysOnWorkerRoute(host)); err != nil {
- t.Fatalf("WakeWorker with an unwritable generations file: %v", err)
- }
- if b.workerStarts() != 1 || b.lastWorker().Exited() {
- t.Fatalf("workerStarts = %d; a failed publication must not block the spawn", b.workerStarts())
- }
- if _, err := os.Stat(path); err == nil {
- t.Fatal("the generations file exists under a file; the test's failure did not happen")
- }
- a.sweepOnce() // still failing: retried and logged, nothing else
- if err := os.Remove(data); err != nil {
- t.Fatal(err)
- }
- if err := os.MkdirAll(data, 0o755); err != nil {
- t.Fatal(err)
- }
- a.sweepOnce()
- if got := readGenerations(t, path); !reflect.DeepEqual(got, map[string][]string{host: {"v1"}}) {
- t.Fatalf("after the sweep's retry the file names %v; want %s: [v1]", got, host)
- }
-}
-
// realWorkerBackend is the fake's lease, restore and replication around a
// REAL ExecBackend worker spawn: the child is a process exec'd by staged path,
// and a recycle runs release.VerifyStaged on real bytes.
@@ -403,7 +345,6 @@ func describeGenerations(gs []*release.Generations) string {
// installed, the captured build replaces the pending one.
func TestWorkerSpawnPublishesItsBuildBeforeTheExec(t *testing.T) {
a, b, clk, _, ver := stagedActivator(t)
- b.runBeforeExec = true
path := release.GenerationsPath(t.TempDir())
a.GenerationsFile = path
host := "bot.x.internal"
@@ -443,7 +384,6 @@ func TestWorkerSpawnPublishesItsBuildBeforeTheExec(t *testing.T) {
// refused build.
func TestWorkerSpawnRefusedWhenItsBuildCannotBePublished(t *testing.T) {
a, b, _, _, _ := stagedActivator(t)
- b.runBeforeExec = true
data := filepath.Join(t.TempDir(), "data")
if err := os.WriteFile(data, []byte("a file where the data dir should be"), 0o644); err != nil {
t.Fatal(err)
diff --git a/internal/activator/worker_staged_test.go b/internal/activator/worker_staged_test.go
index fe0ca501d..a1ee44bb4 100644
--- a/internal/activator/worker_staged_test.go
+++ b/internal/activator/worker_staged_test.go
@@ -26,11 +26,6 @@ type stagedFake struct {
// (onto != nil) returns instead of a child, once: the backend refusing a
// recycle.
failNextStaged error
- // runBeforeExec, when set, makes the fake keep ExecBackend's contract
- // with the activator's before-exec hook (ruling R19 item 3): it runs the
- // hook with the identity it is about to "exec", and a hook error means
- // no child. Off by default, so the R17 tests keep the spawn they pinned.
- runBeforeExec bool
}
func fakeIdentity(ver string) release.StagedIdentity {
@@ -53,14 +48,15 @@ func (b *stagedFake) StartWorkerStaged(ctx context.Context, rt registry.Route, e
if onto != nil && fail != nil {
return nil, release.StagedIdentity{}, fail
}
- if b.runBeforeExec {
- id := fakeIdentity(b.fakeBackend.liveVerNow())
- if onto != nil {
- id = *onto
- }
- if err := runBeforeExec(ctx, id); err != nil {
- return nil, release.StagedIdentity{}, err
- }
+ // ExecBackend's contract with the activator's before-exec hook (ruling
+ // R19 item 3): the hook runs with the identity about to be "exec'd", and
+ // a hook error means no child.
+ id := fakeIdentity(b.fakeBackend.liveVerNow())
+ if onto != nil {
+ id = *onto
+ }
+ if err := runBeforeExec(ctx, id); err != nil {
+ return nil, release.StagedIdentity{}, err
}
p, ver, err := b.fakeBackend.StartWorker(ctx, rt, env)
if err != nil {