Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions E2E_USER_STORIES.md
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,11 @@ runs `source` and `mosquitto` for this, and the specs drive state with a purpose
`drivers/place/e2e/state_harness.cr` (a UDP device whose `connected` and other statuses are
set through `exec`). See "MQTT" in `e2e/README.md`.

**The broker is asserted directly, not only through the page.** Stagehand re-checks every
MQTT alert over REST before showing it (`DashboardsService._updateDashboardAlertList`), which
hides wrong data on the broker. Removing that re-check — it is what makes stagehand slow — is
only safe once the MQTT rows below are all green.
**The broker is asserted directly, not only through the page.** Stagehand builds the hot list
from the broker's retained state alone: the latest value of each topic, with an emptied topic
removed. It used to re-check every alert over REST (a module list and a state read per alert, on
every message), which made it slow and hid wrong data on the broker; that went once the MQTT rows
below were green.

| ID | P | Story | Status |
|----|---|-------|--------|
Expand All @@ -479,7 +480,8 @@ only safe once the MQTT rows below are all green.
| SH-MQTT-19 | P0 | Two quick edits to one system leave source's mapping matching the latest. | **done** — the lifecycle setup makes them (two modules added back to back). pg-orm and placeos-resource handle each change on its own fiber, so source processed them out of order and kept the older module list. Found while building this coverage. |
| SH-MQTT-20 | P1 | After a broker restart, every status is published again. | **done, not automated** — verified by hand (restart mosquitto: 32 of 32 topics back within 2 s; before the fix, 0 until each value changed). |
| SH-MQTT-21 | P1 | Retained topics nothing maps to (left by a module deleted while source was down, or by an older source) are removed when source starts. | **done, not automated** — verified by hand with a planted topic. source spec `sweeps retained topics that nothing maps to`. |
| SH-HOT-02 | P1 | A module added to a system after stagehand has loaded it can still raise an alert. | **done** — `local/stagehand-hotlist.spec.ts`. Stagehand cached each system's module list forever (`_system_modules_cache`); it now refetches, at most every 5 s, before dropping an alert for an unknown device. |
| SH-HOT-02 | P1 | A module added to a system after stagehand has loaded it can still raise an alert. | **done** — `local/stagehand-hotlist.spec.ts`. Was a module list cached forever by the REST re-check, which is gone. |
| SH-HOT-03 | P0 | A deleted module leaves the hot list, the module after it is listed at its index, and the page makes no per-alert REST calls. | **done** — `local/stagehand-hotlist.spec.ts`. Red against the REST re-checking stagehand: an emptied topic failed to parse, so the deleted module stayed listed. |

**The lifecycle specs need source v1.22.1 or later** (PlaceOS/source#65). Against
`placeos-2.2610.1` SH-MQTT-10 … 19 and SH-HOT-02 are red, which is what they are for. Until a
Expand Down
73 changes: 66 additions & 7 deletions apps/stagehand/e2e/local/stagehand-hotlist.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* harness reports connected: false -> listed
* harness reports connected: true -> gone again
*
* The broker is asserted alongside the page at every step. When stagehand
* stops re-checking alerts over REST, this spec is what says the page is still
* right; until then, the broker assertions are what say the data is.
* The broker is asserted alongside the page at every step. Stagehand takes the
* hot list straight from the broker's retained state, with no REST checks, so
* these are what say the page follows the broker.
*
* "Not listed" needs a control (see e2e/README.md, Conventions): the page shows
* a spinner, not an empty table, until its first alert. So the system also
Expand All @@ -29,6 +29,7 @@ import { expect, test } from '../../../../e2e/support/stagehand/fixtures';
import {
addHarness,
createSystem,
deleteModule,
deleteSystem,
setConnected,
sweepSystems,
Expand Down Expand Up @@ -84,10 +85,6 @@ test('a module is listed while it reports connected: false, and only then', asyn
MQTT_PROPAGATION_MS,
);

// NOTE: both modules exist before the system's first alert. Stagehand
// caches a system's module list on its first alert and never refreshes
// it, so a module added after that cannot alert until a reload
// (SH-HOT-02). This spec is about the working path.
await setConnected(adminApi, control.id, false);
await broker.waitForValue(
'Projector',
Expand Down Expand Up @@ -194,3 +191,65 @@ test('SH-HOT-02: a module added after the page loaded the system can still be li
await deleteSystem(adminApi, system.id);
}
});

test('SH-HOT-03: a deleted module leaves the list, and the module after it takes its index, without REST checks', async ({
page,
adminApi,
stagehandAdminToken,
harnessDriverId,
zones,
}) => {
const system = await createSystem(adminApi, zones, 'delete-listed');
const broker = await BrokerView.open(
stagehandAdminToken,
systemFilter(zones.org, system.id),
);
// the per-alert checks stagehand used to make: a system's modules, and a
// module's state
const rest_checks: string[] = [];
page.on('request', (r) => {
const url = r.url();
if (
/\/api\/engine\/v2\/modules\?.*control_system_id=/.test(url) ||
new RegExp(`/api/engine/v2/systems/${system.id}/[^/?]+_\\d+`).test(
url,
)
) {
rest_checks.push(`${r.method()} ${url}`);
}
});
try {
await openHotList(page);

const first = await addHarness(adminApi, system.id, harnessDriverId);
const second = await addHarness(adminApi, system.id, harnessDriverId);
await waitForConnected(adminApi, first.id, true);
await waitForConnected(adminApi, second.id, true);
await setConnected(adminApi, first.id, false);
await setConnected(adminApi, second.id, false);
await expectHotList(page, system.id, ['Display_1', 'Display_2']);

// the second becomes Display_1; nothing is left at Display_2
await deleteModule(adminApi, first.id);
await expectHotList(page, system.id, ['Display_1']);

// and it is the second module listed there, so reconnecting it clears it
await setConnected(adminApi, second.id, true);
await broker.waitForValue(
'Display',
1,
'connected',
true,
MQTT_PROPAGATION_MS,
);
await expectHotList(page, system.id, []);

expect(rest_checks).toEqual([]);
} finally {
await test
.info()
.attach('broker messages', { body: broker.describeLog() });
await broker.close();
await deleteSystem(adminApi, system.id);
}
});
135 changes: 0 additions & 135 deletions apps/stagehand/src/app/alerts.service.ts

This file was deleted.

Loading
Loading