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

[BUG] Asynchronously handled promises break things. #997

Open
3 tasks done
autopulated opened this issue Jan 30, 2024 · 0 comments
Open
3 tasks done

[BUG] Asynchronously handled promises break things. #997

autopulated opened this issue Jan 30, 2024 · 0 comments
Labels
bug something not go good

Comments

@autopulated
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Have you read the CONTRIBUTING guide on posting bugs, and CODE_OF_CONDUCT?

  • yes I read the things

This issue exists in the latest tap version

  • I am using the latest tap

Description

If the code being tested asynchronously handles rejected promises, tap seems to break. In some cases the PromiseRejectionHandledWarning from node also seems to be suppressed, which makes it difficult to understand why tests failed.

Reproduction

A case where the PromiseRejectionHandledWarning seems to be suppressed:

const t = require('tap');

async function slowFn(delay) {
    await new Promise((resolve) => {
        setTimeout(resolve, delay);
    });
};

async function slowFnThatRejects(delay, x) {
    await new Promise((resolve) => {
        setTimeout(resolve, delay);
    });
    throw new Error(`rejection value ${x}`)
}

async function doesWorkBeforeAwaitingPromise() {
    const p1 = slowFnThatRejects(1, 'p1');
    // without this second rejected promise we do get as far as the end of the test, but the test still fails
    const p2 = slowFnThatRejects(5, 'p2');

    await slowFn(20);

    // we never get this far
    console.log('never reached, but should be');
    await Promise.all([p1, p2]);
}

t.test('main test 1', async t => {
    await t.test('asynchronously handled promise rejection', async t => {
        t.rejects(doesWorkBeforeAwaitingPromise, new Error('rejection value p1'), 'should reject with p1 rejection value');
    });
});

// other tests get messed up too, for some reason (this made this tricky to isolate)
t.test('something else', async t => {
    await slowFn(50);
    t.ok('this should be ok');
});

A simpler case, where the warning is printed:

const t = require('tap');

async function slowFn(delay) {
    await new Promise((resolve) => {
        setTimeout(resolve, delay);
    });
};

async function slowFnThatRejects(delay, x) {
    await new Promise((resolve) => {
        setTimeout(resolve, delay);
    });
    throw new Error(`rejection value ${x}`)
}

async function doesWorkBeforeAwaitingPromise() {
    const p1 = slowFnThatRejects(1, 'p1');

    await slowFn(20);

    // we do get this far, but we don't return the correct result
    await p1;
}

t.test('main test 2', async t => {
    await t.test('asynchronously handled promise rejection', async t => {
        t.rejects(doesWorkBeforeAwaitingPromise, new Error('rejection value p1'), 'should reject with p1 rejection value');
    });
});

Environment

node --version
v20.11.0

npx tap versions
tap: 18.7.0
"@tapjs/config": 2.4.15
"@tapjs/core": 1.5.0
"@tapjs/run": 1.5.0
"@tapjs/stack": 1.2.7
"@tapjs/test": 1.4.0
tap-parser: 15.3.1
tap-yaml: 2.2.1
tcompare: 6.4.5
plugins:
  "@tapjs/after": 1.1.18
  "@tapjs/after-each": 1.1.18
  "@tapjs/asserts": 1.1.18
  "@tapjs/before": 1.1.18
  "@tapjs/before-each": 1.1.18
  "@tapjs/filter": 1.2.18
  "@tapjs/fixture": 1.2.18
  "@tapjs/intercept": 1.2.18
  "@tapjs/mock": 1.3.0
  "@tapjs/node-serialize": 1.3.0
  "@tapjs/snapshot": 1.2.18
  "@tapjs/spawn": 1.1.18
  "@tapjs/stdin": 1.1.18
  "@tapjs/typescript": 1.4.0
  "@tapjs/worker": 1.1.18

npx tap config list
# vim: set filetype=yaml :
color: true
coverage-report:
  - text
exclude:
  - "**/@(fixture*(s)|dist)/**"
include:
  - "**/@(test?(s)|__test?(s)__)/**/*.@(js|cjs|mjs|tap|cts|jsx|mts|ts|tsx)"
  - "**/*.@(test?(s)|spec).@(js|cjs|mjs|tap|cts|jsx|mts|ts|tsx)"
  - "**/test?(s).@(js|cjs|mjs|tap|cts|jsx|mts|ts|tsx)"
jobs: 9
reporter: base
snapshot-clean-cwd: true
timeout: 30

npx tap plugin list
@tapjs/after
@tapjs/after-each
@tapjs/asserts
@tapjs/before
@tapjs/before-each
@tapjs/filter
@tapjs/fixture
@tapjs/intercept
@tapjs/mock
@tapjs/node-serialize
@tapjs/snapshot
@tapjs/spawn
@tapjs/stdin
@tapjs/typescript
@tapjs/worker
@autopulated autopulated added the bug something not go good label Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something not go good
Projects
None yet
Development

No branches or pull requests

1 participant