gchq / Gaffer

A large-scale entity and relation database supporting aggregation of properties
Apache License 2.0
1.77k stars 353 forks source link

JUnit 5 causing overridden tests to be missed #2452

Closed t92549 closed 3 years ago

t92549 commented 3 years ago

In JUnit4, when an abstract function had an @Test annotation, any subclasses that implemented the function would inherit the @Test annotation and be tested. This appears to be unintentional and JUnit5 does not work this way: https://github.com/junit-team/junit5/issues/960

This has left the Gaffer codebase with many tests that aren't running. For example in GetAllElementsTest the shouldShallowCloneOperation test in overridden but not tested:

https://github.com/gchq/Gaffer/blob/eec0c3f1e9851e1310b7345b1fe97d812c497608/core/operation/src/test/java/uk/gov/gchq/gaffer/operation/impl/get/GetAllElementsTest.java#L81-L82

But in the same file, the builderShouldCreatePopulatedOperation is overridden and correctly has the @Test annotation added: https://github.com/gchq/Gaffer/blob/eec0c3f1e9851e1310b7345b1fe97d812c497608/core/operation/src/test/java/uk/gov/gchq/gaffer/operation/impl/get/GetAllElementsTest.java#L69-L71

All tests which have @Override annotations and are expecting to inherit @Test but aren't should have @Test added to their annotations.

n3101 commented 3 years ago

@t92549 Is it easy to identify such cases, and if so how many are there?

t92549 commented 3 years ago

@n3101 I think the only real way to gauge it is just to search for @Override in all of the /test directories. Not all of these instances will be tests that are missing the @Test tag, in fact it seems quite a few aren't, however, there are 1234 results that will probably just have to be manually checked. There might be some sort of smart code scanning way to do this (find all functions that inherit from a function with a @Test tag), however, I don't know of one personally.