Skip to content

Test hygiene in the RAW conversion suites: microtask counts, and three deferred minors #257

Description

@adulbrich

Four small gaps deferred during #248's review. None is a correctness bug; the first is the one that will actually cost someone an afternoon.

1. The deferredIo tests hard-code how many times rawToTiff awaits

src/lib/raw-preview.test.ts has several tests shaped like this:

await Promise.resolve();
await Promise.resolve();
io.start("/in/capt01.CR2");

Two ticks, because rawToTiff suspends on cacheKey and then on readFile before it reaches tiffFor. That count is an implementation detail of the function under test, encoded as a magic number in the test.

This already bit once during implementation: the plan specified one tick, io.start() became a silent no-op, and the tests hung to a 5 s Jest timeout rather than failing an assertion. A timeout gives you no information about what went wrong, which is what makes this worth fixing before it happens again.

Any change that adds or removes an await on the path into tiffFor will break these tests in exactly that opaque way. Fix: give deferredIo a whenCalled(path) that resolves when the double actually receives the call, and await that instead of counting ticks. Deterministic, and it stops encoding rawToTiff's internals in its tests.

2. A signal already aborted at call time has no test

convertRawInWorker checks options?.signal?.aborted at the front of the queue. If the signal was already aborted before the call, the same branch handles it correctly by inspection, but nothing covers it. Cheap to add.

3. An onStart callback that throws leaves the caller lied to

In src/lib/raw-worker-client.ts:

options?.onStart?.();
return send(path, bytes, wasmBaseUrl);

If onStart throws, the caller has been told the frame started but send() is never reached, so it never does. No consumer can trigger this today: the only onStart in the tree assigns a boolean. It is a latent trap for the next consumer rather than a live bug, and the ordering is deliberate (nothing throwable is allowed between the signal check and the callback), so any fix must preserve that.

4. A dead line in the accounting test

src/lib/raw-preview.test.ts:242 calls io.start(...) to no effect. clearRawPreviewCache has already emptied the map, so the guard short-circuits before started is ever read. Harmless, but it reads as though it is establishing a precondition that it is not, which is worse than not being there.

Priority

Item 1 is the one to do. The rest are tidy-ups worth picking up alongside the next change in this area.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions