Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deno test exits prematurely if a test empties the event loop #13146

Open
h4l opened this issue Dec 20, 2021 · 15 comments
Open

deno test exits prematurely if a test empties the event loop #13146

h4l opened this issue Dec 20, 2021 · 15 comments
Labels
bug Something isn't working correctly testing related to deno test and coverage

Comments

@h4l
Copy link

h4l commented Dec 20, 2021

If a test run via Deno.test() allows the event loop to empty, the test suite immediately fails without reporting the result of the test or any subsequent tests.

The failure message is also somewhat confusing. It seems to be technically accurate, and makes sense if you understand what's happened. But confusing when you don't realise you've accidentally emptied the event loop, allowing it to halt and are trying to work out why your test is failing (actually not executing).

As a user of the test API, I'd expect in this situation that the test would hang until a timeout was triggered to fail the test.

Maybe an argument for tests having timeouts (per #11133)? E.g. if the test runner had a pending timer for the timeout, the event loop couldn't become empty while a test was executing.

This test module demonstrates the problem:

// empty_event_loop_test.ts
import { assertEquals } from "https://deno.land/std@0.118.0/testing/asserts.ts";

function someApi(flag: boolean) {
  return new Promise((resolve) => {
    if (flag) {
      resolve("foo");
    } else if (!"bug") {
      resolve("bar");
    } else {
      console.log("* accidentally not resolving promise");
    }
  });
}

Deno.test("someApi returns foo when flag is true", async () => {
  const result = await someApi(true);
  assertEquals(result, "foo");
});

Deno.test(
  "[premature-exit]: deno test terminates the whole suite when a test empties the event loop",
  async () => {
    // event loop exits while awaiting someApi's promise
    console.log("\n\nBefore await");
    const result = await someApi(false);
    // this never executes because result is never resolved
    console.log("After await");
    assertEquals(result, "bar");
  },
);

Deno.test("[premature-exit]: simplified example", () => {
  return new Promise(() => {});
});

Deno.test("This is never executes if a premature-exit example does", () => {});

The tests stop executing when the first [premature-exit] test runs:

$ deno test empty_event_loop_test.ts
running 4 tests from file:///private/tmp/empty_event_loop_test.ts
test someApi returns foo when flag is true ... ok (8ms)
test [premature-exit]: deno test terminates the whole suite when a test empties the event loop ...

Before await

* accidentally not resolving promise


test result: FAILED. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out (35ms)

error: Promise resolution is still pending but the event loop has already resolved.

Final test executes as expected if no [premature-exit] tests run:

$ deno test empty_event_loop_test.ts --filter '/^someApi|never executes/'
running 2 tests from file:///private/tmp/empty_event_loop_test.ts
test someApi returns foo when flag is true ... ok (10ms)
test This is never executes if a premature-exit example does ... ok (6ms)

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 2 filtered out (164ms)

The simplified [premature-exit] also causes a premature exit:

$ deno test empty_event_loop_test.ts --filter '/\[premature-exit\]: simplified|never executes/'
running 2 tests from file:///private/tmp/empty_event_loop_test.ts
test [premature-exit]: simplified example ...
test result: FAILED. 0 passed; 0 failed; 0 ignored; 0 measured; 2 filtered out (30ms)

error: Promise resolution is still pending but the event loop has already resolved.
@stale
Copy link

stale bot commented Feb 19, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 19, 2022
@h4l
Copy link
Author

h4l commented Feb 19, 2022

It's still an issue, but should be fixed by the work in #11133.

@stale stale bot removed the stale label Feb 19, 2022
@bartlomieju bartlomieju added the bug Something isn't working correctly label Feb 20, 2022
@bartlomieju bartlomieju added the testing related to deno test and coverage label Apr 27, 2022
@krvajal
Copy link

krvajal commented Feb 6, 2024

Any idea when this will be fixed. It is two years old by now. I am happy to help as well

@bartlomieju
Copy link
Member

@mmastrac can you take a look when you find time? Seems related to your sanitizer work

@skybrian
Copy link

skybrian commented Feb 9, 2024

For people who searched for this bug, a workaround is to set your own timeout using setTimer:

  beforeEach(() => {
    setTimeout(() => {}, 1000);
  });

(Perhaps failing with a better error message.)

In my case, as soon as I did this, the test hung until it timed out, and I realized that a bug in the test was causing a deadlock.

@lucacasonato
Copy link
Member

What are you expecting to happen instead? There is no way the test will ever complete, as there is no task that could ever cause the test to finish.

@h4l
Copy link
Author

h4l commented Sep 18, 2024

@lucacasonato is your question for me or the previous commenter?

@skybrian
Copy link

@lucacasonato

For this bug, I think the specific test should fail and a good message would be something like “test waited on a promise that will never resolve.” It seems like a timeout, except that we can detect that the test will fail without waiting for the timeout.

If you mean what should happen when there’s a deadlock, some systems have deadlock detection, but I don’t know if that’s possible for JavaScript, so I would expect something like a timeout, frustrating though it can be to debug.

@lucacasonato
Copy link
Member

What you are running into is a form of deadlock prevention - if there is no work left to do, but your JS is saying "can't exit yet", we error.

We cannot terminate code in the middle of execution, and then start execution elsewhere later (like in a different test) - JS generally does not support this. So we have to treat this lockup as a fatal error in the test suite (file), like an uncaught exception, and thus terminate the entire test suite.

@skybrian
Copy link

skybrian commented Sep 19, 2024

Yes, it’s understandable that the test suite can’t continue, but it would still be nice to print the name of the individual test(s) that failed, somehow. That is, whichever tests started but didn’t finish when the deadlock happened.

This is speculative since I don’t know how it really works:

The timer trick seems like it might be useful as a way to make another event dispatch happen when the system is otherwise deadlocked. But it also prevents detecting the deadlock.

Maybe there needs to be a way to register an event that fires only if the event queue becomes empty. Or does it work that way already?

@JoobyPM
Copy link

JoobyPM commented Jan 29, 2025

I encounter a similar issue when using CompressionStream and DecompressionStream in Deno. My test cases involve streaming Gzip compression and decompression, and despite disabling sanitisers and adding a short delay (setTimeout), the error still occurs.

Minimal Reproducible Example
import { assertExists } from "jsr:@std/assert";

function createGzipCompressionStream(): {
  compressedReadable: ReadableStream<Uint8Array>;
  compressionWriter: WritableStreamDefaultWriter<Uint8Array>;
} {
  const compressionStream = new CompressionStream("gzip");
  return {
    compressedReadable: compressionStream.readable,
    compressionWriter: compressionStream.writable.getWriter(),
  };
}

Deno.test({
  name: "Basic usage of createGzipCompressionStream",
  sanitizeResources: false,
  sanitizeOps: false,
  fn: async () => {
    const { compressedReadable, compressionWriter } = createGzipCompressionStream();
    assertExists(compressedReadable);
    assertExists(compressionWriter);

    await compressionWriter.write(new TextEncoder().encode("Hello Gzip"));
    await compressionWriter.close();

    await new Promise((resolve) => setTimeout(resolve, 50)); // Ensure event loop is clear
  },
});
Test Command & Error Output
deno test stream_gzip_poc_test.ts
running 1 test from ./test/unit/stream_gzip_poc_test.ts
Basic usage of createGzipCompressionStream ...
ok | 0 passed | 0 failed (1ms)

error: Promise resolution is still pending but the event loop has already resolved

Observation

  • The error occurs even when all promises are awaited and sanitisers are disabled.
  • Adding a short delay does not always resolve the issue.
  • It seems related to how CompressionStream and DecompressionStream interact with the event loop.

Is there any recommended workaround or additional debugging step to confirm whether this is a Deno internals issue? Happy to provide more details if needed.

Thanks!

@BlackAsLight
Copy link

@JoobyPM if I was to guess, the problem may be that you're not consuming or cancelling the compressedReadable so it's just hanging around in memory.

@JoobyPM
Copy link

JoobyPM commented Jan 31, 2025

@BlackAsLight Nope, I tried all I knew, and I don't.

import { assertExists } from "jsr:@std/assert";

function createGzipCompressionStream(): {
  compressedReadable: ReadableStream<Uint8Array>;
  compressionWriter: WritableStreamDefaultWriter<Uint8Array>;
} {
  const compressionStream = new CompressionStream("gzip");
  return {
    compressedReadable: compressionStream.readable,
    compressionWriter: compressionStream.writable.getWriter(),
  };
}


Deno.test({
  name: "With consuming stream - also produce 'error: Promise resolution is still pending but the event loop has already resolved'",
  sanitizeResources: false,
  sanitizeOps: false,
  fn: async () => {
    const { compressedReadable, compressionWriter } = createGzipCompressionStream();
    assertExists(compressedReadable);
    assertExists(compressionWriter);

    await compressionWriter.write(new TextEncoder().encode("Hello Gzip"));
    await compressionWriter.close();

    // Consume the stream
    const reader = compressedReadable.getReader();
    while (true) {
      const { done, value } = await reader.read();
      if (done) break;
      console.log(value);
    }

    await new Promise((resolve) => setTimeout(resolve, 50)); // Ensure event loop is clear
  },
});

Result:

deno test tmp/stream_gzip_poc_test.ts

running 1 test from ./tmp/stream_gzip_poc_test.ts
With consuming stream - also produce 'error: Promise resolution is still pending but the event loop has already resolved' ...
ok | 0 passed | 0 failed (2ms)

error: Promise resolution is still pending but the event loop has already resolved

@BlackAsLight
Copy link

@JoobyPM
I ran into the error myself when testing the functionality of the pull request here denoland/std#6378

For me after calling .write() to it many times, one of them refused to resolve because I wasn't reading from the stream in parallel but series.

Switching the code around to read and write at the same time seemed to fix the issue, but I found it weird that it exists in the first place.

@h4l
Copy link
Author

h4l commented Feb 4, 2025

@JoobyPM @BlackAsLight your issue looks different to this issue. Your code is accidentally leaking a promise (whether through your own bug or the underlying library). The test runner is correctly warning you about this — it's behaving correctly. (This issue is about the test runner exiting without running all the tests.)

(Maybe try using the higher-level stream pipeThrough() API, as it's easy to not quite get things right when manually reading/writing streams: https://docs.deno.com/examples/piping_streams/ .)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly testing related to deno test and coverage
Projects
None yet
Development

No branches or pull requests

7 participants