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

Option to retry tests #1773

Closed
EvanHahn opened this issue Jun 30, 2015 · 41 comments
Closed

Option to retry tests #1773

EvanHahn opened this issue Jun 30, 2015 · 41 comments
Labels
type: feature enhancement proposal

Comments

@EvanHahn
Copy link

When writing Selenium/browser tests, weird things just break unreliably. I suspect most people who write browser tests find this to be true.

I would love to be able to retry failing tests a certain number of times.

I would love to do something like this:

mocha --retries 3 

...or maybe this:

it('works', function () {
  this.retries(3);
  // try stuff...
});

I'd be willing to PR this if this sounds like a good idea.

@chenchaoyi
Copy link

This would be great. Also see #1515

Right now mocha-retry currently doesn't work with the latest Mocha (giggio/mocha-retry#6). And mocha-extra-shot which is "inspired by the ideas of mocha-retry" doesn't work for me either.

@EvanHahn
Copy link
Author

EvanHahn commented Jul 3, 2015

If someone gives me approval to do this, I'll spend some time PRing.

@boneskull
Copy link
Member

Seems useful to me. Mocha could use more functional testing features.

It's not unusual. Exhibit A: our travis.yml.

@mochajs/mocha any other opinions?

@jbnicolai
Copy link

@boneskull I'm all for it, especially since the projects mentions in #1515 seem abandoned.

@jbnicolai jbnicolai added type: feature enhancement proposal status: accepting prs Mocha can use your help with this one! labels Jul 4, 2015
@jbnicolai
Copy link

@EvanHahn if you'd be willing to give this a go I'd be happy to review your proposed changes :)

@janbaykara
Copy link

Is anyone working on a PR for this?

@boneskull
Copy link
Member

@janbaykara Not to my knowledge, but I think we'd like to see this functionality

jegsar added a commit to jegsar/mocha that referenced this issue Oct 26, 2015
@jegsar
Copy link

jegsar commented Oct 26, 2015

@janbaykara
I decided to try and take a crack at this, and best I can tell the retry and tests themselves work however I am having an issue exposing the retries function to allow calling it like so

it('works', function () {
  this.retries(3);
  // try stuff...
});

@ajaks328
Copy link
Contributor

I would really like to see this happen. Especially since we are using Mocha at a functional/e2e testing level. Please let me know if I could help with this in anyway and expedite the implementation.

@jegsar
Copy link

jegsar commented Oct 30, 2015

@ajaykodali I have the implementation working on the Runnable object itself.
Only issue I am having is figuring out how to do the this.retries(x) inside of the it function to point at the retries function I created in the Runnable object.

@chenchaoyi
Copy link

A cmdline option as suggested mocha --retries 3 is good too, user can choose different retry times in different run instead of updating in the test cases.

@ajaks328
Copy link
Contributor

@jegsar do you have this available on a fork so I can take a look?

@jegsar
Copy link

jegsar commented Oct 31, 2015

I like both options though for my use cases the this. option is more useful. once that works I would be happy to add on additional paths.

@ajaykodali master...jegsar:add/1773-retries
It's rather simple what I did, and I am sure I just missed something with the linking to "it" and the other interfaces but I haven't had the time to return to it. if you can give me a pointer on how to do it should be able to finish it up on Monday, Tuesday the latest. If not then I'll continue next weekend most likely.

@longlho
Copy link
Contributor

longlho commented Nov 24, 2015

hi guys, any progress on this? I'm happy to put some work into the outstanding patch that @jegsar has

@jegsar
Copy link

jegsar commented Nov 25, 2015

not yet, still pretty sure i am just missing the link from this.retries in the it block for example to the runnable this.retries

@longlho
Copy link
Contributor

longlho commented Nov 25, 2015

@jegsar mind if I take a look?

@ajaks328
Copy link
Contributor

I looked at it briefly and played with it in trying to hook it up but didn't get to spend much time on it. It does look like something simple missing in the wiring of it.

@jegsar
Copy link

jegsar commented Nov 25, 2015

yeah it's something simple i'm missing just not sure what it is. I if you can figure it out, that'd be great. If not i'll take a look at how timeout works.

@longlho
Copy link
Contributor

longlho commented Nov 30, 2015

@jegsar @boneskull feel free to take a look at my PR and the tests associated w/ it. In short I decided to re-queue the test upon failure since that'll run the appropriate hooks (beforeEach, afterEach) instead of just re-running the test itself (might manifest global state especially if it relies on afterEach for clean-up)

@ajaks328
Copy link
Contributor

ajaks328 commented Dec 9, 2015

Any update or feedback on @longlho 's PR?

longlho pushed a commit to longlho/mocha that referenced this issue Dec 10, 2015
- make retries run proper hooks
- allow retries override at different levels
- expose currentRetry to reporters
@stalniy
Copy link

stalniy commented Dec 21, 2015

@EvanHahn "retry" option underlines unreliability of your tests

@boneskull
Copy link
Member

@stalniy Sometimes it's unavoidable in functional testing. Occasionally it's too difficult and/or time-consuming to track down the source of intermittent failures.

@boneskull
Copy link
Member

@ajaks328
Copy link
Contributor

^.. this exactly. I hear that argument about "unreliability of your tests" all the time. That is definitely true for unit testing where you can mock all external dependencies and ensure your tests are reliable and true. However with e2e/functional testing, attempting to solve for 100% "reliability" in tests often ends up in other issues being masked. Having a "retry" option gives you a chance to at least get estimates on how often the problem test fails or passes and try to go from there. Running a 100 test case suite which takes 2+ hours and having to repeat it due to a single failure is not fun :)

@boneskull
Copy link
Member

yep. if you're retrying your unit tests, you're doing it wrong, but that's not the use case here.

@EvanHahn
Copy link
Author

@stalniy What @boneskull and @ajaykodali said is basically my case—I'm running flaky integration tests across a series of browsers.

@boneskull
Copy link
Member

fyi, I'm heads-down on #1969 atm, so PRs/issues will mainly need to be addressed by other collaborators

cc @mochajs/mocha

@stalniy
Copy link

stalniy commented Dec 21, 2015

Ok guys, but all these are excuses. I have been writing e2e tests using Protractor for 1+ year. We had issues with "sometimes broken" tests until understood that we use ignoreSynchronization incorrectly. Now 7 months later, no phantom failures, everything is predictable and clear.

By the way, I use Page Object pattern which allows to hide complexity of the tests. So, you can abstract your retrying logic using this pattern (e.g. retry to get value of some element multiple times)

I against this feature as it leads to bad/unclear/unpredictable code.

It doesn't matter what you test if your tests sometimes fail you do something wrong, defenetly

@boneskull
Copy link
Member

@stalniy Congratulations on your passing tests! I'm happy your experience with a single automated testing framework--running against a single headless browser--over the past year has given you the confidence to proclaim everyone else must be writing bad code! Even those dummies at Walmart Labs--what the hell do they know, right?

@stalniy
Copy link

stalniy commented Dec 22, 2015

@boneskull I'm sorry that it sounds offensive to you.

Will give you some context so your words will have more facts and no water.

Virtual windows machine, Jenkins, ant, xslt reporter (creates report from junit xml similar to mocha doc reporter), IE10, FF35, Chrome latest, (5h 20m execution), have few tests ("it" constructions) which execute for 45m each one.

No phantom failures. If guys at Walmart Labs can't write stable tests probably they need some advice from an expert.

These are facts. My goal is not to offend somebody but instead to make software being more reliable, clear and predictable. That's why I'm writing here.

this feature is a hack. That's also a fact. Just wanted to enlighten this.

@boneskull
Copy link
Member

I'm not interested in debating someone who doesn't understand what "facts" are. Thank you for your input.

@stalniy
Copy link

stalniy commented Dec 22, 2015

@boneskull That's a pity because it looks like it's more important to win the debate for you than to hear a feedback which doesn't corelate with your perception of the world.

In my opinion you are a chatterbox because you offended me 2 times without even have at least a bit of correct information about what you are writing (the new behavior of professionals?).

You won. Good luck

@EvanHahn
Copy link
Author

I'm happy to have this issue closed and locked if that's what's needed here.

@longlho
Copy link
Contributor

longlho commented Dec 22, 2015

Agree that this is not meant for unit tests. My PR is still open btw in case mocha owners wanna take a look.

@ajaks328
Copy link
Contributor

@EvanHahn this is a valid feature and is supported by popular frameworks in different languages. Since @longlho already has a PR it would be great to have some feedback from the mocha team.

Lets not get side tracked by trolls.

longlho pushed a commit to longlho/mocha that referenced this issue Dec 28, 2015
- make retries run proper hooks
- allow retries override at different levels
- expose currentRetry to reporters
danielstjules added a commit that referenced this issue Dec 28, 2015
Allow to retry failed test from test context for #1773
@danielstjules
Copy link
Contributor

Oops, forgot to close this :)

@cletusw
Copy link

cletusw commented Apr 4, 2016

Any chance of getting this behavior to cover failures in a beforeEach as well? We try to share as much code between our tests as we can, which often means the flaky behaviors are in a beforeEach.

@cletusw
Copy link

cletusw commented Apr 5, 2016

Never mind. Just saw #1989 (comment)

abyx pushed a commit to abyx/mocha that referenced this issue Aug 14, 2016
- make retries run proper hooks
- allow retries override at different levels
- expose currentRetry to reporters
@boneskull boneskull removed the status: accepting prs Mocha can use your help with this one! label Oct 10, 2016
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