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

@ParameterizedRepeatedIfExceptionsTest test status is failed for any repetion #45

Closed
recumbentseeker opened this issue Sep 10, 2019 · 14 comments

Comments

@recumbentseeker
Copy link

Using the following annotation
@ParameterizedRepeatedIfExceptionsTest (repeats = 4, minSuccess = 1, name = "{0}", repeatedName = "repeat", exceptions = AssertionError.class)

If for one value the test runs 3 times - failure, failed repeat, successfull repeat - this results in 2 failed and one passed test.

I would expect one passed test as for @RepeatedIfExceptionsTest.

@Tofel
Copy link

Tofel commented Dec 16, 2019

Well, if you ask me, these line is to blame:

https://github.com/artsok/rerunner-jupiter/blob/master/src/main/java/io/github/artsok/extension/ParameterizedRepeatedExtension.java#L136

just change it to

throw new TestAbortedException("Do not fail completely, but repeat the test", throwable);

and you'll get the desired effect, of the failed execution being treated as skipped.

@MeikeMertsch
Copy link

Any chance we can have this enhancement?

@Tofel
Copy link

Tofel commented Jan 16, 2020

If you badly need it feel free to grab this fork: https://github.com/Tofel/rerunner-jupiter

It is also thread safe-ish (I sometimes do run into issues with test repetition if they take a long time to execute and somehow they are missing from extension context), so you can run tests in parallel. It works nicely most of the time.

artsok added a commit that referenced this issue Jan 16, 2020
@artsok artsok closed this as completed Jan 16, 2020
@MeikeMertsch
Copy link

I bear bad news. I didn't notice a bad side effect when I made that PR 🙈: with parameterized tests, if none of the repetitions succeed, the last one does not fail but gets skipped as well.
I'm sorry.

@Tofel do you have a solution for this? You seem to have a much better grasp of this than I have. Do you mind making a PR with your enhancements? Your fork is not really on mvnrepo.

@MeikeMertsch
Copy link

MeikeMertsch commented Jan 17, 2020

alright. I have something on my master branch that I stole from @artsok 's RepeatIfExceptionExtension. It does do the trick. Almost.

  1. I was not able to write tests for the ParameterizedRepeatedIfExceptionsTest with that assertTestResults method (I am not sure how to call it properly for a parameterized test). That kinda bugs me.

  2. When the last iteration fails at least once but then succeeds, I always get an IndexOutOfBoundsException (ParameterizedRepeatedExtension.java:281 in my fork). And so far I cannot quite figure out where it goes wrong. Currently I assume that "hasNext()" in TestTemplateIteratorParams returns true in that case when it really shouldn't but I am not yet sure how to express that in code. Would any of you have a look and see if you can figure it out?

@MeikeMertsch
Copy link

When writing the last comment, my thoughts got so ordered about it that I believe I fixed the issue now properly. The issue with missing tests for the whole issue remains 😞

@Tofel
Copy link

Tofel commented Jan 17, 2020

Ha! Was about to sit to it and fix what you found, because of course in my version tests would never fail 🤦‍♂ .

If now you combine your solution with mine (or with every class variable packed in ThreadContainer) you should get a thread-safe version :-)

@Tofel
Copy link

Tofel commented Jan 17, 2020

Ah... I have my version on a private Maven. I think once your or mine is 100% functional it is better to create a PR to @artsok's repo than to deploy our forked versions to Maven Central.

@MeikeMertsch
Copy link

Maybe you can run a second PR against Artem's code?
And I am still unhappy that I didn't manage to understand how to write tests like for example https://github.com/artsok/rerunner-jupiter/blob/master/src/test/java/io/github/artsok/ReRunnerTest.java#L92-L102 for parameterized tests (yet). So I feel a bit naked about it.

@MeikeMertsch
Copy link

I don't think I'd ever deploy a fork to Maven Central (I wouldn't even know how). But I'm sure thread-safety will be appreciated

@Tofel
Copy link

Tofel commented Jan 17, 2020

Okay, will have a look tomorrow at the test and will create a PR, as currently the code is super ugly :-)

At some point I'll also have to solve this issue, which happens every now and then:

org.junit.platform.commons.PreconditionViolationException: Illegal state: required test method is not present in the current ExtensionContext

@MeikeMertsch
Copy link

oh, I haven't seen that one yet. Will the test then fail or does it throw an error? Just so I can be on the look out for it and in Intellj I'd look for a yellow or a red outcome ;)

@Tofel
Copy link

Tofel commented Jan 17, 2020

It fails and is not repeated, because that exception is thrown from the ParameterizedRepeatedExtension class. I mean this is not something that should ever happen, that a test method is not present in the ExtensionContext. I need to debug it, but for me it happened only when running 350+ tests suite in parallel in Jenkins.

@MeikeMertsch
Copy link

Ha! I found out what I did wrong with the tests. Found a possibility to write tests for what I did. Just wanna prove they'll run through maven as expected before I push them.

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

No branches or pull requests

4 participants