--test-name-pattern not working for async describe suites #54084
Description
Version
v22.5.1
Platform
Darwin Kernel Version 23.4.0
Subsystem
test runner
What steps will reproduce the bug?
With a suite like the following:
// test.mjs
import { describe, test } from "node:test";
describe("async describe", async (t) => {
await test("some test");
});
describe("sync describe", () => {
test("some test");
});Run the tests with --test-name-pattern="some test":
$ node --test-name-pattern="some test" test.mjs
▶ sync describe
✔ some test (0.362916ms)
▶ sync describe (1.022417ms)
ℹ tests 1
ℹ suites 1
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 6.460416How often does it reproduce? Is there a required condition?
always
What is the expected behavior? Why is that the expected behavior?
I would expect both the test suites async describe and sync describe to work.
What do you see instead?
Only the sync describe suite seem to be working.
Additional information
By running the tests without any test name pattern, they all get executed:
$ node test.mjs
▶ async describe
✔ some test (0.477375ms)
▶ async describe (1.367375ms)
▶ sync describe
✔ some test (0.066833ms)
▶ sync describe (0.200708ms)
ℹ tests 2
ℹ suites 2
ℹ pass 2
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 7.35
Activity
jakecastelli commentedon Jul 29, 2024
The test with
awaitran but the reporter didn't collect it.This code example works prior to
v22.0- points me to #52221cc. @cjihrig do you mind taking a look? 👀
rstagi commentedon Jul 29, 2024
Thank you for the pointer @jakecastelli 🙏
While we wait for @cjihrig, I'll take a look myself too. I'll let you know if I find anything ⚙️
EddieAbbondanzio commentedon Jul 31, 2024
I took a look out of curiosity and it seems to be an async bug related to the logic that changes a suite from filtered to not filtered in
postBuildhere.When the example above is ran, the sync suite gets
filteredset tofalsebefore it's started, but the async suite doesn't getfilteredset tofalseuntil after it was started which causes the reporter to not collect it.@cjihrig would probably know best though.
cjihrig commentedon Jul 31, 2024
Yes, without testing it, I believe what @EddieAbbondanzio is correct. I think the correct fix is to wait for the suites to be finished building before beginning to execute anything.
cjihrig commentedon Aug 2, 2024
I've thought about this a bit, and I think waiting for the suites to complete is necessary, but not enough to fix this. I'm going to add another commit to #54011 that drops the returned promise from the top level
test()andsuite()APIs. The reason is that tests return a promise that resolves once the test runs. If we delay running tests until promises returned by tests resolve, we'll be waiting a long time.test_runner: finish build phase before running tests
test_runner: finish build phase before running tests