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

Objects with cycles that fail assertions cause unhandled rejection, QUnit.done never gets called #1555

Closed
zackthehuman opened this issue Feb 14, 2021 · 1 comment · Fixed by #1577

Comments

@zackthehuman
Copy link
Contributor

zackthehuman commented Feb 14, 2021

Tell us about your runtime:

  • QUnit version: 2.14.0
  • Which environment are you using? (e.g., browser, Node): Node v14.15.4
  • How are you running QUnit? (e.g., script, testem, Grunt): npm test where test is qunit test.js

What are you trying to do?

I want to assert on objects that may contain cycles. This works OK when an assert passes, but when it fails it ends up killing the run due to an unhandled rejection. Most likely the cause is qunitjs/js-reporters#104. When the run dies due to unhandled rejection then QUnit.done is never called.

  • Is skipping QUnit.done expected behavior?

Code that reproduces the problem:

QUnit.begin(() => {
  console.log('QUnit.begin called.');
});

QUnit.done(() => {
  console.log('QUnit.done called.'); // Never gets called.
});

QUnit.test('cyclical failed assert blows up', (assert) => {
  const cyclical = {
    self: null
  };

  cyclical.self = cyclical;
  assert.notOk(cyclical);
});

What did you expect to happen?

I expect the assertion to fail, and the test to fail. Afterward I expect QUnit.done to be called.

What actually happened?

The assert and test fail but QUnit.done is never called because an unhandled rejection takes place. This is important to me because I need to run some teardown/cleanup code otherwise my process will keep node's event loop busy. I'm doing this cleanup in QUnit.done but since it isn't being called my process stays alive.

Consider a situation like this:

let keptAlive;

QUnit.begin(() => {
  console.log('QUnit.begin called.');
  keptAlive = setInterval(() => console.log('Ping-pong.'), 1000);
});

QUnit.done(() => {
  clearInterval(keptAlive);
  console.log('QUnit.done called.');
});

QUnit.test('cyclical failed assert blows up', (assert) => {
  const cyclical = {
    self: null
  };

  cyclical.self = cyclical;
  assert.notOk(cyclical);
});

The interval will never be cleared and so the process will "Ping-pong." forever. Should I be handling this kind of lifecycle another way? My current workaround is to handle the rejection myself:

process.on('unhandledRejection', () => {
  clearInterval(keptAlive);
  console.log('unhandledRejection handler called.');
});
@Krinkle
Copy link
Member

Krinkle commented Mar 13, 2021

Thanks, this should be fixed once we cut a js-reporters release for the TapReporter fix indeed. It might take a few weeks as there's a couple of other things that need to change in js-reporters before we can do another release there.

@Krinkle Krinkle self-assigned this Mar 13, 2021
Krinkle added a commit to Krinkle/qunit that referenced this issue Apr 4, 2021
Krinkle added a commit that referenced this issue Apr 9, 2021
Krinkle added a commit that referenced this issue Apr 9, 2021
Krinkle added a commit to Krinkle/qunit that referenced this issue Apr 9, 2021
Krinkle added a commit that referenced this issue Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants