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

@Test annotations on interface default methods #1078

Closed
nwillc opened this issue Jan 31, 2015 · 32 comments
Closed

@Test annotations on interface default methods #1078

nwillc opened this issue Jan 31, 2015 · 32 comments

Comments

@nwillc
Copy link

nwillc commented Jan 31, 2015

I was hoping to make and interface with @test annotated default methods that my tests could implement. Didn't appear to see these tests.

@kcooney
Copy link
Member

kcooney commented Jan 31, 2015

JUnit currently requires JDK5. Most likely, JUnit 5.0 would require JDK 6. We don't want to require more recent JDKs, because many people can't use the latest JDK (including people on Android).

Can you do this without default methos?

@josephw
Copy link
Contributor

josephw commented Jan 31, 2015

@Test isn't marked as @Inherited; that might be all that's necessary to implement this, but it would be a change in behaviour.

@kcooney
Copy link
Member

kcooney commented Jan 31, 2015

Not sure if making @Test inherited would break people. What happens today if you have a base class with an abstract method annotated with @Test and you override it in a subclass without annotating the overridden.method?

@nwillc
Copy link
Author

nwillc commented Jan 31, 2015

Inheriting from a base abstract works today. It's how I implemented
http://nwillc.github.io/jdk_contract_tests

On Sat, Jan 31, 2015 at 10:10 AM, Kevin Cooney notifications@github.com
wrote:

Not sure if making @test inherited would break people. What happens today
if you have a base class with an abstract method annotated with @test and
you override it in a subclass without annotating the overridden.method?


Reply to this email directly or view it on GitHub
#1078 (comment).

@marcphilipp
Copy link
Member

@kcooney The method is run as a test regardless whether it is annotated with @Test in the subclass or not.

@marcphilipp
Copy link
Member

@josephw Adding @Inherited won't help since JUnit only scans super classes for methods with annotations:
https://github.com/junit-team/junit/blob/master/src/main/java/org/junit/runners/model/TestClass.java#L64

@marcphilipp
Copy link
Member

We'd probably need to scan interfaces for test methods, too.

@Stephan202
Copy link
Contributor

Not 100% sure about Java 8's new default methods on interfaces, but the documentation for @Inherited states:

Note that this meta-annotation type has no effect if the annotated type is used to annotate anything other than a class. Note also that this meta-annotation only causes annotations to be inherited from superclasses; annotations on implemented interfaces have no effect.

See also this SO question. Would be good to verify how default methods are treated.

@Stephan202
Copy link
Contributor

Wait, I just realized that you want subclasses to implement the tests, without having to annotate them as a @Test. In that scenario default methods of course don't apply. So in conclusion: @Inherited won't help here.

@Stephan202
Copy link
Contributor

In fact, @Inherited only applies to types, not to methods. From the same documentation:

If an Inherited meta-annotation is present on an annotation type declaration, and the user queries the annotation type on a class declaration, and the class declaration has no annotation for this type, then the class's superclass will automatically be queried for the annotation type.

@nwillc
Copy link
Author

nwillc commented Jan 31, 2015

Didn't mean to stir up a hornets nest and I'm not sure of it's implications
on breaking backwards compatibility, I just like the idea of the pattern of
creating an interface with default methods annotated @test, and being able
to implement an arbitrary set of these in a test class thus "mixing in" a
set of tests. It would allow implementing contract interfaces and applying
these contracts to a class very elegantly.

On Sat, Jan 31, 2015 at 5:15 PM, Stephan Schroevers <
notifications@github.com> wrote:

In fact, @inherited only applies to types, not to methods. From the same
documentation:

If an Inherited meta-annotation is present on an annotation type
declaration, and the user queries the annotation type on a class
declaration
, and the class declaration has no annotation for this type,
then the class's superclass will automatically be queried for the
annotation type.


Reply to this email directly or view it on GitHub
#1078 (comment).

@kcooney
Copy link
Member

kcooney commented Jan 31, 2015

Can you provide example code showing what you would want to do? I don't quite see what you would get that you couldn't get with abstract classes or delegation, but I haven't used default methods so I am probably missing something. I don't quite see the use case for having a test class implement multiple interfaces that have default methods annotated with @Test

@nwillc
Copy link
Author

nwillc commented Feb 1, 2015

Suppose you're code implemented a Comparator of Longs (MyLongComparator),
but also a slew of other comparators you wanted to test. You could create
the following interface:

interface ComparatorContract {

Comparator<T> getComparator();
T getValue();
T getLesserValue();

@Test
default void shouldReturnZeroOnEquality() throws Exception {

assertThat(getComparator().compare(getValue(),getValue())).isEqualTo(0);
}

@Test
default void shouldReturnNegativeOnLessThan() throws Exception {
    assertThat(getComparator().compare(getLesserValue(),

getValue()).isLessThan(0);
}

@Test
default void shouldReturnPositiveOnGreaterThan() throws Exception {
    assertThat(getComparator().compare(getValue(),

getLesserValue())).isGreaterThan(0);
}
}

And then implement tests like:

public MyLongComparatorTest implements ComparatorContract {

Comparator getComparator() { return new MyLongComparator(); }

Long getValue() { return 1L; }

Long getLesserValue { return 0L; }

}

Now this could be done with inheritance, but perhaps you also want to
enforce that all your classes have names shorter than 25 characters....
using interfaces you could simply create:

interface NameLengthContract {

Class getTestClass();

@Test
default void shouldBeUnderLimit() throws Exception {
    assertThat(getTestClass().getSimpleName().size()).isLessThen(26);
}

}

And enhance your test to:

public MyLongComparatorTest implements ComparatorContract,
NameLengthContract {

Comparator getComparator() { return new MyLongComparator(); }

Long getValue() { return 1L; }

Long getLesserValue { return 0L; }
Class getTestClass() { return MyLongComparator.class; }

}

With inheritance to achieve this you'd need one test class per contract.

On Sat, Jan 31, 2015 at 6:34 PM, Kevin Cooney notifications@github.com
wrote:

Can you provide example code showing what you would want to do? I don't
quite see what you would get that you couldn't get with abstract classes or
delegation, but I haven't used default methods so I am probably missing
something. I don't quite see the use case for having a test class implement
multiple interfaces that have default methods annotated with @test


Reply to this email directly or view it on GitHub
#1078 (comment).

@marcphilipp
Copy link
Member

Makes sense to me.

@Tibor17
Copy link
Contributor

Tibor17 commented Feb 1, 2015

You could do that with @ Inherited annotation because it is Java 5 annotation. The same can be done with @ Ignored.
Unfortunately getSuperClasses() substituted this and it's even worse because you should now find out what strange user's class constructs/inhertiace will be broken.
https://github.com/junit-team/junit/blob/master/src/main/java/org/junit/runners/model/TestClass.java#L64

@Stephan202
Copy link
Contributor

@Tibor17, have a look at my comments a bit further up: @Inherited is unfortunately of no use here.

Can you elaborate on the issues you anticipate with simply modifying #getSuperClasses() to also include interfaces?

@Tibor17
Copy link
Contributor

Tibor17 commented Feb 1, 2015

@Stephan202
I know very well that JUnit does not use @Inherited in TestClass. I am ware of @Category using @Inherited. I am saying in my previous post that I like using Inherited but the most important thing is to find scenarios which break the current behavior.
Sorry but we are fighting with maven and surefire plugin having ambitions to be quite different in the future.

@Stephan202
Copy link
Contributor

@Tibor17: I'm afraid we're talking past each other. I did not mean to say that JUnit currently doesn't use @Inherited for @Test (though that's true, of course ;) ). I meant that we cannot use it, even if we wanted to, because it does not have the semantics we desire. @Inherited applies to classes only, and we always use @Test on a method.

@Tibor17
Copy link
Contributor

Tibor17 commented Feb 1, 2015

@Stephan202
Your right, I forgot that Inherited applies to classes, goo catch. 👍

@marcphilipp
Copy link
Member

We are currently working on a plan to have a JUnit extension/version for Java 8. This would be a good fit!

@marcphilipp marcphilipp added this to the JDK 8 milestone Feb 15, 2015
@jleskovar
Copy link

Are there also plans to support @before and @after default methods in the Java 8 extension?

@marcphilipp
Copy link
Member

If we support @Test we should probably support all method-level-annotations.

@nwillc
Copy link
Author

nwillc commented Feb 15, 2015

Nice. When it's testable I'll jump on it.

On Sun, Feb 15, 2015 at 3:17 AM, Marc Philipp notifications@github.com
wrote:

We are currently working on a plan to have a JUnit extension/version for
Java 8. This would be a good fit!


Reply to this email directly or view it on GitHub
#1078 (comment).

@gvlasov
Copy link

gvlasov commented May 31, 2015

I'd like this implemented, too, because that would make reusing tests much easier. My interface hierarchy is quite convoluted (a geometry library), so each class must be tested for compliance to the contracts of its interface tree. In the current state of JUnit, I can create abstract classes that contain tests for interfaces, and inherit those classes each time an implementation implements an interface, but it causes a lot of boilerplate code anyway. If I could add test methods as mixins, there would be zero boilerplate in my tests.

@kcooney
Copy link
Member

kcooney commented May 31, 2015

@Suseika having a test implement multiple interfaces with default methods can have some of the same problems as multiple inheritance. I wonder if we can find other ways to solve your problem.

I suggest starting a thread that describes your problem either on the mailing list or on StackOverflow.com and adding a link to the thread here.

@marcphilipp marcphilipp modified the milestones: JUnit Lambda, JDK 8 Jun 14, 2015
@danieldietrich
Copy link

Makes sense to me.

+1

@Tibor17
Copy link
Contributor

Tibor17 commented Nov 22, 2015

@test might be interesting in JUnit 4.x because the super method can be reused in parameterized test, but in JUni 5.x I guess we can resolve this issue and problem around inheritance by

  • not having the annotation in specific construct
  • not having test class, only interface instead

Till now the class was only an umbrella for us and mostly not inherited.
The test method just declared itselve as a test and mostly all public methods were tests with no difference except for name and all had the only message for JUnitCore "I am a test".

As an alternative concept
Imaging interface org.junit.JUnitTest having methods compiled in Java 8:

public abstract void test(String name, Function fun);
public abstract void test(String name, JUnitProvider provider, Function fun);
public abstract void test(String name, Function<?> fun); // parameterized runner with one param
, where JUnitProvider may have all: params, Rule instances, other instances as necessary.
Additionally public abstract JUnitTest $(JUnitProvider provider); - see the purpose later.

The JUnitCore will use java.lang.reflect.Proxy with Handler.
This handler specifically is looking for the calls made to these three methods, whatever implementation is inside, and whatever runner is specified. This means the user calls parameterized test in class/interface like this:

test( "Lambda test", (param) -> { ... assertion ... } );
Here we have name which needs to be converted to Description object important for assotiating with junit listeners. So the Handler in Proxy knows now what test has been called and assotiates thrown exception with Description and suppress it due to chaining the calls.

public interface MyTest extends JUnitTest {
  @Override
  @RuleStereotype
  public MyTest $(JUnitProvider provider) {
    return test( "Lambda test", () -> { ... assertion ... } ); // pure test
               .test( "Lambda parameterized test", (param) -> { ... assertion ... } ); // parameterized
  }
}

No @test annotation, no class, only multiple interface inheritance.
You declared order, multiple runners. This is sequential exec but can be parallel in Lambda as well.
Since you have Proxy Handler, the JUnit is aware of the test method calls inside of the interface.
The JUnitProvider has instances inside like @rule , ...
Since the Proxy can call all methods and record the names, the tests can be filtered and finally executed in parallel. This is completely different type of Runner.
JUnitTest can have default functions for Parameterize, Theories, etc, which return empty collection of parameters but can be overridden. This is an extension of multiple runners and the user is implementing the final runner in $, for instance nested tests == Suite, with the help of JUnitProvider.

@chrisvest
Copy link
Contributor

Default interface methods as tests expands the ability to express complex test matrices, which is why I went ahead and did #1222.

Theories have limitations in their ability to express complex fixture construction, and have an awkward tooling story. Parameterised tests are difficult to extend. Abstract tests are limited to a linear hierarchy. Allowing default interface methods as tests would pretty much lift the linear hierarchy restriction of the abstract test pattern.

I don't know @Suseika particular use case, but geometry sounds like a domain where this would be useful.

Java8 has already decided how name clashes should be handled. If a class gets default methods with the same signature from two different interfaces, then that method gets re-abstracted and the concrete implementing class has to implement the method and resolve the conflict that way. When it comes to tests, and you have a name conflict in default methods, you can either rename one of the tests if thats in your power, or you can implement the test method to call the test methods from both interfaces, or create two new test methods call the respective methods on the interfaces.

For example, if the interfaces A and B both provide a test called tt, our implementing test class can resolve the conflict either like this:

public class MyTest implements A, B {
  @Override
  public void tt() {
    A.super.tt();
    B.super.tt();
  }
}

Or like this:

public class MyTest implements A, B {
  @Override
  public void tt() {
  }

  @Test
  public void ttA() {
    A.super.tt();
  }

  @Test
  public void ttB() {
    B.super.tt();
  }
}

However, I think that this will not be a very common problem. Tests usually have long and descriptive names, so I suspect that name conflicts are rare.

The use case that drove me to write a pull request for this feature is the following: I am implementing my own page cache, which does off-heap caching of file data, and therefore has to interface with file systems. To speed things up, I do most of my testing on an in-memory file system, but it is also important that I run my tests on a real file system, since the in-memory implementation might not be a faithful substitute. Further more, I also want to run my tests in a mode where the real file system is doing fault-injection in a different way than usual. The API for my page cache is described and specified in interfaces, and most of the tests are written in terms of those APIs, which means they are decoupled from the implementation.

So far, this gives me the following abstract test hierarchy:

AbstractPageCacheTest
┗ MyPageCacheTest
  ┗ MyPageCacheWithRealFileSystemTest
    ┗ MyPageCacheWithRealAndFaultInjectingFileSystemTest

This works, but I suffer from some of the tests being slow – even when run with the in-memory file system. So I'd like to extract those into IT tests, and run them less often. And I'd like to make all the tests that use a real file system run as ITs as well. I still want to reuse as much of the set-up and fixture code as possible. With default interface methods as tests, I could make the following hierarchy, which would satisfy these requirements without any duplication:

               PageCacheTestSupport (interface)
                 ┃               ┃
AbstractPageCacheTest         PageCacheSlowTestsMixin (interface)
┗ MyPageCacheTest                   ┃   ┃    ┃
  ┗ MyPageCacheWithRealFileSystemIT ┛   ┃    ┃
    ┃                                   ┃    ┗ MyPageCacheSlowIT
    ┃                                   ┃
    ┗ MyPageCacheWithRealAndFaultInjectingFileSystemIT

I could alternatively have a separate hierarchy for the slow tests, and this is actually what I currently do, but it means that I have to duplicate the set-up for making the tests use the real file system, and likewise for the fault injection in the real file system.

That's why I desire this feature.

@Tibor17
Copy link
Contributor

Tibor17 commented Nov 22, 2015

@marcphilipp is right. The inheritance is for classes.
I guess nobody has used @test on Java7 super-interface. So this should not break current users.
My post above was especially due to new style of writing tests with Lambda. I do not have any problem with @chrisvest 's proposal.

@marcphilipp
Copy link
Member

Our new prototype "JUnit Lambda" supports all annotations (@Test, @BeforeEach, ...) on interface default methods.

Please take a look here:
https://github.com/junit-team/junit-lambda/blob/f2572cc953ee3773a3e6f17016ce1516999b494c/sample-project/src/test/java/com/example/SucceedingTestCase.java#L113-L130

@kcooney
Copy link
Member

kcooney commented Mar 14, 2016

I vote to close this, since JUnit lambda does this.

@marcphilipp
Copy link
Member

JUnit 5 supports writing tests as interface default methods:
http://junit-team.github.io/junit5/#interface-default-methods

Therefore, I close this issue.

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

10 participants