test(blob): fix "sensor schema missing" flake in per-device-type sharding suite - #1750
Conversation
The per-device-type LMDB sharding suite restarts http_workers and then immediately asserts all three device schemas (thermostat/doorlock/sensor) are visible in describe_all. restartHttpWorkers only waits on the /openapi readiness probe, which returns 200 as soon as a worker is back up — before the component's schema.graphql has been parsed and all three tables registered and propagated to the metadata describe_all/REST read. That race intermittently fails with "sensor schema missing" (the last of the three) and cascading SensorBlob 404s. Poll describe_all until every device schema is visible before running the assertions. Returns immediately in the happy path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a helper function waitForDeviceSchemas in integrationTests/apiTests/blob.test.mjs to poll the describe_all operation until all required device schemas are visible, addressing an intermittent test flake. The review feedback suggests using optional chaining and error handling when parsing the HTTP response body within the polling loop to prevent potential TypeErrors from masking the actual server errors.
| let body = ''; | ||
| while (Date.now() < deadline) { | ||
| const r = await client.req().send({ operation: 'describe_all' }); | ||
| body = JSON.stringify(r.body); | ||
| if (required.every((frag) => body.includes(frag))) return; | ||
| await setTimeout(250); | ||
| } |
There was a problem hiding this comment.
When asserting on HTTP response bodies in tests, especially within polling or retry blocks, use optional chaining (e.g., r?.body) to prevent TypeErrors from masking the actual server error or response text when the server returns a non-JSON or empty response.
let body = '';
while (Date.now() < deadline) {
try {
const r = await client.req().send({ operation: 'describe_all' });
body = JSON.stringify(r?.body ?? {});
if (required.every((frag) => body.includes(frag))) return;
} catch (err) {
body = err.message;
}
await setTimeout(250);
}References
- Use optional chaining (e.g.,
res?.body?.message) when asserting on HTTP response bodies in tests, especially within polling or retry blocks. This prevents TypeErrors from masking the actual server error or response text when the server returns a non-JSON or empty response.
|
Reviewed; no blockers found. |
Problem
The Per-device-type LMDB database sharding suite in
integrationTests/apiTests/blob.test.mjsintermittently fails in CI with:followed by cascading
Table 'sensor.SensorBlob' does not exist/expected 200 "OK", got 404failures. Observed most recently on Integration Tests 3/6 (Node.js v22) — a single shard/runtime out of ~24, while every other shard and runtime passed, which is the classic flake signature.Root cause
All three device schemas (
thermostat,doorlock,sensor) are declared in oneschema.graphqlloaded by a single component, thenbefore()does:restartHttpWorkersonly waits on the/openapireadiness probe, which returns200the moment a worker is back up — that does not guarantee the component'sschema.graphqlhas been parsed and all three tables registered and propagated to the metadata thatdescribe_all/ REST read. The first test then asserts all three schemas in a single shot and races the tail of component load.sensor— the last of the three declared — is the one that loses the race, and the subsequentsensorblobREST calls 404 for the same reason.Fix
After the restart, poll
describe_alluntil all three device schemas are visible before running the assertions (30s budget, 250ms interval). Returns immediately in the happy path, so it adds no meaningful cost to normal runs — it only absorbs the propagation window that was previously assumed to be zero. Test-only change; no product code touched.This is the same "readiness poll instead of assumed timing" approach as #1091.
Verification
prettier@3.9clean,node --checkclean.🤖 Generated with Claude Code