Several tests in tests/e2e/messaging/performance.spec.ts do not exercise the behaviour their
names describe. The clearest:
test('T167: Pagination loads next 50 messages', async ({ browser }) => {
const viewer = await openAsViewer(browser, fixture!);
try {
await expectConversationLoaded(viewer);
// Wait for initial messages to load and stabilize.
await settleFrames(viewer.page);
} finally {
await viewer.close();
}
});It opens a conversation, asserts the conversation loaded, waits three animation frames, and
closes. It never paginates. The only assertion is one that any loaded conversation passes,
so the test is green whether pagination works, is broken, or is deleted.
Nine calls in that file are the last statement before viewer.close(), which is the signature:
the test ends on a wait rather than on a check.
Why this is worth fixing rather than shrugging at
This is the same family the repo keeps paying for, and there are three fresh examples from one
week: the regex guard that predicted #723 and stayed green through it; the
if (await jumpButton.isVisible()) wrapper that made T009 vacuous whenever the button was
missing — the single failure its name promised to catch; and three tests named for colour
contrast that never measured a ratio.
It matters more now than it did: e2e-local.yml runs on every PR (#575 Phase 3) and is
intended to become a required check. Tests that cannot fail dilute the signal that decision
rests on.
What to do
Go through the file test by test and ask what each name claims, then make it assert that:
- T167 — capture the rendered message count, trigger pagination (scroll to top / click the
loader), and assert the count grew by the expected page size.
- Jump to bottom with smooth scroll — it already guards on
if (jumpVisible), so it is
vacuous whenever the button is absent. Assert the button is present (as T009 now does), then
poll the scroll position, exactly as messaging-scroll.spec.ts T009 was fixed.
- Any test whose body ends in
settleFrames almost certainly ends without a check.
Where a behaviour genuinely cannot be exercised in this environment, test.fixme with a reason
is honest; a green test that asserts nothing is not.
Related
Several tests in
tests/e2e/messaging/performance.spec.tsdo not exercise the behaviour theirnames describe. The clearest:
It opens a conversation, asserts the conversation loaded, waits three animation frames, and
closes. It never paginates. The only assertion is one that any loaded conversation passes,
so the test is green whether pagination works, is broken, or is deleted.
Nine calls in that file are the last statement before
viewer.close(), which is the signature:the test ends on a wait rather than on a check.
Why this is worth fixing rather than shrugging at
This is the same family the repo keeps paying for, and there are three fresh examples from one
week: the regex guard that predicted #723 and stayed green through it; the
if (await jumpButton.isVisible())wrapper that made T009 vacuous whenever the button wasmissing — the single failure its name promised to catch; and three tests named for colour
contrast that never measured a ratio.
It matters more now than it did:
e2e-local.ymlruns on every PR (#575 Phase 3) and isintended to become a required check. Tests that cannot fail dilute the signal that decision
rests on.
What to do
Go through the file test by test and ask what each name claims, then make it assert that:
loader), and assert the count grew by the expected page size.
if (jumpVisible), so it isvacuous whenever the button is absent. Assert the button is present (as T009 now does), then
poll the scroll position, exactly as
messaging-scroll.spec.tsT009 was fixed.settleFramesalmost certainly ends without a check.Where a behaviour genuinely cannot be exercised in this environment,
test.fixmewith a reasonis honest; a green test that asserts nothing is not.
Related