Move Rerunner-Jupiter/improve RetryingTest?

[Michael1993]

Here is a really nice extension of JUnit: rerunner-jupiter.

Has not been updated in ~9 months. If the author agrees I think we should take on this extension.
I think the extension itself fits in nicely with our other extensions but I'm not sure what the proper etiquette is when merging open-source projects. Is that even a thing? Is it rude to even consider it? Someone help me out here. 😭

@nipafx , @Bukama , @aepfli Please give feedback.

Edit: Looking at the commit history it seems the author works on the extension in bursts. For example there is a bit of a gap between 2019-07-02 and 2020-01-10.

[Michael1993]

It is really similar to @RetryingTest. In some ways it offers more, in other ways much less. Maybe we should not move it to Pioneer as-is but extend RetryingTest to offer the same benefits? 🤔

[beatngu13]

@Michael1993 would you mind summarizing the pros and cons compared to @RetryingTest from your POV?

[Michael1993]

I'll compare the two properly this evening and get back to you.

[Michael1993]

Comparison (I'll edit this as I explore the feature):

If you look back the history of this comment you'll probably realize sooner than me that I wanted @RetryingTest and not @RepeatedTest 😩 Sorry! I'll try again:

1.) rerunner-jupiter allows more fine-grained control than @RetryingTest, by allowing you to retry only for specific exceptions. Strangely enough it does not seem to work with Error, however.

This always works, even though I only wanted the test to try again if it found a FileNotFoundException

static int counter = 0;

@RetryingTest(3)
void test() {
  if (counter == 0) {
    counter = 1;
    throw new ExportException("I expected a FileNotFoundException. 😭 ");
  }
}

In rerunner-jupiter this test would look like:

static int counter = 0;

@RepeatedIfExceptionsTest(repeats = 3, exceptions = FileNotFoundException.class)
void test() {
  if (counter == 0) {
    counter = 1;
    throw new ExportException("I expected a FileNotFoundException. 😭 ");
  }
}

This fails! Hooray?

2.) @RetryingTest does not work with params. rerunner-jupiter offers @ParameterizedRepeatedIfExceptionsTest. You can make a composed annotation like this:

@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@ParameterizedTest
@RetryingTest(4)
public @interface ParameterizedRetryingTest {
}

and it "kind of" works...

Except you probably wanted the test to try again with the same parameter. Output of the same test with @ParameterizedRepeatedIfExceptionsTest:

3.) rerunner-jupiter selfishly does not care about your annotations and disregards meta-annotations, while @RetryingTest allows you to be free!

4.) rerunner-jupiter offers suspend() which allows you to specify a "cooldown" time in milliseconds before retrying your test again. According to the author: "It matters, when you get some infrastructure problems and you want to run your tests through timeout."

5.) rerunner-jupiter repeats your tests N number of times meaning if you have 1 failure, you will have N-1 skipped test and 1 successful tests. @RetryingTest stops running your tests when it succeeds, meaning 1 skipped tests and 1 successful test.

6.) Related to 5. - rerunner-jupiter lets you define minSuccess meaning it will try to repeat your test until it succeeds at least that many times. If it can't (e.g.: you repeat 5 times, 3 times failing, minSuccess set to 3) the test will fail. Just like previously, this only means the rest of the tests are skipped (instead of not running at all).

[Bukama]

I think we can improve our extension, but I don't see that we move the rerunner-jupiter. To comment your points

1. I don't like the fact that the rerunner-jupiter does not work with AssertJ. AssertJ is the best assertion framework in my opinion. Not supporting it is a no go.

2. I like the possibility that rerunner-jupiter works with parameterized tests. We should have a look to evolve our extension.

3. I like the suspend() functionality, but it should be an opt-in feature, to keep runs fast.

4. I really like the functionality to add a number the tests has to passed, so you can make flaky tests at least somehow more stable. Also as an opt-in.

5. We should keep an eye on the runs, meaning we should avoid runs that are skipped, but stop the tests when it passed.

At the end: Before doing any implementation we should reach out for the maintainer to ask if it's okay for him (didn't check what Apache v2 licences means for usage).

[Michael1993]

Sidenote for improving RetryingTest:
We should (probably) migrate to AssertionFailedError from AssertionError.

[Bukama]

As @Michael1993 and me like to improve this extension I'll updated the labels

[nipafx]

Good points by @Michael1993 and I 100% agree with @Bukama. Let's do it!

I think it would be a good idea to create a new issue for each of the following changes:

• improve interaction with other test templates
• allow defining exceptions for which to retry
• offer suspend-like attribute to configure pauses between reruns
• offer minSuccess-like attribute to configure how often a test has to pass in total (could this or another attribute be in percentages, i.e. "run the test n times and stop once p% of them pass"?)

The last three could be good first issues, but @RetryingTest is not the most trivial extension. 😉

I propose to close this issue as soon as the other four were created.

[Michael1993]

New issues created, closing this.

[nipafx]

@Michael1993 More inspiration? https://github.com/jaksa76/unreliable (Don't have time to check right now, so I just dump the link here.) We can discuss that here and possibly spawn more issues based on that.