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

Retries per specfile #2930

Closed
wants to merge 4 commits into from

Conversation

bennieswart
Copy link
Contributor

This is a continuation of #2556.

Proposed changes

Add retries on a per-specfile basis, as discussed in #2484.

Previously only test-level and suite-level retries were available, which
are fine in most cases. In any test which involves state, such as on a
server or in a db, the state may be left invalid once the test fails the
first time. Any subsequent retries may have no chance of passing due to
the invalid state they would have to start with.

A new browser instance is created for each specfile, which makes this an
ideal place to hook and setup any other states (server, db). Retries on
this level would mean that the whole setup process could simply be
repeated as if it were for a new specfile, with the results of the
failing specfile (that is to be retried) being discarded.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

Reviewers: @christian-bromann

bennieswart and others added 4 commits September 3, 2018 14:29
Previously only test-level and suite-level retries were available, which
are fine in most cases. In any test which involves state, such as on a
server or in a db, the state may be left invalid once the test fails the
first time. Any subsequent retries may have no chance of passing due to
the invalid state they would have to start with.

A new browser instance is created for each specfile, which makes this an
ideal place to hook and setup any other states (server, db). Retries on
this level would mean that the whole setup process could simply be
repeated as if it were for a new specfile, with the results of the
failing specfile (that is to be retried) being discarded.
This allows the reporter to set the start and end properties of
runnables to the time the event happened as opposed to the time the
reporter got notified of the event.
@bennieswart
Copy link
Contributor Author

@christian-bromann anything that still needs doing here?

@christian-bromann
Copy link
Member

@bennieswart two questions:

  • how is this affecting reporter from not reporting the same spec as failed and passed at the same time?
  • do we want to port this to v5? If so a PR is required.

@bennieswart
Copy link
Contributor Author

@christian-bromann when there are retries left, messages are buffered in a list and only reported once (and if) the tests for that child process have passed. If the child process ends with a failure, the buffered messages are discarded. If no retries are left, messages are unbuffered and reported as they are received. This way, only one set of messages get reported.

Regarding v5, I see no reason why we should not port it to v5 also. There have been quite a few interested parties on the previous PR, so it seems to be a practical feature from which others could benefit as well.

@WillBrock
Copy link
Member

This will be really nice to have.

@aikrasnov
Copy link

Image of qa

@AdrieanKhisbe
Copy link

Very interesting feature!
What's the status on this @bennieswart ? :)

@shackijj
Copy link

shackijj commented Nov 23, 2018

Hello

This feature is really good because it allows running tests in a new environment.

For those who want to get it now, there is an alternative implementation of a test runner which works fine. The one drawback of the runner is that wdio's sync option is not supported there. It's the main reason why I don't want to use that in my projects. Literally it means that I have rewrite hundreads of tests.

@bennieswart @christian-bromann
Looking at the history of this PR, I see that the last message about two monts ago.
Please, let me know if you need some help here.

@christian-bromann
Copy link
Member

This was closed because I moved everything related to v4 to https://github.com/webdriverio-boneyard/v4

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.

7 participants