search

junit-team / junit4

A programmer-oriented testing framework for Java.
https://junit.org/junit4
Eclipse Public License 1.0
8.53k stars 3.28k forks source link

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

Closed Gaibhne closed 6 years ago

Gaibhne commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

The ordering of rules is address by #1445

Gaibhne commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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