Add withExceptions attribute to @ExpectedToFail

[knutwannheden]

Adds a new onExceptions attribute to @ExpectedToFail using which the scope of the extension can be limited to only consider the test as successful if the thrown exception matches one of the given types.

Fixes: #769

[knutwannheden]

This is still in draft state, as I will need to add more documentation and tests. I just wanted to make sure I am not pulling in the wrong direction with this and also check if you already have some input on this.

[Michael1993]

I am undecided if I like this proposal. I really appreciate you taking the initiative and I'm not outright opposed to it, it's just that I worry that despite this being something for a specific use-case, people will be tempted to use it as a "fix-all". Does that make sense?

[knutwannheden]

@Michael1993 I definitely agree with your review comments. I just slapped this together to better convey the idea and to have something concrete to discuss.

I understand your concern but I honestly don't quite see the "fix-all" problem. If anything it is JUnit's own @Disabled I see as a fix-all, that gets used (and then forgotten!) far too often. Also @ExpectedToFail annotations can linger on for a long time, which I think is OK, since they already narrow down the scope a lot (the test actually must fail). Now, this onExceptions attribute narrows this scope down even more, because the test must fail with a specific exception.

My only "concern" as a maintainer would be that the use case is probably rather unusual. But as I think I already mentioned in the issue, JUnit 4's @Test(expected) actually allowed this (even though it was probably primarily designed for what today assertThrows() should be used for) and I found it extremely useful when for example working on a new implementation for some API spec.

[Michael1993]

Okay, I see your point. Please go ahead with the PR. 😄

[Michael1993]

Please apply spotless and add documentation, if you can. Thank you!

[Michael1993]

I'm happy with current state of the PR. I'd ask you to add yourself to the README as a contributor (but that's not mandatory, only if you want to). Thank you for your patience and hard work!

[knutwannheden]

Thank you for your patience and hard work!

Thanks a lot for your thoroughness and patience!

[Bukama]

Running full run

[nipafx]

Hey @knutwannheden, thank you for opening the PR - it looks really good. 😃 A few things could be changed - see below.

But, importantly, can you please open another PR from a feature branch and not main? Thanks!

[nipafx]

Oh, and if you rebase it on the current main, you can fix the merge conflict - it comes from the README, which now .adoc not .md.

[knutwannheden]

@nipafx Thanks for your review comments. I assume I should wait until the other maintainers have chimed in. In the meantime I've updated the PR with the latest changes from main.

[Bukama]

@nipafx Thanks for your review comments. I assume I should wait until the other maintainers have chimed in. In the meantime I've updated the PR with the latest changes from main.

Well what @nipafx didn't wrote: We literally had a team meeting online stream about all Pioneer things yesterday :)

[nipafx]

I managed to push a small fix on the README order to your main branch, but it's a bit cumbersome. We can keep this PR as is, but please keep in mind for the next one, not to open it from your main branch.

Other than that, I will let you take a look at the two remaining comments I made above.

[Michael1993]

If you have the time, please write a few more tests to cover the lines marked in the Sonar analysis.

[knutwannheden]

@Michael1993 Sorry about the long delays!

[beatngu13]

Proposed commit message:

Add withExceptions to ExpectedToFail (#769 / #774)

Adds a new `withExceptions` attribute to `@ExpectedToFail`, which
allows to limit the scope of the extension to only consider the test
as successful if the thrown exception matches one of the given types.

Closes: #769
PR: #774

[beatngu13]

@knutwannheden thanks again! 🙏