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

test_runner: Promise resolution is still pending but the event loop has already resolved #49952

Closed
fraxken opened this issue Sep 29, 2023 · 5 comments
Labels
test_runner Issues and PRs related to the test runner subsystem.

Comments

@fraxken
Copy link
Member

fraxken commented Sep 29, 2023

Version

v20.6.1

Platform

Microsoft Windows NT 10.0.19045.0 x64

Subsystem

node:test

What steps will reproduce the bug?

npm init -y
npm i @openally/timestore
touch index.mjs

Content of index.mjs file

// Import Node.js Dependencies
import assert from "node:assert";
import timers from "node:timers/promises";
import { test } from "node:test";
import { once, EventEmitter } from "node:events";

// Import Third-party Dependencies
import { TimeStore } from "@openally/timestore";

test("should use the provided EventEmitter to broadcast events", async() => {
  const eventEmitter = new EventEmitter();
  const expectedIdentifier = "foobar";

  const store = new TimeStore({
    ttl: 100, eventEmitter, keepEventLoopAlive: false
  });
  store.add(expectedIdentifier);

  const [identifier] = await once(store, TimeStore.Expired, {
    signal: AbortSignal.timeout(500)
  });
  assert.strictEqual(identifier, expectedIdentifier);
});

test("should do some other work", async() => {
  await timers.setTimeout(500);
  assert.strictEqual(1, 1);
});

Then run it node index.mjs (also work with node --test)
image

If you switch keepEventLoopAlive to true my package will not unref the internal timer and the event loop will remain active (and the test suite will be ok).

How often does it reproduce? Is there a required condition?

Always reproduce

What is the expected behavior? Why is that the expected behavior?

Should execute with no error for me (the event loop should remain alive).

What do you see instead?

If one test use once (which doesn't keep the loop alive), it return the error Promise resolution is still pending but the event loop has already resolved and propagate to all other tests (They all fail with the same error).

Additional information

I've encountered this problem on several projects over the past few weeks. One at work on a Fastify test suite where I had to declare a timer to keep the loop alive at the top:

let serverUnderTest: FastifyInstance;
let keepAliveTimer: NodeJS.Timer;

before(() => {
  keepAliveTimer = setInterval(() => void 0, 100_000);
  serverUnderTest = buildServer();
});

after(async() => {
  clearInterval(keepAliveTimer);
  await serverUnderTest.close();
});

I can't share this code as that not open source (looking to build a reproduction).
It test an API that use a class where once is used. I guess Fastify.inject doesn't keep the event loop alive too.

@cjihrig
Copy link
Contributor

cjihrig commented Sep 29, 2023

A couple of notes:

  • Promises alone will not keep the event loop alive, which is why this is failing. This is true for all Node programs, and Node's test runner treats every file as a generic Node application. (See Nodejs does not wait for promise resolution - exits instead #22088 and probably many other issues)
  • This is not specific to Node 20. It fails the same way on Node 18.
  • I ran your code using Deno's test runner, and it fails there as well.

It's not the most intuitive, but I'd say it's at least working as expected.

@Gonz-coding-co
Copy link

Gonz-coding-co commented Sep 29, 2023 via email

@mertcanaltin mertcanaltin added the test_runner Issues and PRs related to the test runner subsystem. label Sep 30, 2023
@bnoordhuis
Copy link
Member

Closing per Colin's comment.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Oct 5, 2023
@albertodiazdorado
Copy link

So how do I keep the event loop alive in my tests @cjihrig ?

@Powersource
Copy link

Powersource commented Mar 23, 2024

@albertodiazdorado i would imagine by awaiting all the promises in your test so the test doesn't close before all promises are done (edit: running into this again and this wasn't very helpful advice)

personally i ran into this error because i was doing await promisify(fn)(arg) but fn wasn't returning a callback, so i just removed the await and promisify

@nodejs nodejs deleted a comment from nicholaswmin Jul 24, 2024
@nodejs nodejs deleted a comment from nicholaswmin Jul 24, 2024
@nodejs nodejs locked as resolved and limited conversation to collaborators Jul 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

No branches or pull requests

7 participants