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

Migrate to JUnit 5 and unify used test API #3138

Merged
merged 4 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,12 @@ tasks.withType<Test>().configureEach {
} else {
(Runtime.getRuntime().availableProcessors() / 2).takeIf { it > 0 } ?: 1
}

javaLauncher.set(javaToolchains.launcherFor {
languageVersion.set(dokkaBuild.testJavaLauncherVersion)
})
}

dependencies {
testImplementation(platform("org.junit:junit-bom:5.9.2"))
// TODO Upgrade all subprojects to use JUnit Jupiter https://github.com/Kotlin/dokka/issues/2924
// Replace these dependencies with `testImplementation("org.junit.jupiter:junit-jupiter:5.9.2")`
// See https://junit.org/junit5/docs/current/user-guide/#running-tests-build-gradle-engines-configure
// Ideally convention plugins should only provide sensible defaults that can be overridden by subprojects.
// If a convention plugin defines dependencies, these cannot be easily overridden by subprojects, and so
// this should be avoided. However, for now , both JUnit 4 and 5 must be supported, and since these are test
// runtime-only dependencies they are not going to have a significant impact subprojects.
// These dependencies should be revisited in #2924, and (for example) moved to each subproject (which is more
// repetitive, but more declarative and clear), or some other solution.
testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine")
testRuntimeOnly("org.junit.vintage:junit-vintage-engine")
// kotlin-test asserts for all projects
testImplementation(kotlin("test-junit"))
testImplementation(platform(libs.junit.bom))
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,17 @@ val integrationTest by tasks.registering(NonCacheableIntegrationTest::class) {
maxHeapSize = "2G"
description = "Runs integration tests."
group = "verification"
useJUnit()

testClassesDirs = integrationTestSourceSet.output.classesDirs
classpath = integrationTestSourceSet.runtimeClasspath

useJUnitPlatform()

setForkEvery(1)
project.properties["dokka_integration_test_parallelism"]?.toString()?.toIntOrNull()?.let { parallelism ->
maxParallelForks = parallelism
}


environment(
"isExhaustive",
project.properties["dokka_integration_test_is_exhaustive"]?.toString()?.toBoolean()
Expand Down
2 changes: 1 addition & 1 deletion core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ dependencies {
}
}

testImplementation(kotlin("test"))
testImplementation(projects.core.testApi)
testImplementation(kotlin("test-junit"))
}

tasks {
Expand Down
2 changes: 1 addition & 1 deletion core/content-matcher-test-utils/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ dependencies {
implementation(projects.core.testApi)

implementation(kotlin("reflect"))
implementation(libs.assertk)
implementation(kotlin("test"))
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
package matchers.content

import assertk.assertThat
import assertk.assertions.contains
import assertk.assertions.isEqualTo
import org.jetbrains.dokka.model.withDescendants
import org.jetbrains.dokka.pages.*
import org.jetbrains.dokka.test.tools.matchers.content.*
import kotlin.reflect.KClass
import kotlin.test.assertEquals
import kotlin.test.asserter

// entry point:
fun ContentNode.assertNode(block: ContentMatcherBuilder<ContentComposite>.() -> Unit) {
Expand Down Expand Up @@ -48,7 +47,7 @@ private val ContentComposite.extractedText

fun <T : ContentComposite> ContentMatcherBuilder<T>.hasExactText(expected: String) {
assertions += {
assertThat(this::extractedText).isEqualTo(expected)
assertEquals(expected, this.extractedText)
}
}

Expand All @@ -74,29 +73,28 @@ fun ContentMatcherBuilder<*>.tabbedGroup(
block: ContentMatcherBuilder<ContentGroup>.() -> Unit
) = composite<ContentGroup> {
block()
check { assertThat(this::style).transform { style -> style.contains(ContentStyle.TabbedContent) }.isEqualTo(true) }
check { assertContains(this.style, ContentStyle.TabbedContent) }
}

fun ContentMatcherBuilder<*>.tab(
tabbedContentType: TabbedContentType, block: ContentMatcherBuilder<ContentGroup>.() -> Unit
) = composite<ContentGroup> {
block()
check {
assertThat(this::extra).transform { extra -> extra[TabbedContentTypeExtra]?.value }
.isEqualTo(tabbedContentType)
assertEquals(tabbedContentType, this.extra[TabbedContentTypeExtra]?.value)
}
}

fun ContentMatcherBuilder<*>.header(expectedLevel: Int? = null, block: ContentMatcherBuilder<ContentHeader>.() -> Unit) =
composite<ContentHeader> {
block()
check { if (expectedLevel != null) assertThat(this::level).isEqualTo(expectedLevel) }
check { if (expectedLevel != null) assertEquals(expectedLevel, this.level) }
}

fun ContentMatcherBuilder<*>.p(block: ContentMatcherBuilder<ContentGroup>.() -> Unit) =
composite<ContentGroup> {
block()
check { assertThat(this::style).contains(TextStyle.Paragraph) }
check { assertContains(this.style, TextStyle.Paragraph) }
}

fun ContentMatcherBuilder<*>.link(block: ContentMatcherBuilder<ContentLink>.() -> Unit) = composite(block)
Expand All @@ -114,7 +112,7 @@ fun ContentMatcherBuilder<*>.codeInline(block: ContentMatcherBuilder<ContentCode

fun ContentMatcherBuilder<*>.caption(block: ContentMatcherBuilder<ContentGroup>.() -> Unit) = composite<ContentGroup> {
block()
check { assertThat(this::style).contains(ContentStyle.Caption) }
check { assertContains(this.style, ContentStyle.Caption) }
}

fun ContentMatcherBuilder<*>.br() = node<ContentBreakLine>()
Expand All @@ -139,3 +137,13 @@ fun ContentMatcherBuilder<ContentDivergentInstance>.divergent(block: ContentMatc

fun ContentMatcherBuilder<ContentDivergentInstance>.after(block: ContentMatcherBuilder<ContentComposite>.() -> Unit) =
composite(block)

/*
* TODO replace with kotlin.test.assertContains after migrating to Kotlin language version 1.5+
*/
private fun <T> assertContains(iterable: Iterable<T>, element: T) {
asserter.assertTrue(
{ "Expected the collection to contain the element.\nCollection <$iterable>, element <$element>." },
iterable.contains(element)
)
}
1 change: 0 additions & 1 deletion core/test-api/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ plugins {
dependencies {
api(projects.core)

implementation("junit:junit:4.13.2") // TODO: remove dependency to junit
implementation(kotlin("reflect"))
}

Expand Down
130 changes: 71 additions & 59 deletions core/test-api/src/main/kotlin/testApi/testRunner/TestRunner.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import org.jetbrains.dokka.plugability.DokkaContext
import org.jetbrains.dokka.plugability.DokkaPlugin
import org.jetbrains.dokka.testApi.logger.TestLogger
import org.jetbrains.dokka.utilities.DokkaLogger
import org.junit.rules.TemporaryFolder
import testApi.testRunner.TestDokkaConfigurationBuilder
import java.io.File
import java.net.URL
Expand All @@ -30,6 +29,9 @@ abstract class AbstractTest<M : TestMethods, T : TestBuilder<M>, D : DokkaTestGe
?: throw InvalidPathException(name, "Cannot be found")

/**
* @param cleanupOutput if set to true, any temporary files will be cleaned up after execution. If set to false,
* it will be left to the user or the OS to delete it. Has no effect if [useOutputLocationFromConfig]
* is also set to true.
* @param useOutputLocationFromConfig if set to true, output location specified in [DokkaConfigurationImpl.outputDir]
* will be used. If set to false, a temporary folder will be used instead.
*/
Expand All @@ -40,24 +42,26 @@ abstract class AbstractTest<M : TestMethods, T : TestBuilder<M>, D : DokkaTestGe
pluginOverrides: List<DokkaPlugin> = emptyList(),
block: T.() -> Unit,
) {
val testMethods = testBuilder().apply(block).build()
val configurationToUse =
if (useOutputLocationFromConfig) {
configuration
} else {
val tempDir = getTempDir(cleanupOutput)
if (useOutputLocationFromConfig) {
runTests(
configuration = configuration,
pluginOverrides = pluginOverrides,
testLogger = logger,
block = block
)
} else {
withTempDirectory(cleanUpAfterUse = cleanupOutput) { tempDir ->
if (!cleanupOutput) {
logger.info("Output generated under: ${tempDir.root.absolutePath}")
logger.info("Output will be generated under: ${tempDir.absolutePath}")
}
configuration.copy(outputDir = tempDir.root)
runTests(
configuration = configuration.copy(outputDir = tempDir),
pluginOverrides = pluginOverrides,
testLogger = logger,
block = block
)
}

dokkaTestGenerator(
configurationToUse,
logger,
testMethods,
pluginOverrides
).generate()
}
}

protected fun testInline(
Expand All @@ -68,42 +72,62 @@ abstract class AbstractTest<M : TestMethods, T : TestBuilder<M>, D : DokkaTestGe
loggerForTest: DokkaLogger = logger,
block: T.() -> Unit,
) {
val testMethods = testBuilder().apply(block).build()
val testDirPath = getTempDir(cleanupOutput).root.toPath().toAbsolutePath()
val fileMap = query.toFileMap()
fileMap.materializeFiles(testDirPath.toAbsolutePath())
if (!cleanupOutput)
loggerForTest.info("Output generated under: ${testDirPath.toAbsolutePath()}")
val newConfiguration = configuration.copy(
outputDir = testDirPath.toFile(),
sourceSets = configuration.sourceSets.map { sourceSet ->
sourceSet.copy(
sourceRoots = sourceSet.sourceRoots.map { file ->
testDirPath.toFile().resolve(file)
}.toSet(),
suppressedFiles = sourceSet.suppressedFiles.map { file ->
testDirPath.toFile().resolve(file)
}.toSet(),
sourceLinks = sourceSet.sourceLinks.map { link ->
link.copy(
localDirectory = testDirPath.toFile().resolve(link.localDirectory).absolutePath
)
}.toSet(),
includes = sourceSet.includes.map { file ->
testDirPath.toFile().resolve(file)
}.toSet()
)
withTempDirectory(cleanUpAfterUse = cleanupOutput) { tempDir ->
if (!cleanupOutput) {
loggerForTest.info("Output will be generated under: ${tempDir.absolutePath}")
}
)

val fileMap = query.toFileMap()
fileMap.materializeFiles(tempDir.toPath().toAbsolutePath())

val newConfiguration = configuration.copy(
outputDir = tempDir,
sourceSets = configuration.sourceSets.map { sourceSet ->
sourceSet.copy(
sourceRoots = sourceSet.sourceRoots.map { file -> tempDir.resolve(file) }.toSet(),
suppressedFiles = sourceSet.suppressedFiles.map { file -> tempDir.resolve(file) }.toSet(),
sourceLinks = sourceSet.sourceLinks.map {
link -> link.copy(localDirectory = tempDir.resolve(link.localDirectory).absolutePath)
}.toSet(),
includes = sourceSet.includes.map { file -> tempDir.resolve(file) }.toSet()
)
}
)
runTests(
configuration = newConfiguration,
pluginOverrides = pluginOverrides,
testLogger = loggerForTest,
block = block
)
}
}

private fun withTempDirectory(cleanUpAfterUse: Boolean, block: (tempDirectory: File) -> Unit) {
val tempDir = this.createTempDir()
try {
block(tempDir)
} finally {
if (cleanUpAfterUse) {
tempDir.delete()
}
}
}
Comment on lines +105 to +114
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't want to add the JUnit dependency only for the temp directories feature, so I implemented it myself.


private fun runTests(
configuration: DokkaConfiguration,
pluginOverrides: List<DokkaPlugin>,
testLogger: DokkaLogger = logger,
block: T.() -> Unit
) {
val testMethods = testBuilder().apply(block).build()
dokkaTestGenerator(
newConfiguration,
loggerForTest,
configuration,
testLogger,
testMethods,
pluginOverrides
).generate()
}


private fun String.toFileMap(): Map<String, String> {
return this.trimIndent().trimMargin()
.replace("\r\n", "\n")
Expand Down Expand Up @@ -141,20 +165,8 @@ abstract class AbstractTest<M : TestMethods, T : TestBuilder<M>, D : DokkaTestGe
Files.write(file, content.toByteArray(charset))
}

private fun getTempDir(cleanupOutput: Boolean) =
if (cleanupOutput) {
TemporaryFolder().apply { create() }
} else {
TemporaryFolderWithoutCleanup().apply { create() }
}

/**
* Creates a temporary folder, but doesn't delete files
* right after it's been used, delegating it to the OS
*/
private class TemporaryFolderWithoutCleanup : TemporaryFolder() {
override fun after() { }
}
@Suppress("DEPRECATION") // TODO migrate to kotlin.io.path.createTempDirectory with languageVersion >= 1.5
private fun createTempDir(): File = kotlin.io.createTempDir()

protected fun dokkaConfiguration(block: TestDokkaConfigurationBuilder.() -> Unit): DokkaConfigurationImpl =
testApi.testRunner.dokkaConfiguration(block)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ Below you will find a complete unit test that passes, and the main takeaways bel
package org.example.dokka.plugin

import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest
import org.junit.Test
import kotlin.test.Test
import kotlin.test.assertEquals

class HideInternalApiPluginTest : BaseAbstractTest() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package org.example.dokka.plugin

import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest
import org.junit.Test
import kotlin.test.Test
import kotlin.test.assertEquals

class HideInternalApiPluginTest : BaseAbstractTest() {
Expand Down
5 changes: 2 additions & 3 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ gradlePlugin-gradleNode = "3.5.1"

## Test
junit = "5.9.2"
assertk = "0.25"
eclipse-jgit = "5.12.0.202106070339-r"

[libraries]
Expand Down Expand Up @@ -115,11 +114,11 @@ apacheMaven-artifact = { module = "org.apache.maven:maven-artifact", version.ref
kotlinx-cli = { module = "org.jetbrains.kotlinx:kotlinx-cli-jvm", version.ref = "kotlinx-cli" }

#### Test dependencies ####
assertk = { module = "com.willowtreeapps.assertk:assertk", version.ref = "assertk" }
eclipse-jgit = { module = "org.eclipse.jgit:org.eclipse.jgit", version.ref = "eclipse-jgit" }

junit-bom = { module = "org.junit:junit-bom", version.ref = "junit" }
junit-jupiter = { module = "org.junit.jupiter:junit-jupiter" }
junit-jupiterApi = { module = "org.junit.jupiter:junit-jupiter-api", version.ref = "junit" }
junit-jupiterParams = { module = "org.junit.jupiter:junit-jupiter-params", version.ref = "junit" }

[plugins]
# Gradle Plugins that are applied directly to subprojects
Expand Down
6 changes: 5 additions & 1 deletion integration-tests/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ plugins {
}

dependencies {
implementation(kotlin("test-junit"))
// Classes from src rely on JUnit's @TempDir and Kotlin's @AfterTest,
// thus these dependencies are needed. Ideally, they should be removed.
implementation(kotlin("test-junit5"))
implementation(libs.junit.jupiterApi)

implementation(libs.kotlinx.coroutines.core)
implementation(libs.jsoup)
implementation(libs.eclipse.jgit)
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/cli/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ evaluationDependsOn(":runners:cli")
evaluationDependsOn(":plugins:base")

dependencies {
implementation(kotlin("test-junit"))
implementation(kotlin("test-junit5"))
implementation(projects.integrationTests)
}

Expand Down
Loading
Loading