denoland / deno_std

The Deno Standard Library
https://jsr.io/@std
MIT License
2.83k stars 581 forks source link

Deno test fails with inscrutable error when `assertSnapshot` is used in update mode but the `--allow-write` flag is missing #5155

Closed bcheidemann closed 1 day ago

bcheidemann commented 5 days ago

Describe the bug

Deno test fails with inscrutable error when assertSnapshot is used in update mode but the --allow-write flag is missing. This is hard to debug without knowledge of the inner workings of assertSnapshot.

Steps to Reproduce

  1. In a new folder, create the file main_test.ts with the following contents:
// main_test.ts
import { assertSnapshot } from "@std/testing/snapshot";

Deno.test("should match the snapshot", async (context) => {
  await assertSnapshot(context, "hello world!");
});
  1. Run deno test --allow-read -- -u and observe the output:
$ deno test --allow-read -- -u
running 1 test from ./main_test.ts
should match the snapshot ... ok (2ms)
Uncaught error from ./main_test.ts FAILED

 ERRORS 

./main_test.ts (uncaught error)
error: null
This error was not caught from a test and caused the test runner to fail on the referenced module.
It most likely originated from a dangling promise, event/timeout handler or top-level code.

 FAILURES 

./main_test.ts (uncaught error)

FAILED | 1 passed | 1 failed (5ms)

error: Test failed

Expected behavior

Ideally, a sensible error would be logged, such as:

$ deno test --allow-read -- -u
running 1 test from ./main_test.ts
should match the snapshot ... FAILED (0ms)

 ERRORS 

should match the snapshot => ./main_test.ts:4:6
error: AssertionError: The --allow-write flag must be passed when updating snapshots.
    throw new AssertionError(msg);
          ^
    at assert (https://jsr.io/@std/assert/1.0.0-rc.2/assert.ts:21:11)
    at ...

 FAILURES 

should match the snapshot => ./main_test.ts:4:6

FAILED | 0 passed | 1 failed (1ms)

error: Test failed

Environment

bcheidemann commented 5 days ago

This issue is likely happening because the permissions error is thrown in the "unload" callback, where the snapshot is opened. To avoid this, we can either ensure that such errors are logged properly by deno test and/or assert that the required permissions are given before registering the callback in assertSnapshot.

Happy to raise a PR for the latter fix. I'm not 100% sure where to start with the former, but also happy to raise a PR if pointed in the right direction.

iuioiua commented 5 days ago

I'm unable to reproduce the reported error on my machine. The behavior is as expected. By the way, it should throw a PermissionDenied error. Do you have any idea what the issue could be, @kt3k?

asher@Ashers-MacBook-Air x % deno test --allow-read -- -u
Check file:///Users/asher/Desktop/x/main_test.ts
running 1 test from ./main_test.ts
should match the snapshot ... FAILED (5ms)

 ERRORS 

should match the snapshot => ./main_test.ts:4:6
error: PermissionDenied: Requires write access to "/Users/asher/Desktop/x/__snapshots__", run again with the --allow-write flag
    await Deno.mkdir(dir, { recursive: true });
               ^
    at Object.mkdir (ext:deno_fs/30_fs.js:203:9)
    at ensureDir (https://jsr.io/@std/fs/1.0.0-rc.2/ensure_dir.ts:41:16)
    at eventLoopTick (ext:core/01_core.js:168:7)
    at async ensureFile (https://jsr.io/@std/fs/1.0.0-rc.2/ensure_file.ts:42:7)
    at async AssertSnapshotContext.#readSnapshotFile (https://jsr.io/@std/testing/0.225.3/snapshot.ts:402:7)
    at async AssertSnapshotContext.getSnapshot (https://jsr.io/@std/testing/0.225.3/snapshot.ts:471:23)
    at async assertSnapshot (https://jsr.io/@std/testing/0.225.3/snapshot.ts:585:20)
    at async file:///Users/asher/Desktop/x/main_test.ts:5:3
deno 1.44.4 (release, aarch64-apple-darwin)
v8 12.6.228.9
typescript 5.4.5
kt3k commented 5 days ago

I was able to reproduce the issue with the below steps:

$ deno test -A main_test.ts -- --update
running 1 test from ./main_test.ts
should match the snapshot ... ok (4ms)
------- post-test output -------

 > 1 snapshot updated.
----- post-test output end -----

ok | 1 passed | 0 failed (6ms)

$ deno test --allow-read main_test.ts -- --update
running 1 test from ./main_test.ts
should match the snapshot ... ok (2ms)
Uncaught error from ./main_test.ts FAILED

 ERRORS 

./main_test.ts (uncaught error)
error: null
This error was not caught from a test and caused the test runner to fail on the referenced module.
It most likely originated from a dangling promise, event/timeout handler or top-level code.

 FAILURES 

./main_test.ts (uncaught error)

FAILED | 1 passed | 1 failed (5ms)

error: Test failed

@bcheidemann

To avoid this, we can either ensure that such errors are logged properly by deno test and/or assert that the required permissions are given before registering the callback in assertSnapshot.

Happy to raise a PR for the latter fix.

The latter solution sounds good to me

bcheidemann commented 1 day ago

@kt3k thanks for expanding on the replication steps :)

To avoid this, we can either ensure that such errors are logged properly by deno test and/or assert that the required permissions are given before registering the callback in assertSnapshot.

Happy to raise a PR for the latter fix.

The latter solution sounds good to me

I have now implemented this in #5201