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

setTimeout() after done() is called (--watch mode) #3014

Closed
gabssnake opened this issue Sep 26, 2017 · 5 comments
Closed

setTimeout() after done() is called (--watch mode) #3014

gabssnake opened this issue Sep 26, 2017 · 5 comments
Labels
stale this has been inactive for a while...

Comments

@gabssnake
Copy link

Hello, thanks for your work on this project.

The following test passes when calling mocha from cli, but the assertion throws after the test passes when adding the --watch flag.

Changing setTimeout to process.nextTick makes the test always pass in both modes.
Changing the delay on setTimeout to lower values (10ms) makes the test pass sometimes in watch mode.

What is the expected behaviour?

var assert = require('assert');

describe('Works consistently in regular cli and watch mode', function() {
  it('ignores setTimeout after done is called', function(done) {
    function myFunc(callback) {
      callback('sync');
      setTimeout(function() { callback('async'); }, 100);
    }

    myFunc(function(data) {
      assert.equal(data, 'sync');
      done();
    });
  });
});

Kind regards,

mocha 3.5.3
node 6.11.2
macOS 10.12.6

@gabssnake gabssnake changed the title setTimeout after done() is called in watch mode setTimeout() after done() is called in watch mode Sep 26, 2017
@gabssnake gabssnake changed the title setTimeout() after done() is called in watch mode setTimeout() after done() is called (--watch mode) Sep 26, 2017
@ScottFreeCode
Copy link
Contributor

Calling done multiple times is an error, as the first done call is supposed to indicate to Mocha that the test is complete. It's possible for the second done call to never execute because Mocha shuts down too quickly before it can, especially if it's the last asynchronous test; --no-exit should prevent that, and I'd imagine --watch prevents it as a side effect as well. Not so sure about anything being changed by nextTick or a shorter timeout -- those should make the error more likely to be shown since the second done call occurs faster, but I'll play around with this a little and see if I discover anything. We do have issue #2879 for changing the default behavior to not shut down if there's anything still running and instead have --exit to get the old default behavior, although it'll have to wait for a semver "major" update.

@gabssnake
Copy link
Author

gabssnake commented Sep 28, 2017

Thanks for your answer.

Please see/try the following test case.

With --no-exit flag, test passes, it takes 10 secs to exit, no assertion is thrown.

With --watch flag, test passes, it takes 10 secs to throw: AssertionError: 'async3' == 'sync'. nextTick and shorter setTimeout never throw.

What is happening?

it('passes, then throws after 10 secs but only in --watch', function(done) {
  function myFunc(callback) {
    callback('sync');
    process.nextTick(function() { callback('async1'); });
    setTimeout(function() { callback('async2'); }, 0);
    setTimeout(function() { callback('async3'); }, 10000);
  }
  myFunc(function(data) {
    assert.equal(data, 'sync');
    done();
  });
});

@boneskull
Copy link
Member

good question.

@stale
Copy link

stale bot commented Feb 3, 2018

I am a bot that watches issues for inactivity.
This issue hasn't had any recent activity, and I'm labeling it stale. In 14 days, if there are no further comments or activity, I will close this issue.
Thanks for contributing to Mocha!

@stale stale bot added the stale this has been inactive for a while... label Feb 3, 2018
@stale stale bot closed this as completed Feb 17, 2018
@gabssnake
Copy link
Author

Time travelers: This issue has been resolved since mocha v4, released Sep 28, 2017.

The behavior is now consistent (always throws) in normal and watch mode, both with and without the --exit flag. Yay for consistency!

Thanks for the good work @boneskull and everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale this has been inactive for a while...
Projects
None yet
Development

No branches or pull requests

3 participants