messenger: count the store's writes, and name an ask's record when it is the only one held (review of issue 422)
mesh/merge-gate pass: builds messenger → novox; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)
mesh/delivery delivered
mesh/merge-gate pass: builds messenger → novox; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)
mesh/delivery delivered
The review's two nits. A write of bytes identical to what is stored was invisible to a comparison of content, so the test store counts its writes and the preview's tests read that count: a persist from inside Preview now fails five of them instead of one. And a router holding an ask's record and no condition said it held none, rather than saying what that key is; the branches are the other way round, with a test that holds nothing else.
This commit is contained in:
@@ -66,6 +66,8 @@ func (f *fakeChannel) Edit(id string, m Message) error {
|
|||||||
type memStore struct {
|
type memStore struct {
|
||||||
recs map[string]Record
|
recs map[string]Record
|
||||||
recent []Sent
|
recent []Sent
|
||||||
|
// puts counts the writes, so a write of bytes identical to what is stored is still seen (issue 422's review).
|
||||||
|
puts int
|
||||||
}
|
}
|
||||||
|
|
||||||
func (m *memStore) Put(r Record) error {
|
func (m *memStore) Put(r Record) error {
|
||||||
@@ -73,6 +75,7 @@ func (m *memStore) Put(r Record) error {
|
|||||||
m.recs = map[string]Record{}
|
m.recs = map[string]Record{}
|
||||||
}
|
}
|
||||||
m.recs[r.Key] = r
|
m.recs[r.Key] = r
|
||||||
|
m.puts++
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
func (m *memStore) Delete(k string) error { delete(m.recs, k); return nil }
|
func (m *memStore) Delete(k string) error { delete(m.recs, k); return nil }
|
||||||
|
|||||||
@@ -68,11 +68,14 @@ func (h *Holder) Preview(key, what string) (map[string]any, error) {
|
|||||||
if !holding || r.Origin != "condition" {
|
if !holding || r.Origin != "condition" {
|
||||||
sort.Strings(conditions)
|
sort.Strings(conditions)
|
||||||
switch {
|
switch {
|
||||||
case len(conditions) == 0:
|
case holding && len(conditions) == 0:
|
||||||
return nil, errorf("no condition %s is open here, and this router holds none: `mesh-controller.conditions` says what the controller holds open", key)
|
return nil, errorf("%s is not a condition but this router's own %s record, which is said as part of an ask, never on its own; it holds no condition",
|
||||||
|
key, r.Origin)
|
||||||
case holding:
|
case holding:
|
||||||
return nil, errorf("%s is not a condition but this router's own %s record, which is said as part of an ask, never on its own; it holds the conditions %s",
|
return nil, errorf("%s is not a condition but this router's own %s record, which is said as part of an ask, never on its own; it holds the conditions %s",
|
||||||
key, r.Origin, strings.Join(conditions, ", "))
|
key, r.Origin, strings.Join(conditions, ", "))
|
||||||
|
case len(conditions) == 0:
|
||||||
|
return nil, errorf("no condition %s is open here, and this router holds none: `mesh-controller.conditions` says what the controller holds open", key)
|
||||||
default:
|
default:
|
||||||
return nil, errorf("no condition %s is open here; it holds %s", key, strings.Join(conditions, ", "))
|
return nil, errorf("no condition %s is open here; it holds %s", key, strings.Join(conditions, ", "))
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -33,9 +33,9 @@ func previewHolder(t *testing.T, issues []TrackedIssue) (*Holder, *fakeChannel,
|
|||||||
// unmoved fails when anything a send would move has moved: the sends, the queue, the history, what is
|
// unmoved fails when anything a send would move has moved: the sends, the queue, the history, what is
|
||||||
// stored, and the record itself.
|
// stored, and the record itself.
|
||||||
type unmoved struct {
|
type unmoved struct {
|
||||||
sends, queued, recent, refusals int
|
sends, queued, recent, refusals, puts int
|
||||||
stored Record
|
stored Record
|
||||||
held Record
|
held Record
|
||||||
}
|
}
|
||||||
|
|
||||||
func mark(t *testing.T, h *Holder, tg, dt *fakeChannel, st *memStore, key string) unmoved {
|
func mark(t *testing.T, h *Holder, tg, dt *fakeChannel, st *memStore, key string) unmoved {
|
||||||
@@ -47,7 +47,7 @@ func mark(t *testing.T, h *Holder, tg, dt *fakeChannel, st *memStore, key string
|
|||||||
queued += len(list)
|
queued += len(list)
|
||||||
}
|
}
|
||||||
return unmoved{sends: len(tg.sends) + len(dt.sends), queued: queued, recent: len(h.recent),
|
return unmoved{sends: len(tg.sends) + len(dt.sends), queued: queued, recent: len(h.recent),
|
||||||
refusals: len(h.refusals), stored: st.recs[key], held: *h.open[key]}
|
refusals: len(h.refusals), puts: st.puts, stored: st.recs[key], held: *h.open[key]}
|
||||||
}
|
}
|
||||||
|
|
||||||
func (was unmoved) check(t *testing.T, h *Holder, tg, dt *fakeChannel, st *memStore, c *clock, key string) {
|
func (was unmoved) check(t *testing.T, h *Holder, tg, dt *fakeChannel, st *memStore, c *clock, key string) {
|
||||||
@@ -66,6 +66,9 @@ func (was unmoved) check(t *testing.T, h *Holder, tg, dt *fakeChannel, st *memSt
|
|||||||
if now.refusals != was.refusals {
|
if now.refusals != was.refusals {
|
||||||
t.Errorf("a preview recorded %d refusal(s): only a send does that", now.refusals-was.refusals)
|
t.Errorf("a preview recorded %d refusal(s): only a send does that", now.refusals-was.refusals)
|
||||||
}
|
}
|
||||||
|
if now.puts != was.puts {
|
||||||
|
t.Errorf("a preview wrote to the store %d time(s), even if the bytes were the same", now.puts-was.puts)
|
||||||
|
}
|
||||||
if !reflect.DeepEqual(now.stored, was.stored) {
|
if !reflect.DeepEqual(now.stored, was.stored) {
|
||||||
t.Errorf("a preview wrote the record: %+v", now.stored)
|
t.Errorf("a preview wrote the record: %+v", now.stored)
|
||||||
}
|
}
|
||||||
@@ -204,9 +207,19 @@ func TestAPreviewOfWhatThisRouterDoesNotHoldIsRefusedByName(t *testing.T) {
|
|||||||
h.open["ask.someone.1"] = &Record{Key: "ask.someone.1", Origin: "ask", Severity: Urgent, Summary: "a question"}
|
h.open["ask.someone.1"] = &Record{Key: "ask.someone.1", Origin: "ask", Severity: Urgent, Summary: "a question"}
|
||||||
h.mu.Unlock()
|
h.mu.Unlock()
|
||||||
_, err = h.Preview("ask.someone.1", "")
|
_, err = h.Preview("ask.someone.1", "")
|
||||||
if err == nil || !strings.Contains(err.Error(), "not a condition") {
|
if err == nil || !strings.Contains(err.Error(), "not a condition") || !strings.Contains(err.Error(), k.Key) {
|
||||||
t.Errorf("an ask's record previewed as a condition: %v", err)
|
t.Errorf("an ask's record previewed as a condition: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A router holding an ask and no condition says what that key is, not that it holds nothing.
|
||||||
|
bare, _, _, _, _ := newHolder(t)
|
||||||
|
bare.mu.Lock()
|
||||||
|
bare.open["ask.someone.2"] = &Record{Key: "ask.someone.2", Origin: "ask", Severity: Urgent, Summary: "a question"}
|
||||||
|
bare.mu.Unlock()
|
||||||
|
_, err = bare.Preview("ask.someone.2", "")
|
||||||
|
if err == nil || !strings.Contains(err.Error(), "not a condition") {
|
||||||
|
t.Errorf("the only record held is an ask's: %v", err)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestAPreviewAsksTheTrackerWhenItsWordWentCold(t *testing.T) {
|
func TestAPreviewAsksTheTrackerWhenItsWordWentCold(t *testing.T) {
|
||||||
|
|||||||
Reference in New Issue
Block a user