junit-team / junit5

✅ The 5th major version of the programmer-friendly testing framework for Java and the JVM
https://junit.org
Other
6.32k stars 1.47k forks source link

Introduce support for retrying failed flaky tests #1558

Open arcuri82 opened 6 years ago

arcuri82 commented 6 years ago

Overview

Feature request to add native support to rerun failing flaky tests

Feature request

Is there any plan to support the ability of rerunning failed flaky tests? For example, it would be great to have something like @Flaky(rerun=3), which would rerun a failing flaky test up to n times.

In the past, in Surefire/Failsafe we could use rerunFailingTestsCount. However, it does not work in JUnit 5. As JUnit 5.0.0 was released nearly 1 year ago, it is unclear if and when it will be supported in Maven.

And, even if it will be supported in Maven in the future, it could be good to have a special annotation to specify that only some tests are expected to be flaky, and not all of them.

At the moment, the workaround is to stick with JUnit 4, which is a kind of a shame as JUnit 5 has a lot of interesting features :( (which I can only use in projects with no flaky tests)

Related SO question.

sbrannen commented 6 years ago

No, to my knowledge there are not currently any plans to implement such a feature.

But... I think it's a very good idea.

I've often wanted something like Spring Retry's @Retryable support for test methods. Of course, Spring Retry supports a much larger feature set than we would ever need/want in JUnit Jupiter.

Thanks for providing the link to the discussion on Stack Overflow. There are two interesting implementations there that can serve as inspiration.

Ideally, I'd like to see something like a @RetryableTest annotation that allows one to specify:

@junit-team/junit-lambda, thoughts?

sbrannen commented 6 years ago

Tentatively slated for 5.4 M1 for team discussion

jbduncan commented 6 years ago

configurable exceptions that initiate a retry, defaulting to {Throwable.class}

Hmm, how about defaulting to Exception.class or Throwable.class-except-blacklisted-exceptions?

My thinking is that in the off-chance an OutOfMemoryError is thrown, it should really be propagated to allow the JVM to shutdown as cleanly as it possibly can. :thinking:

sormuras commented 6 years ago

I don't like flaky tests (read: checks). Who does? I don't like the idea of "fixing" flaky tests by (naiv, smart, conditional, [what|for]-ever) re-execution. I don't want to support that in Jupiter.

Easy work-around:

@Test void test() { REPEAT flaky() UNTIL flaky-signals-green OR break-out }
boolean flaky() { ... return true : false ... }

Perhaps (!) such an annotation could live in an extension providing library: like https://junit-pioneer.org or another one. If we need to expose a new extension point tailored to make the "retry" functionality possible, well, I'm fine with that.

arcuri82 commented 6 years ago

@sormuras I do not like flaky tests either :) but unfortunately they are a reality :(

not an issue with unit tests (unless you work with randomized algorithms), but they are a big problem for E2E tests. There, it is hard to escape from flakyness. Even if a test wrongly fails 1 out 1000 times, when you have hundreds/thousands of E2E tests, there is a high probability that at least 1 fails, failing the whole build.

Of course one can use a try/catch in a loop, but that is a lot of boilerplate that has to be repeated each time for each test.

In the past, it was not an issue with JUnit 4, as this was externally handled with Maven. But this is not the case any more. In my case, this is a major showstopper for using JUnit 5. In some projects we tried JUnit 5 and were forced to revert to 4. In others, we use JUnit 5 for unit tests, but then have a (Maven) module for the E2E using JUnit 4.

nipafx commented 6 years ago

Coincidentally, I recently hacked an extension that repeats failed tests:

class RepeatFailedTestTests {

    private static int FAILS_ONLY_ON_FIRST_INVOCATION;

    @RepeatFailedTest(3)
    void failsNever_executedOnce_passes() { }

    @RepeatFailedTest(3)
    void failsOnlyOnFirstInvocation_executedTwice_passes() {
        FAILS_ONLY_ON_FIRST_INVOCATION++;
        if (FAILS_ONLY_ON_FIRST_INVOCATION == 1) {
            throw new IllegalArgumentException();
        }
    }

    @RepeatFailedTest(3)
    void failsAlways_executedThreeTimes_fails() {
        throw new IllegalArgumentException();
    }

}

If JUnit 5 decides that flaky tests have no place in a proper testing framework, we pioneers don't have such scruples. :wink:

sbrannen commented 6 years ago

Nice, @nicolaiparlog! 👍

I'm guessing you didn't know about the Rerunner Jupiter extension? 🤔

nipafx commented 6 years ago

Nope. :laughing: Most extensions I write come from excursions into the extension model, not from actual use cases (hence the lack of research).

Looks like @arcuri82 has at least one, maybe soon two options to achieve his goal of rerunning flaky tests in JUnit 5 - no need for native support. :wink:

sormuras commented 6 years ago

Related to "flaky tests": https://twitter.com/michaelbolton/status/1032125004480237568

smoyer64 commented 6 years ago

He should sooooo change his name - https://www.youtube.com/watch?v=ADgS_vMGgzY

marcphilipp commented 6 years ago

According to @anonygoose in https://github.com/junit-team/junit5/issues/1079#issuecomment-418351588 the Rerunner extension is "dangerously buggy, and inactive".

@anonygoose Could you please post an example that showcases its bugginess?

anonygoose commented 6 years ago

Hey Marc,

import io.github.artsok.RepeatedIfExceptionsTest;

public class RerunnerBugTest {

    @RepeatedIfExceptionsTest(repeats = 3)
    void test1() {
        System.out.println("Running test 1");
    }

    @RepeatedIfExceptionsTest(repeats = 3)
    void test2() throws Exception {
        System.out.println("Running test 2");
        throw new Exception("How does this not error?  It never runs!");
    }

    @RepeatedIfExceptionsTest(repeats = 3)
    void test3() {
        System.out.println("Running test 3");
    }

}

The above set of tests will run test1, and then skip test2 and test3. As soon as one test annotated with the annotation passes, it will skip any other tests annotated with it.

Uploaded the full example here: https://github.com/anonygoose/rerunner-bug-example

artsok commented 6 years ago

Hi guys :)

I can make Pull Request with ReRun flasky mechanism, but i need information :)

anonygoose commented 6 years ago

Getting official support for a retry feature would be great.

Ideally, I'd like to see something like a @RetryableTest annotation that allows one to specify:

  • maximum number of retries
  • configurable exceptions that initiate a retry, defaulting to {Throwable.class}
  • potentially a minimum success rate

Each of these points are configurable on the current Rerunner extension, and are useful. Carrying them over to any official support seems sensible.

Further consideration may need giving to:

nipafx commented 6 years ago

It would be nice to know whether the JUnit 5 team is considering implementing this.

@artsok: Would you consider integrating your extension into JUnit Pioneer? You would get more visibility and more hands to fix issues or implement features.

sormuras commented 6 years ago

...now on the list for tomorrow's team call.

benken-parasoft commented 6 years ago

dangerously buggy, and inactive

I wanted to second this. I came to a similar conclusion a few weeks ago. I had to give up on other attempts, not feeling they were entirely safe, clean, light-weight, or stable. Instead, I ended up making my own copy of RepeatedTestExtension then made it use a custom TestTemplateInvocationContext Iterator with a TestExecutionExceptionHandler that conditionally throws a TestAbortedException. This seems to be the general approach I've seen floating around but at least I have my own light-weight solution that I have control over. I wish artsok and pioneer the best of luck but it would definitely be nice to have a clean, official, stable, and trusted solution.

filiphr commented 6 years ago

Just to add my 2 cents to this issue. We also have such flaky tests and were using the Surefire rerunFailingTestsCount in order to circumvent this. However, since our migration to JUnit Jupiter we noticed that this does not work anymore 😄. It would be great if there is something official from JUnit or maybe the appropriate plugins (Maven, Gradle) that would still have this support.

If you ask me I think that it is a bit better if we can do this expressively in the test itself and not have it global for the entire build.

marcphilipp commented 6 years ago

Team Decision: As a first step, we think this should be maintained externally as an extension based on @TestTemplate. We might potentially introduce support for rerunning flaky tests in core, but then it should support all kinds of testable methods, including @ParameterizedTests. We'd be happy to provide guidance and review such an extension, whether in the rerunner project, junit-pioneer or another project.

nipafx commented 6 years ago

I've just pushed my proof of concept to Pioneer, I'd like to implement that feature there and am interested in ideas, feedback, and even code if someone wants to write it. :smile_cat:

@marcphilipp This can be closed, right?

tictac-freshmint commented 5 years ago

Surefire can do this, but unfortunately only for JUnit 4: https://maven.apache.org/surefire/maven-surefire-plugin/examples/rerun-failing-tests.html I am not sure what the equivalent for Gradle is and if one of its plugins support retries.

What we need is that in the Surefire test result XML files the information about retries are contained. We use Allure for reporting, and I believe it processes these information in order to display how often a test has been repeated and what failures occured in each retry. Hopefully Surefile will soon implement this for JUnit 5, but I did not find any issue about that. I just created an issue for it.

A solution just for within an IDE will not help us at all. Think on CI/CD and the test result reporting.

I dislike the name ¨RepeatIfFailedTest¨, four words in a long camel case. Why not just @Retry?

ekmaughan commented 5 years ago

My team and I are very anxious to upgrade to JUnit 5, but this has been the hang up for us. May I ask if there are any updates for this? I have noticed that the Rerunner project and Pioneer have yet to implement the ability of stacking a Retry annotation on top of @ParameterizedTests which is key to what we are wanting to accomplish.

marcphilipp commented 5 years ago

@ekmaughan Thanks for letting us know that this feature is important for you. I'm afraid I don't have any updates for you at this point. I agree that this would be a useful feature for integration tests that depend on external resources.

@arcuri82 Have you submitted an issue to the Surefire project for this?

arcuri82 commented 5 years ago

@marcphilipp Someone else already did: https://issues.apache.org/jira/browse/SUREFIRE-1584

dlanaghen commented 5 years ago

Hey @marcphilipp

One more vote but we would prefer configuration like the surefire rerunFailingTestsCount feature.

We have been using JUnit5 since its experimental days with great success. We are using it for integration testing of our API and like others, between network issues and platform issues, there is some level of unpredictability which is magnified because we are hitting our api so hard now that we can run things in parallel.

We have many tests and no one test is likely to be more "flaky" than another. The surefire rerunFailingTestsCount was actually the ideal solution for us rather than a special annotation per test. Ideally we would like to configure this kind of a feature for all tests.

I can see the use case for both a test specific annotation, or a configuration for all tests.

marcphilipp commented 5 years ago

@dlanaghen Makes sense! We could add a global config parameter for the default.

artsok commented 5 years ago

Hey @ekmaughan

You can try to use my implementation with support @ParameterizedTests which I did today. As all main parameterized classes have default visibility I need to set mine owns. For that reason, I introduce a new annotation @ParameterizedRepeatedIfExceptionsTest.

Example:

    private ThreadLocalRandom random = ThreadLocalRandom.current();

    /**
     * By default total repeats = 1 and minimum success = 1.
     * If the test failed by this way start to repeat it by one time with one minimum success.
     *
     * This example with display name, repeated display name, 10 repeats and 2 minimum success with exceptions.
     * Exception depends on random number generation.
     */
    @DisplayName("User payment")
    @ParameterizedRepeatedIfExceptionsTest(name = "payment amount was {0}",
            repeatedName = " (Repeat {currentRepetition} of {totalRepetitions})",
            repeats = 10, exceptions = RuntimeException.class, minSuccess = 2)
    @ValueSource(ints = {40, 55, 61, 700})
    void errorParameterizedTestWithDisplayNameAndRepeatedName(int argument) {
        if (random.nextInt() % 2 == 0) {
            throw new RuntimeException("Exception in Test " + argument);
        }
    }

IDEA Report:

изображение

Explaining: First test with argument '40' failed. Repeat this test with first argument. Wait the sitaution when will have 2 minimum success runs. Then go to the next test with second argument and run it. With second argument way we can see that it was no 2 minimum success follow each other. Try to repeat 10 times but wihout result, go next test with argument '61' and so on.

Allure Report (another test run):

изображение

Checked with this Junit version

<junit.jupiter.version>5.4.2</junit.jupiter.version>
<junit.platform.version>1.4.2</junit.platform.version>

You can try this extension:

<dependency>
    <groupId>io.github.artsok</groupId>
    <artifactId>rerunner-jupiter</artifactId>
    <version>2.0.1</version>
    <scope>test</scope>
</dependency>

The main logic you can find in ParameterizedRepeatedTestExtension.class. Source code you can find here. Don't forget to send star if u want.

cypris commented 5 years ago

Thanks @artsok. Looking forward to when you’ve made sure it works parallelized too!

toby1984 commented 5 years ago

Hi, Any progress on this ? We rely on the 'rerunFailingTestsCount' feature of maven-surefire-plugin and I just learned the hard way that it does not work with JUnit5.

marcphilipp commented 5 years ago

@toby1984 While we'd still like to implement this, we haven't had time to look into it more closely, yet.

One open question is if/how we should report repetitions with "allowed failures". @artsok seems to have reported them as failed, I could also envision reporting them as aborted. Or we could not report them at all and only fail the test in the end if it didn't pass after the configured number of retries.

Another open question is the lifecycle of a repetition. Spock allows to configure which parts of the setup get executed for a retry. I think we can make it simple to start with and repeat the whole lifecycle like we do for repeated and parameterized tests.

Thoughts?

itrifonov commented 5 years ago

VSTS has option in filters "passed on rerun", i don't know how it looks in XML but not Aborted for sure as it's a separate outcome. image

marwatk commented 4 years ago

I have a need for this with some Selenium tests that are the result of a TestFactory and I've been playing around with the new InvocationInterceptor to try to make it work:

    @Override
    public void interceptTestMethod( Invocation<Void> invocation,
            ReflectiveInvocationContext<Method> invocationContext,
            ExtensionContext extensionContext ) throws Throwable {
        runTest( invocation, invocationContext, extensionContext );
    }

    @Override
    public void interceptDynamicTest( Invocation<Void> invocation,
            ExtensionContext extensionContext ) throws Throwable {
        runTest( invocation, null, extensionContext );
    }

    public Void runTest( Invocation<Void> invocation,
            ReflectiveInvocationContext<Method> invocationContext,
            ExtensionContext extensionContext ) throws Throwable {

        int runCount = extensionContext.getTestMethod().isPresent()
                && extensionContext.getTestMethod().get().isAnnotationPresent( Retry.class )
                ? 3 : 1;

        Throwable lastError = null;
        for ( int i = 0; i < runCount; i++ ) {
            try {
                log.debug( "Running run {}/{} of [{}]",
                        i + 1, runCount, extensionContext.getDisplayName() );
                return invocation.proceed();
            }
            catch ( Throwable t ) {
                log.info( "Test run {}/{} of [{}] resulted in error {}",
                        i + 1, runCount, extensionContext.getDisplayName(), t );
                lastError = t;
            }
        }
        throw lastError;
    }

Which seems like a simple place to retry that doesn't report extra tests and avoids the issue of when to run them. It does prevent you from running different parts of the lifecycle, though.

The problem is proceed is validated to ensure it's only called once. The validation makes sense in most cases, but in this case I specifically want to run the test more than once. (When the annotation is present).

Is overriding the validation something that could be considered? Perhaps a way for the interceptor to indicate that it is purposefully re-running proceed? I can submit a PR if so.

jaksa76 commented 4 years ago

I've written a library called unreliable that could help here. It is not junit specific, but it does allow you to run a piece of code several times:

// this will try to invoke the service 5 times
Unreliable.tenaciously(() -> unreliableService.doSomething(), 5); 

There are variants of this syntax that allow you to specify number of retries, exceptions to filter etc.

marcphilipp commented 4 years ago

@marwatk Implementing this using InvocationInterceptor is very limited since it does not provide the option to repeat the previous lifecycle steps. Nevertheless, I can imagine lifting the requirement to call proceed exactly once. There has also been a request to allow skipping it entirely in #2083.

sbrannen commented 4 years ago

@jaksa76, your library looks quite promising. Thanks for sharing.

Seijan commented 4 years ago

There was a step in the right direction from maven-surefire as they reenabled rerunning tests with surefire for JUnit5. This is still in a snapshot version, but maybe some of you find it helpful. https://github.com/apache/maven-surefire/pull/245 As I pointed out there, parameterized tests aren't working at all with this feature, but if you don't use this feature you should be good to go.

Tibor17 commented 4 years ago

@Seijan Summary in current activities: We implemented Surefire rerun for JUnit5 in https://issues.apache.org/jira/browse/SUREFIRE-1701 and there are ongoing issues to fix https://github.com/apache/maven-surefire/pull/245#issuecomment-548949938 The feature skipAfterFailureCount is reported in Surefire https://issues.apache.org/jira/browse/SUREFIRE-1710

Tibor17 commented 4 years ago

@Seijan Now the re-run + @Parameterized test works. The display name in this annotation also works and you can have it in the XML report if you enable that in the configuration of the extension, see the documentation. https://issues.apache.org/jira/browse/SUREFIRE-1716 Feel free to subscribe in the mailing list and participate in the release vote: https://lists.apache.org/thread.html/6b9987be2fbd4d82daf8aaabbd1d58dfa2a533598770e0423d0b5ee0@%3Cdev.maven.apache.org%3E

nishant-vashisth commented 4 years ago

One add-on request here if the Junit5 team goes ahead with this. It'll be nice to have a facet for this Extension as an interface as well rather than just only as an annotation. Something like ExecutionCondition where a config object could be returned on basis of custom logic, say Exception type.

@Override public RetryCondition evaluateExecutionCondition(ExtensionContext context, RetryCondition condition) {

Obviously for someone who needs it to be test level Annotation will then provide values to have the config object via annotation fields

This way, if someone intends to have global support for their E2E tests (I know this sounds ghastly) but,

We have many tests and no one test is likely to be more "flaky" than another.

The Extension can be registered using autodetect config and have it tailored for Exceptions coming from all tests

marcphilipp commented 4 years ago

FWIW Gradle now also supports this via the Test Retry Gradle Plugin.

JohnArrowwood commented 3 years ago

+1 on the need for re-running flaky end-to-end tests. My personal example: Change a user's password in AD. Then confirm that the new password works. But since the change was written directly to the primary node but the bind happens by connecting to the load balancer, there is a good chance that the attempt to bind will hit the secondary node before the password change was able to propagate to it, and when that happens, it fails.

Adding a fixed-length timeout to try and give the data time to propagate is lame. Adding a replication check to explicitly wait for the data to propagate is complicated. And adding a retry loop to every place where things might go wrong is . . , yeah, right, not gonna happen. But adding a config option that says "If it fails, retry, up to N times", that would really make my life MUCH better!

nipafx commented 3 years ago

Here are a few options to rerun failing tests:

sbrannen commented 3 years ago

Unreliable by @jaksa76

You may also be interested in the RetryTemplate from Spring Retry if you are looking for a programmatic approach similar to UnreliableService.

asafm commented 3 years ago

Here are a few options to rerun failing tests:

* [Unreliable](https://github.com/jaksa76/unreliable) by @jaksa76

* [rerunner-jupiter](https://github.com/artsok/rerunner-jupiter) by @artsok

* [`@RetryingTest`](https://junit-pioneer.org/docs/retrying-test/) in JUnit Pioneer

  * Disclosure A: I'm one of the maintainers of Pioneer
  * Disclosure B: We really like those other projects and are in the process of ~stealing~ being inspired by their ideas

From I see, none of them support applying it to all, which uses the standard @Test annotation, right?

marcphilipp commented 3 years ago

Which build tool are you using? Both Maven and Gradle support retrying on the build level.

asafm commented 3 years ago

Which build tool are you using? Both Maven and Gradle support retrying on the build level.

I'm referring specifically using JUnit with the tools list above and not applying it in build tool level.

marcphilipp commented 3 years ago

From I see, none of them support applying it to all, which uses the standard @Test annotation, right?

That's correct.

nipafx commented 3 years ago

@asafm Unless I'm missing something, there's no way to rerun failed tests from within Jupiter without providing a replacement for @Test. If I'm right, you preferred solution doesn't exist.

deepakab03 commented 3 years ago

Hope this is added to the main JUnit Jupiter soon. Another use-case for this is when there are unit tests that depend on timeout conditions and said tests run on CI nodes and the nodes are busy or under-powered. These tests randomly fail on assertion conditions like a particular method should have been called twice, but on CI node it is called thrice because the calling thread went to sleep for a bit, messing up the timeout condition.. if this were to be repeated a couple of times, the chances of it passing would be much higher, increasing the calls to thrice is not the correct option but we are forced to do this when the tests runs on the CI node by means of some conditions which is ugly..

asafm commented 3 years ago

@nipafx You are correct, solution doesn't not exists. I want Jupiter to support the ability to do so :)