test(e2e): give the job specs their own state back
Both specs stage a job through `/__test/emit` and neither cleared it. Nothing resets those stores, so the spec that staged it is the one that should put it back, and `JobsChanged` with `[]` is the whole cleanup -- `JobStore` replaces its list from every snapshot, so `testctl` needs no special case. **The leak as reported did not reproduce, and that is worth recording rather than quietly fixing.** Measured with a temporary probe: a positive control confirmed a staged job really does move the shell at a phone width (`job-band` renders a row, `.main-panel`'s top goes 0 to 55), and the very next page had no job at all. The reason is that every test gets a fresh page and `JobStore.init()` refetches `GetJobs()` from a backend registry `/__test/emit` never writes to -- it calls `events.Deliver`, which touches frontends and no state. So the state cannot cross a spec boundary as described, and the 55px offset the draft assertion saw in that suite run has another cause that is not in evidence. The cleanup stays, because it costs a line and the leak would need only one spec that keeps a page alive, and the comments say what was measured rather than asserting the mechanism. The durable half is the rule, now in the harness reference: measure against the element next to you, not an absolute coordinate. An absolute number in a shell measurement is also a claim about everything above it -- `contentTop === 0` asserts "and no background job is running", which that spec could not arrange. Closes #168
This commit is contained in:
@@ -57,6 +57,21 @@ behind `YJ_TESTCTL=1`, which `scripts/dev-headless.sh` sets and
|
|||||||
staging the work that would produce it — job progress, download
|
staging the work that would produce it — job progress, download
|
||||||
progress, scan progress. It calls `events.Deliver`, which *errors*
|
progress, scan progress. It calls `events.Deliver`, which *errors*
|
||||||
when the event reaches nobody, so a `200` means it really arrived.
|
when the event reaches nobody, so a `200` means it really arrived.
|
||||||
|
- **State you stage, you own** (#168). Nothing resets those stores, so
|
||||||
|
clear yours in `test.afterEach` with the same event that staged it
|
||||||
|
(`emit('JobsChanged', [])`) — the store replaces its list from every
|
||||||
|
snapshot, so `testctl` needs no special case. **Measured: this does
|
||||||
|
not currently cross a spec boundary**, because every test gets a fresh
|
||||||
|
page and `JobStore.init()` refetches `GetJobs()` from a backend
|
||||||
|
registry that `/__test/emit` never writes to. Stated anyway, because
|
||||||
|
it costs one line and the leak needs only one spec that keeps a page
|
||||||
|
alive — but do not cite #168 for a symptom you have not reproduced.
|
||||||
|
- **Measure against the thing next to you, not an absolute
|
||||||
|
coordinate.** An absolute number in a shell measurement is also a
|
||||||
|
claim about everything above it — `contentTop === 0` quietly asserts
|
||||||
|
"and no background job is running", which is not what that spec was
|
||||||
|
about or could arrange, while `contentTop === jobBandBottom` is true
|
||||||
|
either way. This is the half of #168 that stands on its own.
|
||||||
- **`restore` is slow** (~40 s in the suite) because it copies every
|
- **`restore` is slow** (~40 s in the suite) because it copies every
|
||||||
table. Prefer snapshotting once and restoring only when a spec
|
table. Prefer snapshotting once and restoring only when a spec
|
||||||
genuinely mutates state.
|
genuinely mutates state.
|
||||||
|
|||||||
@@ -60,6 +60,23 @@ const PHONE = { width: 424, height: 439 };
|
|||||||
const DESKTOP = { width: 1100, height: 800 };
|
const DESKTOP = { width: 1100, height: 800 };
|
||||||
|
|
||||||
test.describe('background jobs on a phone', () => {
|
test.describe('background jobs on a phone', () => {
|
||||||
|
/**
|
||||||
|
* **State a spec stages is the spec's to clear.** `/__test/emit` writes
|
||||||
|
* to a store nothing resets, so the event that staged a job is the
|
||||||
|
* event that clears it — `JobStore` replaces its whole list from every
|
||||||
|
* snapshot, so `testctl` needs no special case.
|
||||||
|
*
|
||||||
|
* **Measured on #168: this does not currently outlive the page.** Every
|
||||||
|
* test gets a fresh page, and `JobStore.init()` refetches `GetJobs()`
|
||||||
|
* from a backend registry that `/__test/emit` never writes to, so the
|
||||||
|
* staged job is gone before the next spec starts. Ownership is stated
|
||||||
|
* rather than a live leak repaired — the leak needs a page that
|
||||||
|
* survives its own spec, and there is none today.
|
||||||
|
*/
|
||||||
|
test.afterEach(async ({ testctl }) => {
|
||||||
|
await testctl.emit('JobsChanged', []);
|
||||||
|
});
|
||||||
|
|
||||||
test('are shown in the band, without opening anything', async ({
|
test('are shown in the band, without opening anything', async ({
|
||||||
app,
|
app,
|
||||||
testctl,
|
testctl,
|
||||||
|
|||||||
@@ -102,6 +102,22 @@ const collapsed = (page: Page) =>
|
|||||||
}));
|
}));
|
||||||
|
|
||||||
test.describe('the top bar fits the window', () => {
|
test.describe('the top bar fits the window', () => {
|
||||||
|
/**
|
||||||
|
* **State a spec stages is the spec's to clear** (#168). `/__test/emit`
|
||||||
|
* writes to a store nothing resets, and this file stages the widest job
|
||||||
|
* in the app, so it puts it back — with the same event, since the store
|
||||||
|
* replaces its whole list from every snapshot.
|
||||||
|
*
|
||||||
|
* **Measured: it does not currently outlive the page.** Every test gets
|
||||||
|
* a fresh page and `JobStore.init()` refetches `GetJobs()` from a
|
||||||
|
* backend registry `/__test/emit` never writes to, so nothing is being
|
||||||
|
* repaired here; the rule is stated because it costs one line and the
|
||||||
|
* leak would need only one spec that keeps a page alive.
|
||||||
|
*/
|
||||||
|
test.afterEach(async ({ testctl }) => {
|
||||||
|
await testctl.emit('JobsChanged', []);
|
||||||
|
});
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The phone's answer, which is not "it fits" (#57).
|
* The phone's answer, which is not "it fits" (#57).
|
||||||
*
|
*
|
||||||
|
|||||||
Reference in New Issue
Block a user