Put a broken module back at once, and excuse a wait only for a move that added an account group (hq issue 318 review)

cmd/mesh-controller/person_wait_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/novox/mesh-controller/internal/catalogue"
 	"github.com/novox/mesh-controller/internal/conditions"
 	"github.com/novox/mesh-controller/internal/inventory"
 	"github.com/novox/mesh-controller/internal/link"
@@ -76,23 +77,19 @@ func TestAWaitForAPersonIsAPassCarriedAlong(t *testing.T) {
 	now := time.Now()
 	since := now.Add(-time.Minute)
 	account := relogin("lights", "operator")
-	account.Since = since.Add(10 * time.Second) // the send put it in its wait
 	f := gateFacts{now: now, health: map[string]inventory.NodeHealth{"laptop": {Node: "laptop", HeardAt: now,
-		Resources: []inventory.ResourceHealth{account, userUnit("lights", "operator")}}}}
+		Resources: []inventory.ResourceHealth{account, userUnit("lights", "operator")}}},
+		groupsAdded: map[string]bool{"lights": true}}
 	if h, why := moduleHealthWord("lights", "laptop", since, f); h != healthPerson || !strings.Contains(why, "relogin needed on laptop") {
 		t.Fatalf("a wait for a new login reads %v %q; want a wait for a person", h, why)
 	}
-	// **Only the send that put the account in its wait is excused** (issue 318 review): an account waiting
-	// since before the send is judged as before, so a later build of the module is not passed on an old wait.
-	older := f.health["laptop"]
-	older.Resources = []inventory.ResourceHealth{relogin("lights", "operator"), userUnit("lights", "operator")}
-	older.Resources[0].Since = since.Add(-time.Hour)
-	f.health["laptop"] = older
+	// **Only a move that put the account in a new group is excused** (issue 318 review): a later build that
+	// adds no group did not bring the wait, and is judged as before.
+	f.groupsAdded["lights"] = false
 	if h, why := moduleHealthWord("lights", "laptop", since, f); h != healthNotYet {
-		t.Fatalf("a wait from before the send reads %v %q; want not yet", h, why)
+		t.Fatalf("a wait the move did not bring reads %v %q; want not yet", h, why)
 	}
-	f.health["laptop"] = inventory.NodeHealth{Node: "laptop", HeardAt: now,
-		Resources: []inventory.ResourceHealth{account, userUnit("lights", "operator")}}
+	f.groupsAdded["lights"] = true
 	h := f.health["laptop"]
 	h.Resources = append(h.Resources, inventory.ResourceHealth{Module: "lights", Resource: "lights.web", Kind: "container",
 		Target: "lights", State: link.StateUnhealthy, Reason: "down"})
@@ -273,6 +270,12 @@ func TestAConditionThatBecameTheOtherKindSaysSoWhenItClears(t *testing.T) {
 		for _, e := range told.Said() {
 			if e.Change == conditions.ChangeCleared {
 				resolved = append(resolved, e.Condition.Key+": "+e.Condition.Resolved)
+				// The clearing line is held to the plain rule too (ADR 0253).
+				w := conditions.Words{Headline: e.Condition.Headline, Explanation: e.Condition.Explanation,
+					Resolved: e.Condition.Resolved, Needs: e.Condition.Needs}
+				if why, ok := conditions.PlainWords(w, "laptop"); !ok {
+					t.Errorf("%s: its clearing words are not plain: %s", e.Condition.Key, why)
+				}
 			}
 		}
 	}
@@ -280,3 +283,30 @@ func TestAConditionThatBecameTheOtherKindSaysSoWhenItClears(t *testing.T) {
 		t.Fatalf("cleared %v; want the not-working condition cleared as now waiting for a login", resolved)
 	}
 }
+
+// Which moves add an account group, read from the builds' manifests (issue 318 review).
+func TestOnlyAMoveThatAddsAnAccountGroupCanBringAWait(t *testing.T) {
+	user := func(groups ...any) catalogue.Manifest {
+		return catalogue.Manifest{Module: "lights", Resources: []map[string]any{{"id": "operator", "type": "user",
+			"name": "operator", "groups": groups}}}
+	}
+	none := catalogue.Manifest{Module: "lights"}
+	for _, c := range []struct {
+		name     string
+		from     catalogue.Manifest
+		had      bool
+		to       catalogue.Manifest
+		addsSome bool
+	}{
+		{"a group added", none, true, user("lights"), true},
+		{"the same group kept", user("lights"), true, user("lights"), false},
+		{"a second group added", user("lights"), true, user("lights", "video"), true},
+		{"a group taken away", user("lights", "video"), true, user("lights"), false},
+		{"new to the machine, with a group", none, false, user("lights"), true},
+		{"new to the machine, no group", none, false, none, false},
+	} {
+		if got := addsAccountGroups(c.from, c.had, c.to); got != c.addsSome {
+			t.Errorf("%s: adds %v; want %v", c.name, got, c.addsSome)
+		}
+	}
+}