You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
IsTestableMethod silently ignores @Test annotated methods that return a value (or are static or are private).
IsTestClassWithTests uses IsTestMethod to discover tests. IsTestMethod is a subclass of IsTestableMethod that sets IsTestableMethod.mustReturnVoid to true.
If you annotate a method with @Test it's clear you want it run as a test. So if you've mistakenly annotated a method that doesn't conform to the requirements of a test then this should result in an error. It shouldn't be just a warning, it should actively fail the test run. However, at the moment such methods are simply silently ignored.
Steps to reproduce
Run the following test class, it contains four tests but only one of them will be run. There is no warning or indication that the other three have been skipped - they are silently ignored despite being annotated with @Test. One is ignored because it returns a value, one because it's static and one because it's private.
This issue came up in Kotlin code where the value issue silently turned up when I made what I thought was an unproblematic change. I started with something like this:
Obviously fooTest does more than just invoke the passed in lambda but that's the core of it. We had a huge number of tests like this and everything worked fine. Then, in certain cases, I wanted to call fooTest more than once within a single test and I wanted it to return a result that could be checked in such tests (and ignored in all the existing tests). So I made this change, there were no warnings but suddenly many of our tests stopped running. They stopped because the existing tests now no longer returned void.
If you've annotated something with @Test you clearly want it run - it's a mistake if you've used @Test with something that can't be run as a test and it should fail the test run (printing a warning isn't enough - in a huge Maven build these things can get lost).
I would suggest changing the logic for IsTestableMethod.test to something like this:
@Overridepublicbooleantest(Methodcandidate) {
if (!isAnnotated(candidate, this.annotationType)) {
returnfalse;
}
// Beyond this point you know the test annotation is present.// Please do not collapse the following into a single statement.if (isStatic(candidate)) {
thrownewJUnitException(candidate.getName() + " cannot be static");
}
if (isPrivate(candidate)) {
thrownewJUnitException(candidate.getName() + " cannot be private");
}
if (isAbstract(candidate)) {
returnfalse;
}
if (returnsVoid(candidate) != this.mustReturnVoid) {
thrownewJUnitException(candidate.getName() + " must return " + (this.mustReturnVoid ? "void" : "a value"));
}
returntrue;
}
The text was updated successfully, but these errors were encountered:
IsTestableMethod
silently ignores@Test
annotated methods that return a value (or are static or are private).IsTestClassWithTests
usesIsTestMethod
to discover tests.IsTestMethod
is a subclass ofIsTestableMethod
that setsIsTestableMethod.mustReturnVoid
totrue
.If you annotate a method with
@Test
it's clear you want it run as a test. So if you've mistakenly annotated a method that doesn't conform to the requirements of a test then this should result in an error. It shouldn't be just a warning, it should actively fail the test run. However, at the moment such methods are simply silently ignored.Steps to reproduce
Run the following test class, it contains four tests but only one of them will be run. There is no warning or indication that the other three have been skipped - they are silently ignored despite being annotated with
@Test
. One is ignored because it returns a value, one because it'sstatic
and one because it'sprivate
.This issue came up in Kotlin code where the value issue silently turned up when I made what I thought was an unproblematic change. I started with something like this:
Obviously
fooTest
does more than just invoke the passed in lambda but that's the core of it. We had a huge number of tests like this and everything worked fine. Then, in certain cases, I wanted to callfooTest
more than once within a single test and I wanted it to return a result that could be checked in such tests (and ignored in all the existing tests). So I made this change, there were no warnings but suddenly many of our tests stopped running. They stopped because the existing tests now no longer returned void.If you've annotated something with
@Test
you clearly want it run - it's a mistake if you've used@Test
with something that can't be run as a test and it should fail the test run (printing a warning isn't enough - in a huge Maven build these things can get lost).Context
Build Tool/IDE:
Deliverables
I would suggest changing the logic for
IsTestableMethod.test
to something like this:The text was updated successfully, but these errors were encountered: