Allow to retry failed test from test context for #1773 #1989

longlho · 2015-11-30T20:09:41Z

make retries run proper hooks
allow retries override at different levels

danielstjules · 2015-12-01T09:36:30Z

lib/suite.js

@@ -108,6 +110,25 @@ Suite.prototype.timeout = function(ms) {
 };

 /**
+ * Set timeout `ms` or short-hand such as "2s".


Doc comments need updating?

danielstjules · 2015-12-01T09:39:52Z

Looks like you may have accidentally deleted mocha.js as part of that commit? Could you amend and restore it?

longlho · 2015-12-01T16:59:59Z

@danielstjules my bad :) commit ammended

tinganho · 2015-12-03T08:08:09Z

lib/mocha.js

+ * @param {number} retry times
+ * @return {Mocha}
+ */
+Mocha.prototype.retires = function(retries) {


Typo here retires -> retries.

Also the parameter name should be n? At least it is on your other methods.

ah my bad, fixed

longlho · 2015-12-05T21:01:20Z

ping @boneskull @danielstjules

danielstjules · 2015-12-09T17:09:31Z

test/integration/fixtures/retries/early-pass.js

@@ -0,0 +1,11 @@
+describe('retries', function() {
+  this.retries(4);


this.retries(1) probably makes sense here?

danielstjules · 2015-12-09T17:10:05Z

Great job on the tests :)

longlho · 2015-12-10T05:26:03Z

@danielstjules tks :) changes made

danielstjules · 2015-12-10T05:30:15Z

Thanks! And can you git checkout 67d0facb5153feb7db4d8e161778436a75117189 mocha.js && git commit --amend Not sure why it's got a diff :)

longlho · 2015-12-10T05:38:41Z

done :)

danielstjules · 2015-12-24T18:23:41Z

Playing around with this branch today and will let you know if I have anymore feedback :)

danielstjules · 2015-12-25T15:46:18Z

lib/mocha.js

+ * Set the number of times to retry failed tests.
+ *
+ * @param {Number} retry times
+ * @return {Mocha}


Duplicate annotations here

ah, I fixed that, but turns out there're actually a bunch of other methods w/ dupe annotations (timeout, enableTimeout...)

danielstjules · 2015-12-25T16:04:35Z

Comments aside, the feature looks good to me. :) It behaves exactly as I'd expect:

afterEach hooks are ran after test failures, before a subsequent retry
Despite multiple retries, tests are only emitted once to reporters
.retries() precedence makes sense: cli < suite < test. As an example, this.retries(3); within a test body will take precedence over this.retries(1); at the suite level, or any --retries n cli arg
Works well with --bail

Integration tests validate that hook behavior is correct with retries. Unless @mochajs/mocha has any objections, I hope to merge this after the comments are fixed. I think it'll fit nicely with a new minor release! :)

tinganho · 2015-12-25T16:48:08Z

I think mutating the tests variable in here is quite bad. Just think about it, an outside developer now need to learn this special rule. Although, IMHO this is an "ugly hack" to support retries.

danielstjules · 2015-12-25T16:54:23Z

Just think about it, an outside developer now need to learn this special rule.

What special rule? A developer just needs to read runTests, and it's clear: var tests = suite.tests.slice(); https://github.com/longlho/mocha/blob/1773-retries/lib/runner.js#L436 It should be quite evident that it's a shallow copy of the suite.tests array :)

- make retries run proper hooks - allow retries override at different levels - expose currentRetry to reporters

longlho · 2015-12-28T02:17:07Z

@danielstjules changes made :)

danielstjules · 2015-12-28T16:32:26Z

Awesome! Thanks again for all your work on this! I'm sure a lot of people are looking forward to using it :) Should be pretty helpful for browser-based acceptance test suites.

Allow to retry failed test from test context for #1773

tinganho · 2016-01-06T13:56:05Z

Does this support retrying beforeEach and afterEach hooks too?

I have tests that does HTTP calls in beforeEach hook and afterEach hook and they stall sometimes.

tinganho · 2016-01-06T13:57:42Z

Also maybe try to retry after and before hooks as well. I have some HTTP calls there too that have got stalled.

danielstjules · 2016-01-06T15:02:15Z

Does this support retrying beforeEach and afterEach hooks too?

No, just tests, and I'd personally be opposed to the idea of retries for hooks. Hooks serve as setup/teardowns for tests, and can thus be used to cleanup before/after a failed test. For failed hooks (afterEach, after), you can't be sure of the state when retrying. If you have brittle logic in hooks, I'd suggest making a helper function and moving it to be invoked from the specs.

ajaks328 · 2016-02-10T05:26:44Z

Awesome! Just saw that this work has been merged and released. Been looking forward to this one. Thanks guys.

aaronabramov · 2016-02-24T01:22:49Z

regarding retries in before and after hooks.
would an API like this work?

afterEach(function() {
  if(this.isThereAnyErrorsInTheConsole()) {
    this.retry(); // fails the test and retries it again
  }
});

that's just an explicit retry call that doesn't happen automatically.

in my project, we check for react propTypes and other stuff validation errors after the test passed.
When running e2e tests it becomes very flaky, mostly because of some random errors (like an image didn't load, or some third party ads library freaked out and threw up in the console)
the current options are:

Use an external helper and call it after every test (cons: people will forget to do it and there is not much you can do about it)
Create a global wrapper global.it = wrapInConsoleOutputValidator(global.it) (cons: hacky as hell)

this is also related to this issue, i assume... #2120

kashinross · 2017-08-08T23:31:58Z

I'm currently using this on some of my tests and I love the functionality. I'm currently using this.retries(1) at the describe level but I noticed the retry logic always attempts to run the current it(). Is there any way to retry the entire describe block on a failure?

danielstjules reviewed Dec 1, 2015
View reviewed changes

longlho force-pushed the 1773-retries branch from 1116875 to 5d44022 Compare December 1, 2015 16:59

tinganho reviewed Dec 3, 2015
View reviewed changes

longlho force-pushed the 1773-retries branch from 5d44022 to 1c7d62e Compare December 3, 2015 15:45

longlho force-pushed the 1773-retries branch 2 times, most recently from cadf34e to b09dc2a Compare December 9, 2015 15:46

danielstjules reviewed Dec 9, 2015
View reviewed changes

longlho force-pushed the 1773-retries branch from b09dc2a to 985e436 Compare December 9, 2015 18:50

longlho force-pushed the 1773-retries branch 3 times, most recently from d7aeaf6 to bdedd85 Compare December 10, 2015 05:38

danielstjules reviewed Dec 25, 2015
View reviewed changes

Allow to retry failed test from test context for mochajs#1773

7d394ab

- make retries run proper hooks - allow retries override at different levels - expose currentRetry to reporters

longlho force-pushed the 1773-retries branch from bdedd85 to 7d394ab Compare December 28, 2015 02:16

danielstjules added a commit that referenced this pull request Dec 28, 2015

Merge pull request #1989 from longlho/1773-retries

d811eb9

Allow to retry failed test from test context for #1773

danielstjules merged commit d811eb9 into mochajs:master Dec 28, 2015

longlho deleted the 1773-retries branch December 28, 2015 17:55

klamping mentioned this pull request Jan 6, 2016

New version of Mocha requested #2040

Closed

longlho mentioned this pull request Feb 24, 2016

Retry logic is not working when beforeEach() or afterEach() functions fail #2120

Closed

aaronabramov mentioned this pull request Feb 25, 2016

🚀 Feature: Retry if afterEach fails #2127

Closed

This was referenced Apr 5, 2016

Option to retry tests #1773

Closed

Mocha programmatic API doesn't report retries #2188

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to retry failed test from test context for #1773 #1989

Allow to retry failed test from test context for #1773 #1989

longlho commented Nov 30, 2015

danielstjules Dec 1, 2015

longlho Dec 1, 2015

danielstjules commented Dec 1, 2015

longlho commented Dec 1, 2015

tinganho Dec 3, 2015

tinganho Dec 3, 2015

longlho Dec 3, 2015

longlho commented Dec 5, 2015

danielstjules Dec 9, 2015

danielstjules commented Dec 9, 2015

longlho commented Dec 10, 2015

danielstjules commented Dec 10, 2015

longlho commented Dec 10, 2015

danielstjules commented Dec 24, 2015

danielstjules Dec 25, 2015

longlho Dec 28, 2015

danielstjules commented Dec 25, 2015

tinganho commented Dec 25, 2015

danielstjules commented Dec 25, 2015

longlho commented Dec 28, 2015

danielstjules commented Dec 28, 2015

tinganho commented Jan 6, 2016

tinganho commented Jan 6, 2016

danielstjules commented Jan 6, 2016

ajaks328 commented Feb 10, 2016

aaronabramov commented Feb 24, 2016

kashinross commented Aug 8, 2017

		@@ -0,0 +1,11 @@
		describe('retries', function() {
		this.retries(4);

Allow to retry failed test from test context for #1773 #1989

Allow to retry failed test from test context for #1773 #1989

Conversation

longlho commented Nov 30, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielstjules commented Dec 1, 2015

longlho commented Dec 1, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

longlho commented Dec 5, 2015

Choose a reason for hiding this comment

danielstjules commented Dec 9, 2015

longlho commented Dec 10, 2015

danielstjules commented Dec 10, 2015

longlho commented Dec 10, 2015

danielstjules commented Dec 24, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielstjules commented Dec 25, 2015

tinganho commented Dec 25, 2015

danielstjules commented Dec 25, 2015

longlho commented Dec 28, 2015

danielstjules commented Dec 28, 2015

tinganho commented Jan 6, 2016

tinganho commented Jan 6, 2016

danielstjules commented Jan 6, 2016

ajaks328 commented Feb 10, 2016

aaronabramov commented Feb 24, 2016

kashinross commented Aug 8, 2017