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

disallow/warn if this.timeout() called multiple times within same test #1673

Closed
ianwremmel opened this issue Apr 26, 2015 · 6 comments
Closed
Labels
type: feature enhancement proposal

Comments

@ianwremmel
Copy link
Contributor

The following tests run to completion, but report they've timedout after period*4 ms. Is this a bug or is calling this.timeout() multiple times simply a bad idea?

'use strict';

// These are examples of how to do weird things with Mocha

describe.only('Mocha', function() {
  describe('#timeout()', function() {
    var start;

    beforeEach(function() {
      start = Date.now();
    });

    afterEach(function() {
      console.log('Duration:', Date.now() - start);
    });

    var period = 250;
    var duration = 3000;

    it('resets the test\'s timer everytime it is called (Promise syntax)', function() {
      var interval;
      var i = 0;
      interval = setInterval(function() {
        console.log('reset', i++);
        this.timeout(period*2);
      }.bind(this), period);

      return new Promise(function(resolve) {
        setTimeout(function() {
          console.log('resolve');
          clearInterval(interval);
          this.timeout(period*4);
          resolve();
        }.bind(this), duration);
      }.bind(this));
    });

    it('resets the test\'s timer everytime it is called (done syntax)', function(done) {
      var i = 0;
      var interval = setInterval(function() {
        console.log('reset', i++);
        this.timeout(period*2);
      }.bind(this), period);

      setTimeout(function() {
        console.log('done');
        clearInterval(interval);
        this.timeout(period*4);
        done();
      }.bind(this), duration);
    });
  });
});

Output:

  Mocha
    #timeout()
Mocha #timeout() resets the test's timer everytime it is called (Promise syntax)
reset 0
reset 1
reset 2
reset 3
reset 4
reset 5
reset 6
reset 7
reset 8
reset 9
reset 10
resolve
      1) resets the test's timer everytime it is called (Promise syntax)
Duration: 3005
Mocha #timeout() resets the test's timer everytime it is called (done syntax)
reset 0
reset 1
reset 2
reset 3
reset 4
reset 5
reset 6
reset 7
reset 8
reset 9
reset 10
done
      2) resets the test's timer everytime it is called (done syntax)
Duration: 3003


  0 passing (6s)
  2 failing

  1) Mocha #timeout() resets the test's timer everytime it is called (Promise syntax):
     Error: timeout of 1000ms exceeded. Ensure the done() callback is being called in this test.


  2) Mocha #timeout() resets the test's timer everytime it is called (done syntax):
     Error: timeout of 1000ms exceeded. Ensure the done() callback is being called in this test.
      at Context.<anonymous> (test/unit/spec/mocha.js:49:9)
@boneskull boneskull changed the title Calling this.timeout() multiple times in the same delays the timeout failure, but does not avoid it. disallow/warn if this.timeout() called multiple times within same test Apr 26, 2015
@boneskull
Copy link
Contributor

it's not intended to be called more than once. for a breaking change, in the future, I'd recommend throwing an exception if it's called more than once in the same test.

break it up over several tests...dynamically generate them if you need to.

@boneskull boneskull added this to the v3.0.0 milestone Apr 26, 2015
@boneskull
Copy link
Contributor

@mochajs/mocha open 4 discussion

@boneskull boneskull added the type: feature enhancement proposal label Apr 26, 2015
@ianwremmel
Copy link
Contributor Author

The case I had in mind is during test setup. I need to make an API call to create a test user but that call sometimes fails, so there's a retry built in. Let's say it takes on average 2 seconds to create a test user and I want to retry up to 3 times. Rather than setting my overall timeout to 3*2.2 (the 200ms is to account for the fact that 2 seconds is an average), I'd rather reset the timeout if the call fails. That way, I never consider more than 2.2 seconds acceptable for creating a test user.

So far, the best I've come up with is to call this.timeout(this.timeout() + 2000), but that doesn't have quite the same effect.

@boneskull
Copy link
Contributor

taking a step back, you may want to ask yourself is Mocha is the right tool for this. you're doing performance testing. while Mocha has some API points that can be used for this (timeout(), slow()), it hasn't really been considered a "core" use case. traditionally, Mocha's more of a functional/integration/unit testing tool.

that being said, I could see plugins implementing more features around this type of testing, but currently, support is slim.

@ianwremmel
Copy link
Contributor Author

I see your point, but mocha is definitely the right tool. My timeouts are already set pretty high; it's not so much that I'm using it for performance testing as keeping our server team honest :)

I think I came up with a solution that'll work, based on one of your comments: rather than repeated calling timeout, I'll dynamically generate set of before hooks:

(simplified example)

for(var i = 0; i < 3; i++) {
  var user
  before(function makeTestUser() {
    if (user) {
      return;
    }

    return createTestUser()
      .then(function() {
        user = u;
      });
  });

  before(function prepareTestUser() {
    if (user.prepared) {
      return;
    }
    return user.doSomethingAsyncToPrepareForTheTest()
      .then(function() {
        user.prepared = true;
      });
  });
}

it('does something', function() {
  return user.doSomethingAsync();
});

@boneskull boneskull removed this from the v3.0.0 milestone Jul 3, 2016
@boneskull
Copy link
Contributor

going to close unless this becomes highly desired

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature enhancement proposal
Projects
None yet
Development

No branches or pull requests

2 participants