Skip to content

Commit

Permalink
chore: Try fix flakey public processor test (#11348)
Browse files Browse the repository at this point in the history
Tries fixing a flakey test from the public processor (example failed run
[here](https://github.com/AztecProtocol/aztec-packages/actions/runs/12864214502/job/35862843875#step:3:1373)).
This PR:

- Doubles all times in the flakey unit test, so small variations in CI
affect less
- Adds tests for the public processor and catches the edge case of
timeout being accidentally zero (and thus ignored)
- Removes operations inbetween the timeout being computed and the action
being kicked off
  • Loading branch information
spalladino authored Jan 20, 2025
1 parent bb96e7c commit 8de55d4
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 9 deletions.
20 changes: 20 additions & 0 deletions yarn-project/foundation/src/timer/timeout.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { sleep } from '../sleep/index.js';
import { executeTimeout } from './timeout.js';

describe('timeout', () => {
it('execs within timeout', async () => {
await expect(executeTimeout(() => sleep(200).then(() => 'ok'), 300)).resolves.toEqual('ok');
});

it('rejects with custom error on timeout', async () => {
await expect(executeTimeout(() => sleep(500), 200, 'Timed out!')).rejects.toThrow('Timed out!');
});

it('rejects if timeout is zero', async () => {
await expect(executeTimeout(() => sleep(500), 0, 'Timed out!')).rejects.toThrow('Timed out!');
});

it('rejects if timeout is negative', async () => {
await expect(executeTimeout(() => sleep(500), -100, 'Timed out!')).rejects.toThrow('Timed out!');
});
});
2 changes: 1 addition & 1 deletion yarn-project/foundation/src/timer/timeout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export class TimeoutTask<T> {
* @throws An error with a message indicating the function was interrupted due to exceeding the specified timeout.
*/
public async exec() {
const interruptTimeout = !this.timeout ? 0 : setTimeout(this.interrupt, this.timeout);
const interruptTimeout = setTimeout(this.interrupt, this.timeout);
try {
const start = Date.now();
const result = await Promise.race<T>([this.fn(), this.interruptPromise]);
Expand Down
6 changes: 3 additions & 3 deletions yarn-project/simulator/src/public/public_processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,12 @@ describe('public_processor', () => {

// The simulator will take 400ms to process each tx
publicTxSimulator.simulate.mockImplementation(async () => {
await sleep(400);
await sleep(800);
return mockedEnqueuedCallsResult;
});

// We allocate a deadline of 1s, so only one 2 txs should fit
const deadline = new Date(Date.now() + 1000);
// We allocate a deadline of 2s, so only 2 txs should fit
const deadline = new Date(Date.now() + 2000);
const [processed, failed] = await processor.process(txs, { deadline });

expect(processed.length).toBe(2);
Expand Down
11 changes: 6 additions & 5 deletions yarn-project/simulator/src/public/public_processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,17 +372,18 @@ export class PublicProcessor implements Traceable {
return await processFn();
}

const txHash = tx.getTxHash().toString();
const timeout = +deadline - this.dateProvider.now();
if (timeout <= 0) {
throw new PublicProcessorTimeoutError();
}

this.log.debug(`Processing tx ${tx.getTxHash().toString()} within ${timeout}ms`, {
deadline: deadline.toISOString(),
now: new Date(this.dateProvider.now()).toISOString(),
txHash: tx.getTxHash().toString(),
txHash,
});

if (timeout < 0) {
throw new PublicProcessorTimeoutError();
}

return await executeTimeout(
() => processFn(),
timeout,
Expand Down

0 comments on commit 8de55d4

Please sign in to comment.