Skip to content

Commit f1d09b7

Browse files
heskewclaude
andcommitted
test(integration): address Gemini review — harden waitFor/awaitJob
Apply the three medium-priority suggestions from the gemini-code-assist review on PR #1233: - waitFor: wrap produce/until in try/catch, swallowing a transient failure (e.g. a momentary non-200 under CI contention) and retrying; re-throw only on the final attempt so a persistent failure still surfaces. Supersedes the earlier "keep it strict" choice — the retry-then-final-rethrow semantic is the better default for a reusable contention-riding poll helper. - awaitJob: optional-chain response.body?.[0]?.status to avoid a TypeError on a null/empty body. - row_count assertion: optional-chain r.body?.[0]?.row_count so a malformed body yields a clean assertion diff rather than a property-access throw. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 616f0df commit f1d09b7

2 files changed

Lines changed: 12 additions & 4 deletions

File tree

integrationTests/apiTests/northwind.test.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8760,7 +8760,7 @@ suite('Northwind operations', { skip: skipSuite }, (ctx) => {
87608760
.expect(200),
87618761
{ until: (res) => res.body?.[0]?.row_count === 30, timeoutSeconds: isBunRuntime ? 60 : 15 }
87628762
);
8763-
assert.equal(r.body[0].row_count, 30, r.text);
8763+
assert.equal(r.body?.[0]?.row_count, 30, r.text);
87648764
});
87658765

87668766
test.skip('Import CSV from S3 to table w/ full attr perms - update', async () => {

integrationTests/apiTests/utils/operations.mjs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export async function awaitJob(client, jobId, timeoutSeconds = 15) {
2828
let elapsed = 0;
2929
do {
3030
response = await client.req().send({ operation: 'get_job', id: jobId }).expect(200);
31-
const status = response.body[0]?.status;
31+
const status = response.body?.[0]?.status;
3232
// Stop only on a terminal status. A freshly-started job is briefly
3333
// CREATED (queued, before the worker flips it to IN_PROGRESS) and a
3434
// just-created job record can momentarily come back empty; returning on
@@ -87,6 +87,10 @@ export async function awaitJobCompleted(client, jobId, options = {}) {
8787
* an asynchronous side effect — the source of the fixed-delay races in #1222.
8888
*
8989
* @template T
90+
* A transient failure from `produce`/`until` (e.g. a momentary non-200 under CI
91+
* contention) is swallowed and retried; the error is re-thrown only if it
92+
* happens on the final attempt, so a persistent failure still surfaces.
93+
*
9094
* @param {() => Promise<T> | T} produce Produces the current value (e.g. runs a query).
9195
* @param {{ until: (value: T) => boolean, timeoutSeconds?: number, intervalMs?: number }} options
9296
* @returns {Promise<T>} the last produced value (satisfying `until`, or the final attempt on timeout)
@@ -95,8 +99,12 @@ export async function waitFor(produce, { until, timeoutSeconds = 30, intervalMs
9599
const attempts = Math.max(1, Math.ceil((timeoutSeconds * 1000) / intervalMs));
96100
let value;
97101
for (let attempt = 0; attempt < attempts; attempt++) {
98-
value = await produce();
99-
if (until(value)) return value;
102+
try {
103+
value = await produce();
104+
if (until(value)) return value;
105+
} catch (error) {
106+
if (attempt === attempts - 1) throw error;
107+
}
100108
if (attempt < attempts - 1) await setTimeout(intervalMs);
101109
}
102110
return value;

0 commit comments

Comments
 (0)