deno resource leak test false positive when using filter

[loynoir]

Version: Deno 1.46.0

bug

deno resource leak test false positive when using filter

reproduce

$ deno test -A ./x.ts 
running 2 tests from ./x.ts
a ... ok (11ms)
b ... FAILED (6ms)

 ERRORS 

b => ./x.ts:12:6
error: Leaks detected:
  - A child process was started during the test, but not closed during the test. Close the child process by calling `proc.kill()` or `proc.close()`.
  - An async operation to wait for a subprocess to exit was started in this test, but never completed. This is often caused by not awaiting the result of a `Deno.Process#status` call.
To get more details where leaks occurred, run again with the --trace-leaks flag.

 FAILURES 

b => ./x.ts:12:6

FAILED | 1 passed | 1 failed (19ms)

error: Test failed

actual

deno resource leak test false positive when using filter

$ deno test -A --filter b ./x.ts 
running 1 test from ./x.ts
b ... ok (14ms)

ok | 1 passed | 0 failed | 1 filtered out (15ms)

expected

$ deno test -A --filter b ./x.ts 
(complain resource leak when using test filter)

additional

Found this bug when https://github.com/evanw/esbuild/issues/3896

[marvinhagemeister]

Smaller reproduction:

File foo_test.ts:

import * as child_process from "node:child_process";

const worker = "foo.ts";

Deno.test("a", async () => {
  const cp = child_process.spawn(Deno.execPath(), [worker], {});
  await new Promise((r) => setTimeout(r, 200));
  cp.kill();
});

Deno.test("b", async () => {
  const cp = child_process.spawn(Deno.execPath(), [worker], {});
  await new Promise((r) => setTimeout(r, 200));
  cp.kill();
});

File foo.ts:

await new Promise((r) => setTimeout(r, 9999));

[lucacasonato]

The reproduction you provided is not correct @marvinhagemeister. The reported leak is genuine. You are just not waiting for the process to actually exit after calling cp.kill. You have to wait for the "exit" event to fire on the sub process.

This is because subprocesses do not die synchronously on Unix. Calling cp.exit just sends the subprocess a signal (like SIGTERM) that the sub process can do with whatever it wants. For SIGTERM for example, most processes would shut down pretty quickly. However, that is not guaranteed. A process can just ignore the signal you send it.

Because of that you always have to wait for the process to actually exit. And with node:child_process you do that by waiting for the exit event.

[marvinhagemeister]

Related https://github.com/evanw/esbuild/pull/3701

[kt3k]

I think the cases leak ops flakily and that is more likely happen when the 2 cases run together. I don't think this is an issue of filtering.

I was able to reproduce the issue with the filter enabled:

/tmp/reproduce $ deno test -A --filter=a x.ts
running 1 test from ./x.ts
a ... FAILED (27ms)

 ERRORS 

a => ./x.ts:5:6
error: Leaks detected:
  - A child process was started during the test, but not closed during the test. Close the child process by calling `proc.kill()` or `proc.close()`.
  - An async operation to wait for a subprocess to exit was started in this test, but never completed. This is often caused by not awaiting the result of a `Deno.Process#status` call.
To get more details where leaks occurred, run again with the --trace-leaks flag.

 FAILURES 

a => ./x.ts:5:6

FAILED | 0 passed | 1 failed (28ms)

error: Test failed