Junit5: Document minimum requirements for implementing a TestEngine

[codecholeric]

This is kind of a follow up on #1109 ... I didn't get around to this up to now, but now I have a working POC of a TestEngine with similar features to the custom JUnit 4 runner.

You can check this out here
https://github.com/TNG/ArchUnit/tree/junit5-support/archunit-junit/junit5/src/main/java/com/tngtech/archunit/junit

I like the power, e.g. that I can just mark @AnalyzeClasses with @Testable and my IDE will pick it up, and the engine will be 'magically' discovered, without the need to specify sth. like the old ArchUnitRunner :smiley:

However, with regards to the TestEngine itself, I'm kind of unsure. Maybe I've overlooked some documentation here, the design seems very flexible, but that also makes it hard to know which contract to adhere to.

As an example: TestDescriptor offers an Optional<TestSource> getSource() with the JavaDoc "Get the source of the test or container described by this descriptor, if available.". So I assumed, I could leave that out for my first draft, implement only the most necessary. And the tests ran in IntelliJ and they ran in Maven. However, in Gradle they were silently skipped. It took me some digging into the Gradle sources to find out, that Gradle uses the test source, to determine, if this test is a 'composite' or not. And if the source is absent, an AssertionError is thrown, which in turn gets silently swallowed.

Or another one, I'm pretty sure, that the "old" Gradle JUnit 5 plugin (before Gradle 4.6) would call the engine with a ClasspathRootSelector, while with Gradle 4.6 this suddenly changed to a set of ClassSelector.

For my POC now, I've only added support for ClassSelector, the question is, do I need to add support for all the selector types?
I'm just not sure, how I can be certain, that I covered the minimum to run on all common platforms. Do I really need to add support for UriSelector for example, or is this irrelevant for all common IDEs and Build tools anyway?

It feels a little bit, that to implement a TestEngine, one has to study the Jupiter engine and copy the behavior, since sticking to API + Javadoc does sometimes lead to surprising results (which can of course also be my fault for not completely understanding the concepts :wink: )

Anyway, any tipps what I really need to be somewhat on the safe side (that this will behave natural for daily development tasks) would be highly appreciated!!

[marcphilipp]

Thanks for the feedback! I think you're right that we've reached a point where it makes sense to document recommendations/guidelines on how to implement a class-based engine.

TestDescriptor offers an Optional<TestSource> getSource() with the JavaDoc "Get the source of the test or container described by this descriptor, if available.". So I assumed, I could leave that out for my first draft, implement only the most necessary. And the tests ran in IntelliJ and they ran in Maven. However, in Gradle they were silently skipped. It took me some digging into the Gradle sources to find out, that Gradle uses the test source, to determine, if this test is a 'composite' or not.

AFAIK Gradle plans to support test engines that work with files. Thus, this might need to be changed anyway. However, please open an issue with this one for Gradle to be sure. IMHO this should not be necessary.

I'm pretty sure, that the "old" Gradle JUnit 5 plugin (before Gradle 4.6) would call the engine with a ClasspathRootSelector, while with Gradle 4.6 this suddenly changed to a set of ClassSelector.

Yeah, Gradle and Maven use ClassSelectors because it was easier to support their existing execution model this way. Both do this in order to be able to partition the set of test classes and submit subsets to a number of forked VMs to support parallel execution.

Currently, I would give you the following recommendations on selectors to implement:

• All engines:
  
  
  
  • UniqueIdSelector, to re-run tests in IDEs
  
  
• Class-based engines:
  
  
  
  • ClasspathRootSelector, for ConsoleLauncher and junit-platform-gradle-plugin
  
  
  • ClassSelector, for Gradle and Maven
  
  
  • Nice-to-have:
    
    
    
    
    
    • MethodSelector, for IDEs in case methods can be executed
    
    
    
    • PackageSelector
    
    
    
    • ModuleSelector
    
    
    
  
  
• File-based engines:
  
  
  
  • FileSelector
  
  
  • DirectorySelector
  
  
  • UriSelector
  
  
  • ClasspathResourceSelector
  
  

@junit-team/junit-lambda Would you agree?

I think we should document these "basic assumptions" in more detail. Moreover, I think it would make sense to publish a tutorial on how to write a custom class-based test engine.

[codecholeric]

Thank you for your tips :smiley:
I think the reference to Cucumber at https://github.com/gradle/gradle/issues/4773, made it more clear to me, what the intentions of these categories of selectors are, in particular, that there are two bigger categories at all (thanks for your enumeration!!)
Maybe it would have helped me, to see these categories somehow in the code, but I don't know if that is against the generic design (e.g. if there was a common interface ClassBasedDiscoverySelector and FileBasedDiscoverySelector, or a package, etc., with the respective Javadoc). The way I read through the API, I wasn't sure, if I might have to support a FileSelector for a class file, for example.
I'll implement your suggestions then, i.e. all the selectors for class-based engines, non of the file-based ones.
I've also opened an issue with Gradle, to hear their thoughts: https://github.com/gradle/gradle/issues/4912
If there's anything I can do, to support the documentation, etc., let me know.

[sbrannen]

As an example: TestDescriptor offers an Optional<TestSource> getSource() with the JavaDoc "Get the source of the test or container described by this descriptor, if available.". So I assumed, I could leave that out for my first draft, implement only the most necessary. And the tests ran in IntelliJ and they ran in Maven. However, in Gradle they were silently skipped. It took me some digging into the Gradle sources to find out, that Gradle uses the test source, to determine, if this test is a 'composite' or not. And if the source is absent, an AssertionError is thrown, which in turn gets silently swallowed.

I would say that's a bug in Gradle: a TestSource is not required. That's why it's an Optional.

[sbrannen]

@marcphilipp, regarding the _missing_ documentation, I agree that we really need to add that, and the "categories" you've laid out are very useful for the uninitiated.

It's true that the selectors are confusing for pretty much anyone other than a core committer. This isn't the first time we've seen someone thinking that a Java class could be selected as a "file" or that a Java package could be selected as a "directory".

With regard to a tutorial, we should definitely put it in the backlog, but I'd consider that _nice to have_ for the moment.

[sbrannen]

I've also opened an issue with Gradle, to hear their thoughts: gradle/gradle#4912

Thanks!

[codecholeric]

I was wondering, at first I screwed up @Tag handling (by forgetting to even handle those in my TestEngine), but after I collected the tags for each TestDescriptor, I noticed, that the junit-platform-launcher would still execute all the tests. After peeking into the Jupiter engine, I realized, that for example the MethodBasedTestDescriptor adds the tags of the parent to its own.
After I had added this to all my test descriptors, the tagging worked as expected. However I kept wondering, if this is not a generic principle, that should have been moved into the platform-launcher? Since it seems, that tags should always propagate through descriptor hierarchies, or is there any case where this is not true?

Also I've noticed, that the @Target restriction of @Tag (TYPE and METHOD), make @Tag somewhat unsuitable for my case, since I also have tests in the form of fields. I've noticed that in different places, I guess fields were never considered to represent tests. However considering @Tag, would you consider it hurtful, to allow it on fields as well?

I had the same problem with @Ignore in JUnit 4 and again with @Disabled, that's why I had to introduce @ArchIgnore.

If @Tag is not gonna be allowed on fields, I probably have to introduce @ArchTag the same way, to do the same as @Tag, but also support annotating fields.

[sbrannen]

Since it seems, that tags should always propagate through descriptor hierarchies, or is there any case where this is not true?

I'd say that depends wholly on the semantics defined and implemented by the actual TestEngine.

In other words, "tagging" is a platform-level concept, but "inheritance of tags" can (and likely should) be engine-specific behavior.

In the very least, that's how the team has approached these topics up to this point.

[sbrannen]

Also I've noticed, that the @Target restriction of @Tag (TYPE and METHOD), make @Tag somewhat unsuitable for my case, since I also have tests in the form of fields. I've noticed that in different places, I guess fields were never considered to represent tests. However considering @Tag, would you consider it hurtful, to allow it on fields as well?

@Tag is part of the JUnit Jupiter API and therefore part of its programming model.

Since Jupiter does not support fields as _testable_ elements of its programming model, there has not yet been a need to allow @Tag to be declared on a field.

Generally speaking, the team does its best to ensure that annotations can only be declared in meaningful places, since that helps to reduce user error.

To answer your question, yes I would consider it potentially harmful to allow @Tag to be declared on a field if there is no feature in Jupiter that actually supports that use case.

But if you feel strongly about it, feel free to open a new issue dedicated to that topic.

[codecholeric]

Okay, if that is a conscious approach about tagging... I would really be interested in a case, where there is a hierarchy of TestDescriptors, where tagging should not propagate...
You're right about @Tag, I didn't pay close attention there, I agree with you then, that it is harmful :wink:
Jupiter API should be tailored to Jupiter, and Jupiter doesn't know field tests, so it would decrease understandability.
On the other hand, that means that maybe I let something creep into my TestEngine, that shouldn't have, maybe I should immediately separate Jupiter API then, and introduce @ArchTag and @ArchIgnore instead of @Tag and @Disabled.
What do you think?
I think I got confused, since you can also just run plain Jupiter tests and use the ArchUnit core components for tests. But then you don't have field tests and similar. I thought it's easier to learn, if it's familiar from Jupiter, but the Jupiter API doesn't really match all those cases.

[codecholeric]

It took me quite a while, but I think I have a working sample now. I would be really happy about any feedback; if I've understood the concepts, etc. It seems to do what I want in the situations I've tested, but it's really hard for me to tell, if I've covered all corner cases of all build tools, etc.

The current state of the implementation can be checked out at https://github.com/TNG/ArchUnit/tree/junit5-support/archunit-junit/junit5

I've described how to obtain the current SNAPSHOT artifacts with Gradle in https://github.com/TNG/ArchUnit/issues/34#issuecomment-405123575

I've tried to follow the example of the Jupiter Engine, i.e. I've created an api artifact for testCompile dependencies and an engine artifact for testRuntime dependencies. Furthermore I've created an engine-api artifact to cover the parts that could be useful for a client calling the engine.

Also I've tried to implement support for all the selectors recommended by @marcphilipp , except the ModuleSelector (I'll wait with that until someone really requests that feature :wink:)

You can check out working examples in
https://github.com/TNG/ArchUnit/tree/junit5-support/archunit-example/example-junit5

I have one follow up question: For the archunit-junit5 engine it would make sense to be able to select fields as well. Thus I've created that engine-api module, that covers a FieldSelector analogous to the MethodSelector. I guess I could write my own Gradle plugin to support that, but that seems somewhat overkill. Is there any way, to plug in such engine extensions into the official test support? How would I integrate such a thing in the best way?

[marcphilipp]

Is there any way, to plug in such engine extensions into the official test support?

You mean Gradle's Test task, right? I'm afraid currently there isn't. Maybe we could come up with a URI-like syntax to express selectors which tools like Gradle could support generically. 🤔

[codecholeric]

Yeah, I mean Gradle's test task. I don't know how exactly that would look like, but maybe sth. like

buildScript {
    dependencies {
        classpath 'com.tngtech.archunit:archunit-junit5-engine-api:0.9.0'
    }
}

test {
    useJUnitPlatform {
        additionalSelectors(props) {
            return props['archunit-select-field'].collect { FieldSelector.selectField(it)) }
        }
    }
}

And then you could write

./gradlew test \
    --archunit-select-field com.app.MyArchTest#ruleField \
    --archunit-select-field com.app.MyArchTest#anotherField

or similar? I don't know how hard that would be (and if it's feasible at all).
Or something more generic that would work for other build tools as well (what you probably meant).
I guess some way of specifying a factory method to create a selector and string inputs might work as well, like

selectorFactory=com.tngtech.archunit.junit5.FieldSelector.selectField

and then

selectors=com.app.MyArchTest#ruleField,com.app.MyArchTest#anotherField

or similar.

[marcphilipp]

The thing is: Gradle currently does all the discovery and just hands over the partioned set of classes. There are already two Gradle issues on this topic: https://github.com/gradle/gradle/issues/4773 and https://github.com/gradle/gradle/issues/4252.

[codecholeric]

Okay, I guess I'll just keep my eyes open for Gradle improvements there. Looks like there's some major work to do, that might be a little more important than my edge case here :wink:
Anyway, I guess I'll release the first shot of the ArchUnitTestEngine with the next version, since I have not heard back about any major problems with the SNAPSHOT.

[Frontrider]

Is there any progress on this issue, or there are still no materials on it? If there is none, than why do you have the platform in the first place, if we can't actually use it?
It's a bit rough opinion, but you should see where I'm coming from.

The javadoc provided no help so far on how one should go about implementing one.

[codecholeric]

If it helps you could take a look at

https://github.com/TNG/ArchUnit/blob/master/archunit-junit/junit5/engine/src/main/java/com/tngtech/archunit/junit/ArchUnitTestEngine.java

I think this is pretty much a minimal class based engine if you want things to work like

• running a single test class (class selector) / running all tests with Gradle (since Gradle adds all tests as class selectors instead of classpath root selector)
• a single method (method selector)
• all tests with the JUnit Console Launcher (classpath root selector)
• all tests within a package (e.g. IDE) (package selector)
• re-run a test (unique-id selector)

I also think if you don't want any surprises you need all of those (I don't know if anyone uses classpath root selector, besides JUnit Console Launcher, but still).

In parts the tricky thing is to see if the tools are already far enough, e.g. Gradle test runner ignoring tests that don't have a ClassSource attached, even though it's Optional.

[Frontrider]

Thank you. I looked at the Jupiter and the Vintage engines, but it was little help.
I'll take a look when I get there again.