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

async iterator can cause silent process exit in Deno.test() #7088

Closed
lareau opened this issue Aug 17, 2020 · 4 comments · Fixed by #7968
Closed

async iterator can cause silent process exit in Deno.test() #7088

lareau opened this issue Aug 17, 2020 · 4 comments · Fixed by #7968
Assignees
Labels
bug Something isn't working correctly cli related to cli/ dir high priority

Comments

@lareau
Copy link

lareau commented Aug 17, 2020

The following script test.ts causes deno test to silently perform the equivalent of exit(0) from within the test fn():

$ deno test ./test.ts && echo OK
running 2 tests
test bug ... OK

All works as expected if the bug test is filtered out:

$ deno test ./test.ts --filter ok && echo OK
running 1 tests
test ok ... ok (3ms)

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

OK

My version info:

$ deno --version
deno 1.3.0
v8 8.6.334
typescript 3.9.7
$ grep DESC /etc/lsb-release
DISTRIB_DESCRIPTION="Ubuntu 20.04.1 LTS"
$ uname -s -p -v
Linux #46-Ubuntu SMP Fri Jul 10 00:24:02 UTC 2020 x86_64

Note: This is a minimal reproducer for a bug that showed up in a timeout-based iterater, thus the odd construction.

File test.ts:

class FooIterator implements AsyncIterableIterator<number> {
  resolve: any;

  constructor() {
    this.resolve = null;
  }

  [Symbol.asyncIterator](): AsyncIterableIterator<number> {
    return this;
  }

  next(): Promise<IteratorResult<number>> {
    return new Promise((res) => {
      this.resolve = res;
    });
  }
}

Deno.test({
  name: "bug",
  async fn() {
    const foo = new FooIterator();
    for await (const value of foo) {
      console.log(`value=${value}`);
    }
  },
});

Deno.test({
  name: "ok",
  async fn() {},
});
@bartlomieju bartlomieju added bug Something isn't working correctly cli related to cli/ dir labels Aug 27, 2020
@magurotuna
Copy link
Member

I'm investigating this issue.

@magurotuna
Copy link
Member

This is caused because neither resolve nor reject is not called in promise. The promise state is "pending" forever, so the subsequent steps won't be executed.

So true minimum reproducible would be:

Deno.test({
  name: "bug",
  fn() {
    return new Promise((_resolve, _reject) => {
      console.log("in promise");
      // Neither `resolve` nor `reject` is called
    });
  },
});

Deno.test({
  name: "ok",
  async fn() {
    console.log("ok test");
  },
});

The output looks like:

running 2 tests
test bug ... in promise

It would be better for users if the other tests could be executed regardless of one test having ever-pending promise, though simply detecting a promise that will be never resolved or rejected is absolutely difficult.

A possible solution I'm considering is

  • allow each test case to have its timeout period
  • If the promise state is still "pending" when the test has been timed out, then the test is reported as "TIMEOUT".

For example, timeout period would be set like this:

Deno.test({
  name: "bug",
  fn() {
    return new Promise((_resolve, _reject) => {
      console.log("in promise");
    });
  },
  timeout: 1000, // which is better, msec or sec...
});

Deno.test({
  name: "ok",
  async fn() {
    console.log("ok test");
  },
  // timeout should be optional, so we have to decide reasonable default value
});

Then the output would change to:

running 2 tests
test bug ... in promise
TIMEOUT (1000ms)
test ok ... ok test
ok (1ms)

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

I'd like to hear any opinions on my solution or this issue itself.

@bartlomieju
Copy link
Member

I did some debugging of this issue - creating a promise without ever resolving/rejecting it doesn't really cause the process to exit - it causes to stop execution of remaining JS code in the runtime without any error - load and unload event handlers are still triggered.

Maybe it's related to the fact that TestRunner (internal class used to run tests one by one) is an async iterator itself?

@bartlomieju
Copy link
Member

When #7946 lands this issue can be resolved by updating cli/test_runner.rs to call await Deno.runTests()

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 cli related to cli/ dir high priority
Projects
None yet
3 participants