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

resolves #1300: passing 0 to timeout() will disable timeouts #1301

Merged
merged 1 commit into from
Aug 6, 2014

Conversation

boneskull
Copy link
Contributor

After 5dad9a3, it seems this.timeout(0) stopped working.

I re-enabled it inasmuch as I understood how it works, and added a test.

I was unsure whether or not @travisjeffery had intended to change the API, so that's why this is a PR.

@ksmithut
Copy link

ksmithut commented Aug 6, 2014

Any word on getting this merged? All my tests broke because of this timeout issue. I can set just a high timeout, but I prefer setting it to 0.

travisjeffery added a commit that referenced this pull request Aug 6, 2014
resolves #1300: passing 0 to timeout() will disable timeouts
@travisjeffery travisjeffery merged commit 473c439 into mochajs:master Aug 6, 2014
@ksmithut
Copy link

ksmithut commented Aug 6, 2014

@boneskull thanks!

@ksmithut
Copy link

ksmithut commented Aug 6, 2014

@boneskull this didn't seem to work....

describe('Mocha Test', function () {
  before(function (done) {
    this.timeout(0);
    setTimeout(done, 5000);
  });
  describe('sub tests', function () {
    it('should do something awesome', function () {
      return;
    });
  });
});

The above test when run with the new version of mocha still breaks with a timeout of 2000ms exceeded.

@travisjeffery
Copy link
Contributor

iirc before never supported this

@ksmithut
Copy link

ksmithut commented Aug 6, 2014

@travisjeffery it worked until just recently. If it's not meant to be supported, that's fine, but the documentation should be updated if that's the case.

@boneskull
Copy link
Contributor Author

@ksmithut: has not been released yet. did you try from master? just got merged.

On Aug 6, 2014, at 13:23, ksmithut notifications@github.com wrote:

@boneskull this didn't seem to work....

describe('Mocha Test', function () {
before(function (done) {
this.timeout(0);
setTimeout(done, 5000);
});
describe('sub tests', function () {
it('should do something awesome', function () {
return;
});
});
});
The above test run with the new version of mocha still breaks.


Reply to this email directly or view it on GitHub.

@ksmithut
Copy link

ksmithut commented Aug 7, 2014

Ah, I saw the package get published soon after this pull request was merged in (or close around), so I assumed it was included in the publish to 1.21.4. I'll give it another try.

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

Successfully merging this pull request may close these issues.

3 participants