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

Align versions of JUnit and use the same test api across the project #2924

Closed
IgnatBeresnev opened this issue Mar 16, 2023 · 2 comments · Fixed by #3138
Closed

Align versions of JUnit and use the same test api across the project #2924

IgnatBeresnev opened this issue Mar 16, 2023 · 2 comments · Fixed by #3138
Labels
tech-debt A technical issue that is not observable by the users, but improves maintainers quality of life

Comments

@IgnatBeresnev
Copy link
Member

IgnatBeresnev commented Mar 16, 2023

JUnit version

Some of Dokka's tests are run with JUnit 4, some are run with JUnit 5. Since these versions require different assert imports and different configuration (useJUnit() vs useJUnitPlatform()), it's difficult to extract the test configuration into a common convention plugin (see #2918), and might lead to difficult to uncover bugs.

Proposal: only one version of JUnit shoud be used across the whole project, either JUnit 4 or JUnit 5, with common configuration in a convention plugin.

Test API

At the moment, there's no consensus on the used test API within the project.

Different tests use different @Test annotation:

  • kotlin.test.Test
  • org.junit.jupiter.api.Test
  • org.junit.Test

Different tests use different assert imports:

  • kotlin.test.assertEquals
  • org.junit.jupiter.api.Assertions.assertEquals
  • org.junit.Assert.assertEquals

Moreover, it's not a given that the tests written for kotlin.test.Test use only kotlin.test.assertEquals - they can be intertwined, with kotlin.test tests using asserts from JUnit 5 and visa versa.

Some tests also utilize non test API for asserts, such as:

  • kotlin.check
  • kotlin.assert
  • kotlin.checkNotNull

All this can lead to inconsistent behaviour, bugs and difficult to read error messages.

Proposal: Dokka should use the same base asserts across the whole project. We can agree to stick to kotlin.test or to JUnit 5 only, without using non-test API. Additionally, we can agree to add more complex asserts such as Hamcrest or AssertJ, but if we do - we should stick with it across the whole project as well.

Configuration

With previous proposals addressed, the tests should be configured in one of the convention plugins (existing or an additional one), so that all projects use the same configuration.

@IgnatBeresnev IgnatBeresnev added the tech-debt A technical issue that is not observable by the users, but improves maintainers quality of life label Mar 16, 2023
@IgnatBeresnev IgnatBeresnev changed the title Align versions of JUnit and use the same asserts across the project Align versions of JUnit and use the same test api across the project Mar 16, 2023
@aSemy
Copy link
Contributor

aSemy commented Mar 16, 2023

Additionally, we can agree to add more complex asserts such as Hamcrest or AssertJ, but if we do - we should stick with it across the whole project as well.

AssertK is also present as well :)

package matchers.content
import assertk.assertThat
import assertk.assertions.contains
import assertk.assertions.isEqualTo
import assertk.assertions.isNotNull

Of course any unification is an improvement, so I don't really mind which assertion library is used too much, but here's my thoughts:

I'd personally prefer it if Hamcrest and AssertJ were removed or replaced. They are very Java-focused, and do not include nice Kotlin specific integrations (like 'assert is instance' doesn't trigger a smart-cast).

Just using kotlin.test is an option. The Kotlinx Serialization tests get away without an external assertions library.

And to throw another contender in the ring, I can really recommend the Kotest assertions (which aren't dependent on using Kotest for structuring the tests, it's possible to use kotlin.test + Kotest assertions). They are very Kotlin focused, fluent, easy to read, and have almost every assertion type available.

But, to re-iterate, it doesn't really matter to me: so long as it's all JUnit5 + one assertion library, it's good.

@IgnatBeresnev
Copy link
Member Author

IgnatBeresnev commented Aug 30, 2023

I investigated the possibility of migrating to Kotest or some other assertion library as part of the migration to JUnit 5, but the scope of changes and "new" technologies in the project would be too big for one PR.

Decided to stick with kotlin.test as much as possible for now, although some JUnit API is still used for Parameterized tests and temporary directories.

Down the road, if kotlin.test doesn't cover our needs, we'll be able to choose an assertion library and migrate to it gradually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech-debt A technical issue that is not observable by the users, but improves maintainers quality of life
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants