Add Kotlin specific assertion helpers #1001

JLLeitschuh · 2017-08-04T00:48:47Z

Overview

This adds several Kotlin friendly assertions to the standard JUnit assertion API.

Closes #924

What does this actually add?

The Assertions.kt file you see has top-level functions. When the compiler runs it actually creates a "hidden" class called AssertionsKt that contains these static methods. This name could be changed with an annotation to something like KotlinAssertions. This name will never actually show up in anyone's code but will end up in the compiled bytecode so changing the name of this file will be a breaking change (like changing the name of any class in Java).

Documentation

Unfortunately, I was unable to get (Dokka)[https://github.com/Kotlin/dokka] to behave correctly so that it would generate JavaDocs. The best that we can do on this front is document these methods in the user guide and show how they can be imported.

I hereby agree to the terms of the JUnit Contributor License Agreement.

Definition of Done

There are no TODOs left in the code
Method preconditions are checked and documented in the method's Javadoc
Coding conventions (e.g. for logging) have been followed
Change is covered by automated tests
Public API has Javadoc and @API annotations
Change is documented in the User Guide and Release Notes
All continuous integration builds pass

JLLeitschuh · 2017-08-04T01:17:00Z

build.gradle

@@ -434,6 +439,13 @@ subprojects { subproj ->
 replaceRegex 'Empty line between last method and class closure', /\n([\s]+)}\n}\n$/, '\n$1}\n\n}\n'
 replaceRegex 'Remove line breaks between consecutive closing parentheses', /\)\n[\s]+\)\n/, '))\n'
 }
+
+ kotlin {
+ ktlint("0.9.0")


ktlint is the only kotlin linter that exists at present. I use it for all of my projects and it's pretty nice.

If you want to have it use tabs instead of spaces for indentation it can be configured using an .editorconfig file in the root directory of the project.

JLLeitschuh · 2017-08-04T01:46:11Z

Somethings up with JKD9 that I'll try to solve tomorrow.

jbduncan · 2017-08-04T09:16:20Z

junit-jupiter-api/src/main/kotlin/org/junit/jupiter/api/Assertions.kt

+ * Example usage:
+ * ```kotlin
+ * val exception = assertThrows<IllegalArgumentException> {
+ * throw IllegalArgumentException("Talk to a 🦆")


@JLLeitschuh On my phone, this particular sentence renders as

throw IllegalArgumentException("Talk to the <box-shape>")

where "<box-shape>" renders as a box-shaped character.

Was this intended?

Its an emoji 🦆 (duck) character.
https://en.wikipedia.org/wiki/Rubber_duck_debugging

Okay, thanks @JLLeitschuh.

It renders correctly as a duck emoji on my copy of Chrome, but unfortunately it still renders as a box (the character an application displays when it cannot identify what a character is supposed to be) in Gmail and on my Android phone.

I wonder if it would be more maintainable to swap the duck emoji out for a non-emoji character or phrase.

Sorry, I meant to say that the duck emoji 🦆 renders correctly for me in your comment at #1001 (comment), but that it fails to render as a duck emoji in the GitHub code view on Chrome.

This is what it looks like for me in the GitHub code view:

Only in your iPhone browser right? It shows up fine for me on macOS Chrome.

Do you want me to change this to something that will render correctly on an IOS device?

I'd quite like it to be changed so it renders correctly on Windows and Android, if that's alright.

(I'm not sure if you saw my earlier comment, but I don't actually own an iOS device. My devices are Windows and Android. 😄)

Right, totally misread your message 😆

Sure, I can fix that.
Question...
Does the emoji in this code example render correctly for you?
http://junit.org/junit5/docs/current/user-guide/#writing-tests-display-names

Hi @JLLeitschuh, yes it does. Here's a screenshot of what it looks like for me on my Windows 7 machine.

Huh... This is what it looks like for me on macOS 10.12.6 with chrome.

Anyways, yes, I can change it.

JLLeitschuh · 2017-08-04T15:16:21Z

I'd like to add a kotlin example to the adocs docs if this is something that the maintainers think think this is the right route to go down.

JLLeitschuh · 2017-08-04T15:37:35Z

I'm asking in the kotlin slack channel about the problem with the compiler on JDK 9.

JLLeitschuh · 2017-08-09T15:43:55Z

Related issue:
https://youtrack.jetbrains.com/issue/KT-19557

AustinShalit · 2017-08-17T16:35:09Z

You should add a helper method for assertThrows that also takes a message.

sormuras · 2017-08-17T17:37:00Z

The JDK 9 issue "Should be fixed in Kotlin 1.1.4"

JLLeitschuh · 2017-08-17T19:07:45Z

This is actually blocked on:
gradle/kotlin-dsl-samples#454

I'm going to have to revert my changes to changing junit-jupiter-api.gradle.kts to a kts file and figure out how to do that artifact nonsense with groovy.

Closes junit-team#924

jbduncan · 2017-10-17T17:51:35Z

@JLLeitschuh Hmm. As far as I'm aware, there's nothing else stopping this PR from being merged. I can only assume that the JUnit 5 team are busy with IRL things and have prioritised other tasks like Java 9 compatibility over this PR.

So I'm sorry to say that I don't know for sure why this PR is in limbo. :/

marcphilipp · 2017-10-18T19:58:02Z

Is there anything besides the current merge conflict preventing this from being merged?
I feel like this PR is stuck in limbo. If you have any other problems with it please let me know so I can fix those up.

Sorry to keep you waiting! The current plan is to get 5.1 M1 out with JDK 9 module scanning/selector support and then reconsider open PRs and the 5.1 backlog. As I've stated in #924 (comment), we want to add support to make writing assertions in Kotlin easier. So, please bear with us a little longer. We'll eventually get to this. We do not expect you to keep this branch up-to-date every time we commit something on master.

JLLeitschuh · 2017-10-18T20:14:20Z

Thank you for this explanation. Really helpful. I would have loved to see this in 5.1.0-M1 but I can wait if necessary.

I hadn't realized that you had made the team decision explicit in that comment. Thank you for pointing it out.

Please let me know when you want me to update this PR to resolve merge conflicts in preparation for merging.

marcphilipp · 2017-11-09T13:22:29Z

Hi @JLLeitschuh, we've finally decided to merge this pull request. However, there are still a few problems that need to be fixed:

We had to hardcode kotlinOptions{ jvmTarget = '1.8' } which is fine.
Spotless fails when this PR is rebased with the current master because on master the indentatio of all Kotlin files was changed to use tabs already. So, please add an .editorconfig to make ktlint use tabs, too.

JLLeitschuh · 2017-11-09T14:50:01Z

You can't make ktlint use tabs, it's only configured to use spaces. The only configuration that there seems to be is to use either two spaces or four.
pinterest/ktlint#43 (comment)

I'm going to go with the ktlint default of using four spaces.

Why should I use ktlint?

Simplicity.

Spending time on configuration (& maintenance down the road) of hundred-line long style config file(s) is counter-productive. Instead of wasting your energy on something that has no business value - focus on what really matters (not debating whether to use tabs or spaces).
- Ktlint ReadMe

JLLeitschuh · 2017-11-09T14:58:10Z

junit-jupiter-api/src/main/kotlin/org/junit/jupiter/api/Assertions.kt

@@ -0,0 +1,93 @@
+/*


The compiler will generate a class called AssertionsKt to store these methods inside of. Is this acceptable? We can rename the generated class if necessary.

I think that's fine

JLLeitschuh · 2017-11-09T14:59:21Z

documentation/src/docs/asciidoc/writing-tests.adoc

+used in https://kotlinlang.org/[Kotlin]. All JUnit Jupiter Kotlin assertions are top-level
+functions in the `org.junit.jupiter.api` package.
+
+// TODO: Change to using kotlin language highlighting after switch to rouge syntax highlighter


What is the status of switching to the rouge syntax highlighter?

Still blocked by asciidoctor/asciidoctor#1040.

JLLeitschuh · 2017-11-09T15:27:33Z

This has been updated per your comments. Let me know if you have anything else you need me to resolve.

shyiko · 2017-11-09T20:07:21Z

@JLLeitschuh .editorconfig below makes ktlint ignore indentation completely (allowing tabs). This is not recommended as both Jetbrains & Google advocate 4-space-indentation but is perfectly valid as far ktlint is concerned.

[*.{kt,kts}]
indent_size=unset

JLLeitschuh · 2017-11-09T20:15:07Z

@marcphilipp This is your project, it's up to you to decide.

@shyiko It would be good if that were documented somewhere (site or readme). Thanks for jumping in here!

marcphilipp · 2017-11-10T19:32:18Z

documentation/src/docs/asciidoc/writing-tests.adoc

@@ -94,6 +94,16 @@ are `static` methods in the `{Assertions}` class.
 include::{testDir}/example/AssertionsDemo.java[tags=user_guide]
 ----

+Junit Jupiter also comes with a few assertion methods that lend themselves well to being


marcphilipp · 2017-11-11T16:02:14Z

junit-jupiter-api/src/main/kotlin/org/junit/jupiter/api/Assertions.kt

+ * [Stream] of functions to be executed.
+ */
+internal typealias ExecutableStream = Stream<() -> Unit>
+internal fun ExecutableStream.convert() = map { Executable(it) }


Can we make these private?

Effectively they are. internal is different from java's "package private".
"Package private" in java means that if I (a consumer) create a file in a package called org.junit.api then I can access "package private" things. The same is not true in Kotlin for internal.
Internal is internal only to source code that is compiled together.

I can make them private if you wish though.

So I was able to make them private.
I remember that in earlier versions of kotlin you couldn't call private things inside of inline functions. I'm not certain if that limitation still exists. But it doesn't seem to be a problem here.

JLLeitschuh · 2017-11-13T17:11:46Z

This travis failure doesn't look like it's created by me:

A problem occurred evaluating root project 'junit5'.
> Could not resolve all files for configuration 'classpath'.
   > Could not resolve de.schauderhaft.degraph:degraph-check:0.1.4.
     Required by:
         unspecified:unspecified:unspecified
      > Could not resolve de.schauderhaft.degraph:degraph-check:0.1.4.
         > Could not get resource 'https://repo1.maven.org/maven2/de/schauderhaft/degraph/degraph-check/0.1.4/degraph-check-0.1.4.pom'.
            > Could not GET 'https://repo1.maven.org/maven2/de/schauderhaft/degraph/degraph-check/0.1.4/degraph-check-0.1.4.pom'.
               > Host name 'repo1.maven.org' does not match the certificate subject provided by the peer (CN=repo.maven.apache.org, O="Sonatype, Inc", L=Fulton, ST=MD, C=US)

sormuras · 2017-11-13T17:30:52Z

Restarting didn't solve the issue -- seems related to https://issues.sonatype.org/browse/MVNCENTRAL-1369

sormuras · 2017-11-13T17:40:49Z

Same setup using Gradle 4.3.1 on master does work: https://travis-ci.org/junit-team/junit5/builds/300986101#L643

I don't see any changes in this PR that might effect the SSL configuration. https://repo1.maven.org responded as expected. Curious whether the next build will also fail with the same issue... let's see what happens: https://travis-ci.org/junit-team/junit5/builds/301525585

sormuras · 2017-11-13T17:48:01Z

Same on master now... sigh

mkobit · 2017-11-13T18:44:56Z

https://issues.sonatype.org/browse/MVNCENTRAL-2870 is the issue on the Maven Central side, gradle/gradle#3463 is for Gradle to use the proper CDN URL.

marcphilipp · 2017-11-13T19:07:51Z

Merged in 27c9c0d and 75c3d17. Thanks for your perseverance, @JLLeitschuh! 👍

JLLeitschuh · 2017-11-13T19:10:13Z

Wooot!!!! This makes me so happy!!!

Closes junit-team#1001.

JLLeitschuh changed the title ~~Feature/kotlin assertion helpers~~ Add kotlin specific assertion helpers Aug 4, 2017

JLLeitschuh force-pushed the feature/kotlin-assertion-helpers branch from 6ff9cd8 to d8d7136 Compare August 4, 2017 01:13

JLLeitschuh commented Aug 4, 2017

View reviewed changes

jbduncan reviewed Aug 4, 2017

View reviewed changes

JLLeitschuh mentioned this pull request Aug 9, 2017

Bump Gradle to version 4.1 #1012

Merged

7 tasks

sbrannen changed the title ~~Add kotlin specific assertion helpers~~ Add Kotlin specific assertion helpers Aug 19, 2017

ghost assigned sbrannen Aug 19, 2017

ghost added the status: in progress label Aug 19, 2017

sbrannen added component: Jupiter component: Kotlin theme: programming model type: enhancement labels Aug 19, 2017

sbrannen removed their assignment Aug 19, 2017

marcphilipp added this to the 5.1 Backlog milestone Aug 19, 2017

JLLeitschuh added 4 commits August 30, 2017 16:06

Add Kotlin Assertion methods

2d6ca09

Closes junit-team#924

Cleanup build and ensure kotlin sources are included in api JAR

451930e

Cleanup spotless problems on existing kotlin code

b5279aa

Remove duplicate kotlin plugin application

7c9102c

JLLeitschuh force-pushed the feature/kotlin-assertion-helpers branch from b7a3807 to 7c9102c Compare August 30, 2017 20:18

Revert changes to using Gradle Kotlin DSL

43c956e

JLLeitschuh force-pushed the feature/kotlin-assertion-helpers branch from 83c5bf6 to 43c956e Compare August 30, 2017 20:39

JLLeitschuh added 2 commits August 30, 2017 16:40

Remove duck emoji from kotlin assertion documentation

d995cde

Use kotlin-stdlib instead of kotlin-stdlib-jre8

bc0c5e3

Merge branch 'master' into feature/kotlin-assertion-helpers

b0bfc63

ghost assigned marcphilipp Nov 9, 2017

Reformat existing kotlin sources to use spaces instead of tabs

99b7569

JLLeitschuh commented Nov 9, 2017

View reviewed changes

Use jvmTarget of 1.8 for KotlinCompile

0fa5c98

marcphilipp reviewed Nov 10, 2017

View reviewed changes

Typo: Junit -> JUnit

e226327

marcphilipp reviewed Nov 11, 2017

View reviewed changes

JLLeitschuh added 2 commits November 13, 2017 11:56

Make ExecutableStream private

6fdbf08

Merge branch 'master' into feature/kotlin-assertion-helpers

a726c91

marcphilipp closed this in 75c3d17 Nov 13, 2017

ghost removed the status: in progress label Nov 13, 2017

JLLeitschuh mentioned this pull request Nov 16, 2017

Add assertAll? npryce/hamkrest#29

Closed

Andrei94 pushed a commit to Andrei94/junit5 that referenced this pull request Jun 23, 2018

Polish User Guide and Release Notes for junit-team#924

8617bae

Closes junit-team#1001.

Add Kotlin specific assertion helpers #1001

Add Kotlin specific assertion helpers #1001

Conversation

JLLeitschuh commented Aug 4, 2017 • edited Loading

Overview

What does this actually add?

Documentation

Definition of Done

JLLeitschuh Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

JLLeitschuh commented Aug 4, 2017

jbduncan Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbduncan Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

jbduncan Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JLLeitschuh Aug 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JLLeitschuh commented Aug 4, 2017

JLLeitschuh commented Aug 4, 2017

JLLeitschuh commented Aug 9, 2017

AustinShalit commented Aug 17, 2017

sormuras commented Aug 17, 2017 • edited Loading

JLLeitschuh commented Aug 17, 2017

jbduncan commented Oct 17, 2017

marcphilipp commented Oct 18, 2017

JLLeitschuh commented Oct 18, 2017

marcphilipp commented Nov 9, 2017

JLLeitschuh commented Nov 9, 2017 • edited Loading

Why should I use ktlint?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JLLeitschuh commented Nov 9, 2017

shyiko commented Nov 9, 2017 • edited Loading

JLLeitschuh commented Nov 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JLLeitschuh commented Nov 13, 2017

sormuras commented Nov 13, 2017

sormuras commented Nov 13, 2017 • edited Loading

sormuras commented Nov 13, 2017

mkobit commented Nov 13, 2017

marcphilipp commented Nov 13, 2017 • edited Loading

JLLeitschuh commented Nov 13, 2017

JLLeitschuh commented Aug 4, 2017 •

edited

Loading

JLLeitschuh Aug 4, 2017 •

edited

Loading

jbduncan Aug 4, 2017 •

edited

Loading

jbduncan Aug 4, 2017 •

edited

Loading

jbduncan Aug 4, 2017 •

edited

Loading

JLLeitschuh Aug 17, 2017 •

edited

Loading

sormuras commented Aug 17, 2017 •

edited

Loading

JLLeitschuh commented Nov 9, 2017 •

edited

Loading

shyiko commented Nov 9, 2017 •

edited

Loading

sormuras commented Nov 13, 2017 •

edited

Loading

marcphilipp commented Nov 13, 2017 •

edited

Loading