Deno.test t.step incorrectly identifies leaky async ops

[andykais]

Deno.test tries to detect leaky async ops and error those out to the user. I believe I have found a case where deno outputs a false positive. The repro below will produce the following error

Deno.test({
  name: 'repro',
  fn: async t => {
    const interval_id = setInterval(() => {}, 1000)
    clearInterval(interval_id)
    await t.step('subtest', () => {})
  },
})

repro ...
  subtest ... FAILED (6ms)
repro ... FAILED (due to 1 failed step) (13ms)

 ERRORS

repro ... subtest => ./test/registry.test.ts:94:13
error: Leaking async ops:
  - 1 async operation to sleep for a duration was started before this test, but was completed during the test. Async operations should not complete in a test if they were not started in that test.
            This is often caused by not cancelling a `setTimeout` or `setInterval` call.

 FAILURES

repro ... subtest => ./test/registry.test.ts:94:13

FAILED | 0 passed | 1 failed (1 step) | (120ms)```

it is my understanding that the code is doing the right thing, cleaning up setInterval before `t.step`. One could also argue that test steps should not worry so much about async ops of a parent test, but in my particular case I do not need the async ops to carry over into the subtest. Mostly, I am just at a loss on how I am supposed to clean up resources before starting the subtest. If I am doing something wrong here too please let me know! Thanks

[wrnrlr]

There is indeed something weird going on, you can add {sanitizeOps:false, sanitizeResources:false} as options to the example above to fix the test error but it is weird to have to do this when this is not needed when there is no subtest.

Moreover I've run unto an example where even this won't help when using more then one subtest:

To run this you have to setup a postgres database like this because I coudn't find an easier way to reproduce it:

psql -C "create database test; create user test with password 'test'; grant all privileges on database test to test; grant all privileges on schema public to test;"

import pg from 'https://deno.land/x/postgresjs/mod.js'
Deno.test('db', {only:true,sanitizeOps:false,sanitizeResources:false}, async t=>{
  const sql = await pg({database:'test',username:'test',password:'test'});
  try {
    await sql.begin(async sql => {
      await sql`create table tmp(id serial primary key)`
      await sql`insert into tmp values (1)`
      t.step('select', async () => {await sql`select id from tmp`})
      t.step('delete', async () => {await sql`delete from tmp where id = 1`}) // comment this out and the test does pass!
      throw 'rollback'
    })
  } catch (e) {
    if (e!=='rollback') throw e
  }
})

[bartlomieju]

@dsherret is this expected behavior? If so we should update the documentation as there's not a single mention that sanitizers do not work with test steps.

[bartlomieju]

Discussed offline. Turns out this is a bug and it's not an expected behavior.

[wrnrlr]

Just an update, someone on discord mentioned that the step functions in my example need to be await'ed and indeed that solved at least the problem I wanted to report. I don't know about OP though.

Fixed example:

Deno.test('db', {only:true,sanitizeOps:false,sanitizeResources:false}, async t=>{
  const sql = await pg({database:'test',username:'test',password:'test'});
  try {
    await sql.begin(async sql => {
      await sql`create table tmp(id serial primary key)`
      await sql`insert into tmp values (1)`
      await t.step('select', async () => {await sql`select id from tmp`})
      await t.step('delete', async () => {await sql`delete from tmp where id = 1`}) // comment this out and the test does pass!
      throw 'rollback'
    })
  } catch (e) {
    if (e!=='rollback') throw e
  }
})

[andykais]

that is different than my issue. The code snippet in the initial message does await t.step

[elvitin]

I am also facing the same issue; I simulated this behavior in this small project https://github.com/vinc3re/the-leaking-problem/blob/main/README.md

[bartlomieju]

@mmastrac op sanitizer rewrite should be a stepping stone to address this problem.