diff --git a/internal/catalogue/operators_machine_test.go b/internal/catalogue/operators_machine_test.go index 3bcc9086..9dfa7b37 100644 --- a/internal/catalogue/operators_machine_test.go +++ b/internal/catalogue/operators_machine_test.go @@ -68,9 +68,10 @@ func TestFailedIsAnOptionalVerbOfTheServiceManager(t *testing.T) { holder := func(serves ...string) Manifest { return Manifest{Module: "systemd", Version: "1", Claims: []Claim{{Name: ServiceManagerSeat, Scope: ScopeNode, Serves: serves}}} } - eight := []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal"} + eight := []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal", + "reset-failed", "wanted-by", "unlink-dangling"} if err := CanHold(holder(eight...), seat); err != nil { - t.Fatalf("today's holder, without failed, is refused: %v", err) + t.Fatalf("a holder without failed is refused: %v", err) } if err := CanHold(holder(append(eight, "failed")...), seat); err != nil { t.Fatalf("a holder serving failed is refused: %v", err) @@ -81,7 +82,7 @@ func TestFailedIsAnOptionalVerbOfTheServiceManager(t *testing.T) { if err := CanHold(holder(eight[1:]...), seat); err == nil || !strings.Contains(err.Error(), "does not serve units") { t.Fatalf("a holder missing a required verb was accepted: %v", err) } - optional := map[string]bool{"failed": true, "reset-failed": true, "wanted-by": true, "unlink-dangling": true} + optional := map[string]bool{"failed": true} for _, v := range seat.Serves { if v.Optional != optional[v.Name] { t.Errorf("%s optional: %v", v.Name, v.Optional) @@ -89,26 +90,35 @@ func TestFailedIsAnOptionalVerbOfTheServiceManager(t *testing.T) { } } -// **`reset-failed` and `wanted-by` join the seat optional** (novox/hq issue 332, ADR 0246 step 1): the -// holder running today, serving the nine verbs, still holds the seat; a holder serving all eleven is not -// refused; each replaces the shell command it names; and read back from the store's row they stay optional. -func TestResetFailedAndWantedByAreOptionalVerbsOfTheServiceManager(t *testing.T) { +// **`reset-failed`, `wanted-by` and `unlink-dangling` are required** (novox/hq issue 332, ADR 0246 step 3): +// every holder serves them, so a holder serving only the nine verbs before them is refused by the verb it +// lacks; a holder serving all twelve holds the seat; each replaces the shell command it names; and read +// back from the store's row they stay required. +func TestResetFailedWantedByAndUnlinkDanglingAreRequiredVerbsOfTheServiceManager(t *testing.T) { defer UseSeats(DefaultSeats()) nine := []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal", "failed"} + all := append(slices.Clone(nine), "reset-failed", "wanted-by", "unlink-dangling") holder := func(serves ...string) Manifest { return Manifest{Module: "systemd", Version: "1", Claims: []Claim{{Name: ServiceManagerSeat, Scope: ScopeNode, Serves: serves}}} } + refusedWithout := func(seat Seat) { + t.Helper() + for _, verb := range []string{"reset-failed", "wanted-by", "unlink-dangling"} { + serves := slices.DeleteFunc(slices.Clone(all), func(v string) bool { return v == verb }) + if err := CanHold(holder(serves...), seat); err == nil || !strings.Contains(err.Error(), "does not serve "+verb) { + t.Errorf("a holder not serving %s holds the seat: %v", verb, err) + } + } + } seat, _ := SeatNamed(ServiceManagerSeat) - if err := CanHold(holder(nine...), seat); err != nil { - t.Fatalf("today's holder, without the two verbs, is refused: %v", err) + if err := CanHold(holder(nine...), seat); err == nil { + t.Fatal("a holder serving only the nine verbs before issue 332 holds the seat") } - if err := CanHold(holder(append(nine, "reset-failed", "wanted-by")...), seat); err != nil { - t.Fatalf("a holder serving both is refused: %v", err) + refusedWithout(seat) + if err := CanHold(holder(all...), seat); err != nil { + t.Fatalf("a holder serving all twelve is refused: %v", err) } - if err := CanHold(holder(append(nine, "reset-failed", "wanted-by", "unlink-dangling")...), seat); err != nil { - t.Fatalf("a holder serving unlink-dangling too is refused: %v", err) - } - if err := CanHold(holder(append(nine, "why-started")...), seat); err == nil || !strings.Contains(err.Error(), "does not promise") { + if err := CanHold(holder(append(slices.Clone(all), "why-started")...), seat); err == nil || !strings.Contains(err.Error(), "does not promise") { t.Fatalf("a verb the seat does not promise was accepted: %v", err) } replaces := map[string]string{"reset-failed": "systemctl reset-failed", "wanted-by": "systemctl list-dependencies --reverse", @@ -118,6 +128,9 @@ func TestResetFailedAndWantedByAreOptionalVerbsOfTheServiceManager(t *testing.T) if !ok { continue } + if v.Optional { + t.Errorf("%s is still optional", v.Name) + } if !slices.Contains(v.Replaces, want) { t.Errorf("%s does not say it replaces %q: %v", v.Name, want, v.Replaces) } @@ -148,8 +161,9 @@ func TestResetFailedAndWantedByAreOptionalVerbsOfTheServiceManager(t *testing.T) } UseSeats(rows) seat, _ = SeatNamed(ServiceManagerSeat) - if err := CanHold(holder(nine...), seat); err != nil { - t.Fatalf("read back from the row, the two verbs are required: %v", err) + refusedWithout(seat) + if err := CanHold(holder(all...), seat); err != nil { + t.Fatalf("read back from the row, a holder serving all twelve is refused: %v", err) } } @@ -169,8 +183,9 @@ func TestAnOptionalVerbStaysOptionalInASetReadFromTheStore(t *testing.T) { UseSeats(rows) seat, _ := SeatNamed(ServiceManagerSeat) holder := Manifest{Module: "systemd", Version: "1", Claims: []Claim{{Name: ServiceManagerSeat, Scope: ScopeNode, - Serves: []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal"}}}} + Serves: []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal", + "reset-failed", "wanted-by", "unlink-dangling"}}}} if err := CanHold(holder, seat); err != nil { - t.Fatalf("a set read from rows refuses today's holder: %v", err) + t.Fatalf("a set read from rows refuses a holder without the optional failed: %v", err) } } diff --git a/internal/catalogue/seats.go b/internal/catalogue/seats.go index c83aca05..ec9a9f50 100644 --- a/internal/catalogue/seats.go +++ b/internal/catalogue/seats.go @@ -678,13 +678,13 @@ func serviceManagerVerbs() []Verb { Replaces: []string{"systemctl --failed"}}, // **Clearing a failed record, and finding what starts a unit** (novox/hq issue 332): a unit whose // file is gone stays failed in its manager until the record is reset, and what keeps asking for it - // is a dependency or an enable link no verb could show. Both optional while their holders catch up - // (ADR 0246 step 1); a later change requires them. - {Name: "reset-failed", Optional: true, Description: "Clear one unit's failed record in its manager — " + + // is a dependency or an enable link no verb could show. Required since every holder serves them + // (ADR 0246 step 3). + {Name: "reset-failed", Description: "Clear one unit's failed record in its manager — " + "its failed state and its start-limit count — and answer its state after. It removes no file and " + "starts nothing.", Input: scoped(unit, []string{"unit"}), Replaces: []string{"systemctl reset-failed"}}, - {Name: "wanted-by", Optional: true, Description: "What wants, requires or triggers one unit, read-only: the " + + {Name: "wanted-by", Description: "What wants, requires or triggers one unit, read-only: the " + "manager's reverse dependencies (WantedBy, RequiredBy, UpheldBy, BoundBy, TriggeredBy and the rest) and " + "every enable link naming it in the scope's configuration directories (*.wants, *.requires, *.upholds), " + "each with where it points and whether it dangles — the account's own directory included in user scope.", @@ -692,9 +692,9 @@ func serviceManagerVerbs() []Verb { Replaces: []string{"systemctl list-dependencies --reverse", "systemctl show -p WantedBy", "ls ~/.config/systemd/user/*.wants"}}, // **A dangling enable link removed on the person's word** (novox/hq issue 332): `systemctl disable` - // leaves a link whose unit file is gone, so the manager keeps asking for the unit. Optional until its - // holders serve it (ADR 0246 step 1). - {Name: "unlink-dangling", Optional: true, Description: "Remove the dangling enable links named for one " + + // leaves a link whose unit file is gone, so the manager keeps asking for the unit. Required since + // every holder serves it (ADR 0246 step 3). + {Name: "unlink-dangling", Description: "Remove the dangling enable links named for one " + "unit — links in a *.wants, *.requires or *.upholds directory of the account's or the machine's own " + "configuration whose target does not exist, which `disable` leaves once the unit's file is gone — on " + "the person's word: `why` is required. Each is recorded (its path, its target, the reason) before it is " +