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

Update TestWatcher Javadoc to specify that it should be the "outer" rule #1436

Closed
Gaibhne opened this issue Apr 2, 2017 · 6 comments
Closed
Milestone

Comments

@Gaibhne
Copy link

Gaibhne commented Apr 2, 2017

Depending on the field name of the ExpectedException rule, the rule works or breaks when used in conjunction with a TestWatcher. More details, as well as a test case demonstrating the faulty behaviour, can be seen at:

http://stackoverflow.com/questions/43088543/junit-test-both-passes-and-fails-conflict-using-both-expectedexception-and-tes

@kcooney
Copy link
Member

kcooney commented Apr 2, 2017

The ordering of rules is undefined. If you need to enforce ordering between rules, you can use RuleChain. We could perhaps make it easier to discover this. Any suggestions?

@kcooney
Copy link
Member

kcooney commented May 2, 2017

@Gaibhne do you have any suggestions for how you could have discovered RuleChain as the solution to your problem (Java doc improvements, updates to the wki, etc)?

@panchenko
Copy link
Contributor

panchenko commented May 14, 2017

The ordering of rules is address by #1445

@Gaibhne
Copy link
Author

Gaibhne commented May 14, 2017

While ordering fixes the issue, what happens is not actually something that SHOULD be something that depends on the order of the rules, so there is no way I would have ever gotten the idea to read any documentation relating to rule orders.

Don't get me wrong, as a programmer, I see why it happens the way it does, but that is only in hind sight. Nothing but a meaningful error or warning message could have pointed me towards looking into rule ordering mechanisms.

I'm not sure if it is feasible to produce any sort of error or warning for a scenario like this. In the end, the bug is that the TestWatcher follows different rules, no pun intended, about what is considered a failure or a success than JUnit.

@panchenko
Copy link
Contributor

This issue can be addressed in the following ways:

  1. If TestWatcher is implemented as a rule it can access the final result only it is the outermost one. This limitation can be added to the class javadoc.
  2. Extend ordering of rules, so a rule implementation can specify how it should be ordered.
  3. Deprecate TestWatcher and implement replacement via RunListener. Would be the best approach.

@kcooney kcooney changed the title ExpectedException bug based on field name (!) Update TestWatcher Javadoc to specify that it should be the "outer" rule Aug 7, 2017
@kcooney kcooney added this to the 4.13 milestone Aug 7, 2017
@kcooney
Copy link
Member

kcooney commented Aug 7, 2017

I think updating the Javadoc of TestWatcher makes since so I've updated the summary of this issue accordingly.

stefanbirkner added a commit to stefanbirkner/junit that referenced this issue Feb 13, 2018
TestWatcher may see failed tests as successful and vice versa if it is
not the outermost rule and another rule changes the result of a test
(e.g. ErrorCollector or ExpectedException).

Fixes junit-team#1436.
stefanbirkner added a commit to stefanbirkner/junit that referenced this issue Feb 14, 2018
TestWatcher may see failed tests as successful and vice versa if it is
not the outermost rule and another rule changes the result of a test
(e.g. ErrorCollector or ExpectedException).

Fixes junit-team#1436.
stefanbirkner added a commit to stefanbirkner/junit that referenced this issue Feb 15, 2018
TestWatcher may see failed tests as successful and vice versa if it is
not the outermost rule and another rule changes the result of a test
(e.g. ErrorCollector or ExpectedException).

Fixes junit-team#1436.
stefanbirkner added a commit to stefanbirkner/junit that referenced this issue Feb 17, 2018
TestWatcher may see failed tests as successful and vice versa if it is
not the outermost rule and another rule changes the result of a test
(e.g. ErrorCollector or ExpectedException).

Fixes junit-team#1436.
sebasjm pushed a commit to sebasjm/junit4 that referenced this issue Mar 11, 2018
TestWatcher may see failed tests as successful and vice versa if it is
not the outermost rule and another rule changes the result of a test
(e.g. ErrorCollector or ExpectedException).

Fixes junit-team#1436.
aristotle0x01 pushed a commit to aristotle0x01/junit4 that referenced this issue Jun 27, 2022
TestWatcher may see failed tests as successful and vice versa if it is
not the outermost rule and another rule changes the result of a test
(e.g. ErrorCollector or ExpectedException).

Fixes junit-team#1436.
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

3 participants