Introduce Kotlin Assertion Helper Library

[JLLeitschuh]

Feature Request

This is mostly a "nice to have" syntactically but the current a select few methods that all take [SomeWrapperType]<Executable> don't play as nicely as they could in kotlin.

Someone could easily create a kotlin DSL library wrapper over junit 5.
I did this with the Google Guice DI library creating Kotlin Guiced. I could just as easily do the same thing for Junit5 but if there is an interest it would be nice if the small DSL library was vendored by the Junit team.

The methods that I've currently found that don't play the nicest are the following:

assertAll(Stream<Executable> executables):

In kotlin calling this method becomes:

assertAll(someCollection.stream().map {
    Executable {
         assertEquals(6,6)
    }
})

This could be simplified by providing a method:

fun assertAll(executables: Stream<() -> Unit>) = assertAll(executables.map { Executable(it) })

This would allow for the much nicer syntax:

assertAll(someCollection.stream().map {
    { assertEquals(6,6) }
})

// Or even this:
assertAll(someCollection.stream().map {
    listOf(
        { assertEquals(6,6) }
        { assertEquals(3,3) }
    )
}.flatMap { it.stream() })

assertAll(Executable... executables):

In kotlin calling this method becomes:

assertAll(
    Executable { assertEquals(6, 6) },
    Executable { assertEquals(3, 3) },
    Executable { assertEquals(10_000, 10_000) }
)

This could be simplified by providing a method:

// This calls the above suggested method.
fun assertAll(vararg executables: () -> Unit) = assertAll(executables.toList().stream())

This would allow for the much nicer syntax:

assertAll(
   { assertEquals(6, 6) },
   { assertEquals(3, 3) },
   { assertEquals(10_000, 10_000) }
)

Similar methods would need to be provided for the methods that take String header as well.

assertThrows(Class<T> expectedType, Executable executable)

Calling in Kotlin it becomes:

assertThrows(IllegalArgumentException::class.java) { throw IllegalArgumentException() }

Yeay! We get auto conversion because this method takes Executable executable not a collection. However the IllegalArgumentException::class.java is kind of clunky.

This one could use reified types to make this nicer!

If we provide a method:

inline fun <reified T> assertThrows(executable : () -> Unit) = assertThrows(T::class.java, Executable(executable))

Then the syntax to call it in kotlin becomes:

assertThrows<IllegalArgumentException> { throw IllegalArgumentException() }

Note
These problems don't exist with all methods that take Executable.
For example, this is completely valid given the current Junit5 API:

assertTimeout(Duration.ZERO) { }

Deliverables

• Create a junit-jupiter-kotlin-api.
• Provide Kotlin friendly DSL in the junit-jupiter-kotlin-api library.

[marcphilipp]

Are there Kotlin-friendly assertions libraries you could use instead?

[JLLeitschuh]

There are a few. Many of them that provide full runtime syntax helpers piggyback off of Junit4.

I've been using hamkrest for some nicer assert.that([something]) syntax.

Are there any particular complexities of implementing your own assertAll method to be wary of?

Perhaps I just need to have something that throws the org.opentest4j.MultipleFailuresError and I'd get the same nice printing behaviour from the Junit5 engine.

[marcphilipp]

Yes, that's all you'd need. Do you want to give it a shot?

[sbrannen]

@sdeleuze, since you've introduced several Kotlin improvements for Spring Framework 5.0, can you provide us any guidance here on how to achieve a nice user experience with Kotlin with the least amount of work on the JUnit side?

[JLLeitschuh]

Maybe this is something that hamkrest would like to support? I'll open an issue there and ask. Otherwise I'll look into implementing these things in my own library.

If this is outside of the scope of the Junit mission statement I totally understand. I just wanted to run it by the devs here before charging off on my own path.

Is the org.opentest4j.MultipleFailuresError object stable enough to link against in ways that won't have API breaking changes?

[sbrannen]

org.opentest4j.MultipleFailuresError should in fact be relatively stable by this point, but keep in mind that it's also still in milestone phase.

[sdeleuze]

Since getting proper Kotlin support on assertion side is low hanging fruit + non intrusive, I would be super interested in getting a place inside https://github.com/junit-team organization to contribute a few Kotlin extensions, function or aliases for current JUnit 5 constructs.

Would it be doable to create a junit-jupiter-kotlin-api or junit-jupiter-kotlin-extensions module in https://github.com/junit-team/junit.contrib in order to allow interested people (for now at least @JLLeitschuh and me) to contribute.

IMO that would be a small addition to current API, but it would be super useful for Kotlin developers. On Spring side, I really would like to push JUnit 5 as a first class Kotlin JVM testing library, such extension would really help.

Any thoughts?

[sbrannen]

Hi @sdeleuze,

That's awesome that you're interested in helping out with the Kotlin support! 👏

As for the logistics, are PRs too difficult for you in this regard?

If so, I suppose the team will have to decide on how best to proceed.

Cheers,

Sam

[sbrannen]

@sdeleuze, just to be clear...

Are you only proposing to add Kotlin extensions for the Assertions API in JUnit Jupiter?

Or are you proposing to add additional support for parts of the JUnit Platform or other parts of JUnit Jupiter?

[sdeleuze]

I am not sure yet about the scope, but I will be happy to bootstrap such module with Assertions extensions to begin with. If we find other area where useful Kotlin extensions could be provided, let's add them too but I don't think there will be a huge amount needed.

[marcphilipp]

Is it feasible to include Kotlin-specific class files in the junit-jupiter-api JAR? Or does it make more sense to create an additional artifact?

[sdeleuze]

Yes it is feasible and given the few kbytes needed, I would recommend that way, like we did on the Spring side. If you are ok with that, should I submit a PR (when I will be back from holyday) ?

[JLLeitschuh]

If we do that won't it require including the kotlin stdlib/reflection lib as a dependency? Or am I wrong?

[sdeleuze]

Nope, the principle would be to have 0 impact for Java developers except the few kb of bytecode, dependencies on kotlin-stdlib and kotlin-reflect would be defined as optional or provided in order to be used during compilation of JUnit but not required on user side. We already do that on Spring Framework 5 without any impact for java developers.

[marcphilipp]

We will have to make sure that all the Kotlin things are in separate classes, though, right?

[JLLeitschuh]

You could probably just use Kotlin's first class functions. I think when it gets compiled they get wrapped in their own static class (with the file name as the class name) (this is for java interop) but the kotlin language totally hides this.

[sdeleuze]

Indeed

[JLLeitschuh]

What configuration is spring-framework added using to support it showing up in the maven pom as an optional dependency? I know how to do that in the dependency notation but not with the java-library plugin configurations.

[JLLeitschuh]

Cursory look at what I've got so far.

package org.junit.jupiter.api

import org.junit.jupiter.api.function.Executable
import org.junit.platform.commons.meta.API
import org.junit.platform.commons.meta.API.Usage.Experimental
import java.util.stream.Stream

internal typealias ExecutableStream = Stream<() -> Unit>
internal fun ExecutableStream.convert() = map { Executable(it) }

/**
 * @see Assertions.assertAll
 * @since 5.0
 */
@API(Experimental)
fun assertAll(executables: ExecutableStream) = Assertions.assertAll(executables.convert())

/**
 * @see Assertions.assertAll
 * @since 5.0
 */
@API(Experimental)
fun assertAll(heading : String?, executables: ExecutableStream) = Assertions.assertAll(heading, executables.convert())

/**
 * @see Assertions.assertAll
 * @since 5.0
 */
@API(Experimental)
fun assertAll(vararg executables: () -> Unit) = assertAll(executables.toList().stream())

/**
 * @see Assertions.assertAll
 * @since 5.0
 */
@API(Experimental)
fun assertAll(heading: String?, vararg executables: () -> Unit) = assertAll(heading, executables.toList().stream())

/**
 * @see Assertions.assertThrows
 * @since 5.0
 */
@API(Experimental)
inline fun <reified T : Throwable> assertThrows(noinline executable : () -> Unit) : T =
    Assertions.assertThrows(T::class.java, Executable(executable))

[sdeleuze]

@JLLeitschuh that's via https://github.com/spring-gradle-plugins/propdeps-plugin.

[JLLeitschuh]

Is the use of this plugin on master? I'm not seeing it anywhere. Nor am I seeing the string spring in any gradle file either.

[sbrannen]

@JLLeitschuh, the JUnit 5 build does not currently use any Gradle plugins from Spring; however, that doesn't mean we couldn't switch to that if we cannot find an alternative with our current build infrastructure.

[jbduncan]

@JLLeitschuh I assume that compileOnly is what one would use for "optional" or "provided" dependencies, even if one is already using java-library configurations like implementation. That's the approach I'm currently using for #961. However I've not Googled anything, so I'm not sure about this.

[jbduncan]

Looking at https://docs.gradle.org/current/userguide/java_library_plugin.html, it looks to me that compileOnly may be the right configuration to use.

[jbduncan]

@JLLeitschuh Well, having said that, I realise now and admit that I don't understand the reason why you're after a configuration for "optional" or "provided" dependencies. Can you explain further?

[JLLeitschuh]

Reasoning: #924 (comment)

[jbduncan]

@JLLeitschuh Okay, cool! Then yes, I would make a reasonable guess that compileOnly is probably what one needs to compile Kotlin sources the way described in #924 (comment), unless Spring Framework do it differently.

[marcphilipp]

Team decision:

• Determine if Kotlin extension code can be published in a separate artifact
• Declare Kotlin extensions as experimental API

[sbrannen]

FYI: we now use Spring's propdeps-plugin on master.

Thus, this should be used for optional Kotlin dependencies in the generated Maven POMs as well (if the Kotlin extensions are published in the junit-jupiter-api artifact).

[sbrannen]

@sdeleuze and @JLLeitschuh,

As you can see from the above "team decision," we'd like to know the pros and cons for having the Kotlin extensions published in the junit-jupiter-api artifact vs. publishing them in a separate artifact (e.g., junit-jupiter-api-kotlin).

Can you two please expound on that?

Keep in mind that we would ideally like to publish the Kotlin extensions as a separate artifact if possible. 😉

[sdeleuze]

Using compileOnly and testCompileOnly is also an option supported by default by Gradle, which will not expose Kotlin as an optional dependency but given the fact that application will have Kotlin stdlib in their classpath + this kind of issues that's maybe on option to consider.

About where publishing the extensions, Kotlin does not require publishing them in the same artifact, but I do think it would make sense to ship them in junit-jupiter-api because you are not publishing a Kotlin API, Kotlin and Java will share the same API with a few additional methods for Kotlin users. The few kilobytes added by 6 additional methods won't hurt any java developer, and will provide Kotlin support by default for Kotlin ones, without having to add an additional almost empty dependency with conceptually the equivalent of one util class. With PER_CLASS lifecycle already provide a good Kotlin experience and with non-intrusive Kotlin extension mechanism, that would be IMO a missed opportunity to provide such native Kotlin support. Also extensions are very tightly linked to the class they extend, making them evolve together make sense, it is not about separated APIs, it about about a single one IMO.

[sbrannen]

Also extensions are very tightly linked to the class they extend, making them evolve together make sense, it is not about separated APIs, it about about a single one IMO.

I tend to agree!

But one concern that the team has (which I forgot to mention -- sorry) is the topic of split packages.

Please excuse our ignorance, but...

Does a Kotlin extension have to reside in the same package as the Java class it is extending?

In other words, if we package the Kotlin extension for Jupiter's Assertions class, would the compiled version of Assertions.kt have to reside in the same package as Assertions.class?

[sbrannen]

@sdeleuze,

Since we are already using Spring's propdeps-plugin (i.e., for the @API Guardian), I figured we'd just use it for Kotlin, too.

So, just to clarify...

Are are you saying you would recommend against using the propdeps-plugin to declare Kotlin libraries as optional in the generated Maven POMs?

[JLLeitschuh]

It does seem like a very few methods to create an entirely new artifact for (6 or so methods).
If you think that this API should reside in a different package than the one I put it in org.junit.jupiter.api (perhaps org.junit.jupiter.api.kotlin?) then maybe I could see an argument for not having it in the same artifact.

Please excuse our ignorance, but...

Does a Kotlin extension have to reside in the same package as the Java class it is extending?

In other words, if we package the Kotlin extension for Jupiter's Assertions class, would the compiled version of Assertions.kt have to reside in the same package as Assertions.class?

I'm not actually adding extension methods to the Assertions class. I'm adding top level methods to the org.junit.jupiter.api package.

So when you use the methods in kotlin code you import the method, not the class.
Eg:

// The kotlin version of `assertThrows`
import org.junit.jupiter.api.assertThrows
// The java version of `assertThrows`
import org.junit.jupiter.api.Assertions.assertThrows

If I were using kotlin extension methods, for example if I wanted to expose a kotlin api on DynamicContainer::getFirstChild() It would not have to be in the org.junit.jupiter.api package.
However, an API consumer would have to import the extension method from whatever package it is defined in.

Here are some examples of extension functions not in the same package as the type they they are extending:

package com.company.util.collections

import com.google.common.collect.ImmutableList
import com.google.common.collect.ImmutableSet

fun <T> Collection<T>.toImmutableList() : ImmutableList<T> = ImmutableList.copyOf(this)

fun <T> Collection<T>.toImmutableSet() : ImmutableSet<T> = ImmutableSet.copyOf(this)

operator fun <T> ImmutableList<T>.plus(elements : ImmutableList<T>) =
    ImmutableList.builder<T>().addAll(this).addAll(elements).build()!!

[JLLeitschuh]

Are are you saying you would recommend against using the propdeps-plugin to declare Kotlin libraries as optional in the generated Maven POMs?

Doesn't using compileOnly as I have in the PR here not add it as a dependency in the generated POM?

[sdeleuze]

@sbrannen In fact split packages are another good reason for putting Kotlin JUnit extensions into the same JAR. Extension can live in the package you want, as described by @JLLeitschuh, but I think this is a good convention for users to make it live in the package of the class they extend. And since both src/main/java and src/main/kotlin are going into the same JAR in the same package, no split package issue.

@sbrannen @JLLeitschuh I have no strong opinion about using compileOnly (which does not add any entry in the POM) or using propdeps-plugin + optional, I just say both are possible. I just say both are possible.

[sbrannen]

Hi guys,

Thanks for chiming in!

We'll take your feedback into consideration.

[sbrannen]

Doesn't using compileOnly as I have in the PR here not add it as a dependency in the generated POM?

Yes.

But... we prefer for the generated Maven POMs to tell the whole story, namely that there is in fact a dependency which just happens to be optional.

The complete absence of a declaration on a dependency in the generated Maven POMs does not convey the fact that such a dependency exists (even if only optional).

[binkley]

Using the nice Kotlin version of assertThrows I get this compiler error:

Cannot inline bytecode built with JVM target 1.8 into bytecode that is being built with JVM target 1.6. Please specify proper '-jvm-target' option

Switching to the Java version of assertThrows does not error, but is also less idiomatic. I believe this is an unfortunate side effect of JetBrains' implementation of inline and reified, though I'm unable to find a good reference for this and the impact of bytecode version.

Is there a build of JUnit targeting JVM 1.8 bytecode?

[sormuras]

That's how we configure the Kotlin compiler:

junit5/build.gradle

Lines 190 to 196 in be331a4

	tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile) {
	kotlinOptions {
	jvmTarget = '1.8'
	apiVersion = '1.1'
	languageVersion = '1.1'
	}
	}