Skip to content

Commit

Permalink
Don't generate a broken test when a constructor has a value class par…
Browse files Browse the repository at this point in the history
…ameter (#94)

An implementation detail of the Kotlin compiler gets in our way.

Closes: #93

Co-authored-by: Jesse Wilson <[email protected]>
  • Loading branch information
swankjesse and squarejesse authored Jan 27, 2025
1 parent adf8e8c commit c9f361f
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 1 deletion.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
## [Unreleased]
[Unreleased]: https://github.com/cashapp/burst/compare/2.3.0...HEAD

**Fixed**

* Don't fail for constructors that have value class parameters. Unfortunately we can't run such tests from the IDE.


## [2.4.0] *(2025-01-24)*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,31 @@ class BurstGradlePluginTest {
}
}

@Test
fun valueClassConstructor() {
val projectDir = File("src/test/projects/valueClassConstructor")

val taskName = ":lib:test"
val result = createRunner(projectDir, "clean", taskName).build()
assertThat(result.task(taskName)!!.outcome).isIn(*SUCCESS_OUTCOMES)

val testResults = projectDir.resolve("lib/build/test-results")

// There's no default specialization.
assertThat(testResults.resolve("test/TEST-CoffeeTest.xml").exists()).isFalse()

val sampleTest = readTestSuite(testResults.resolve("test/TEST-CoffeeTest_Decaf.xml"))
val sampleTestTest = sampleTest.testCases.single()
assertThat(sampleTestTest.name).isEqualTo("test")
assertThat(sampleTestTest.skipped).isFalse()
assertThat(sampleTest.systemOut).isEqualTo(
"""
|running Decaf
|
""".trimMargin(),
)
}

private fun createRunner(
projectDir: File,
vararg taskNames: String,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import org.jetbrains.kotlin.gradle.dsl.JvmTarget
import org.jetbrains.kotlin.gradle.tasks.KotlinJvmCompile

buildscript {
repositories {
maven {
url = file("$rootDir/../../../../../build/testMaven").toURI()
}
mavenCentral()
google()
}
dependencies {
classpath("app.cash.burst:burst-gradle-plugin:${project.property("burstVersion")}")
classpath(libs.kotlin.gradlePlugin)
}
}

allprojects {
repositories {
maven {
url = file("$rootDir/../../../../../build/testMaven").toURI()
}
mavenCentral()
google()
}

tasks.withType(JavaCompile::class.java).configureEach {
sourceCompatibility = "1.8"
targetCompatibility = "1.8"
}

tasks.withType(KotlinJvmCompile::class.java).configureEach {
compilerOptions {
jvmTarget.set(JvmTarget.JVM_1_8)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
plugins {
kotlin("jvm")
id("app.cash.burst")
}

dependencies {
testImplementation(kotlin("test"))
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import app.cash.burst.Burst
import app.cash.burst.burstValues
import kotlin.test.Test

@Burst
class CoffeeTest(
private val espresso: Espresso = burstValues(Decaf, Regular, Double),
) {
@Test
fun test() {
println("running $espresso")
}
}

@JvmInline
value class Espresso(val ordinal: Int) {
override fun toString() = when (this) {
Decaf -> "Decaf"
Regular -> "Regular"
Double -> "Double"
else -> "$ordinal"
}
}

val Decaf = Espresso(0)
val Regular = Espresso(1)
val Double = Espresso(2)
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
dependencyResolutionManagement {
versionCatalogs {
create("libs") {
from(files("../../../../../gradle/libs.versions.toml"))
}
}
}

include(":lib")
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,11 @@ private val junit5TestClassId = junit5Package.classId("Test")
private val kotlinTestPackage = FqPackageName("kotlin.test")
private val kotlinTestClassId = kotlinTestPackage.classId("Test")

private val kotlinJvmFqPackage = FqPackageName("kotlin.jvm")
private val jvmInlineAnnotationId = kotlinJvmFqPackage.classId("JvmInline")

internal val IrAnnotationContainer.hasAtBurst: Boolean
get() = hasAnnotation(burstAnnotationId)

internal val IrAnnotationContainer.hasAtJvmInline: Boolean
get() = hasAnnotation(jvmInlineAnnotationId)
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import org.jetbrains.kotlin.ir.declarations.IrConstructor
import org.jetbrains.kotlin.ir.declarations.IrFile
import org.jetbrains.kotlin.ir.symbols.UnsafeDuringIrConstructionAPI
import org.jetbrains.kotlin.ir.types.IrTypeSystemContextImpl
import org.jetbrains.kotlin.ir.types.getClass
import org.jetbrains.kotlin.ir.util.addFakeOverrides
import org.jetbrains.kotlin.ir.util.constructors
import org.jetbrains.kotlin.ir.util.createImplicitParameterDeclarationWithWrappedDescriptor
Expand Down Expand Up @@ -89,7 +90,10 @@ internal class ClassSpecializer(
if (valueParameters.isEmpty()) return // Nothing to do.

val specializations = specializations(pluginContext, burstApis, valueParameters)
val indexOfDefaultSpecialization = specializations.indexOfFirst { it.isDefault }
val indexOfDefaultSpecialization = when {
onlyConstructor.defaultSpecializationIsBroken() -> -1
else -> specializations.indexOfFirst { it.isDefault }
}

// Make sure the constructor we're using is accessible. Drop the default arguments to prevent
// JUnit from using it.
Expand Down Expand Up @@ -180,4 +184,18 @@ internal class ClassSpecializer(
}
pluginContext.metadataDeclarationRegistrar.registerConstructorAsMetadataVisible(constructor)
}

/**
* Returns true if the default specialization would be broken for this constructor.
*
* We don't support the default specialization if any constructor parameter is a JVM value class.
* For each constructor that declares a value class parameter, the Kotlin compiler generates an
* extra (not-user-visible) constructor. But JUnit requires each test class to have exactly one
* public constructor.
*
* https://github.com/cashapp/burst/issues/93
*/
private fun IrConstructor.defaultSpecializationIsBroken(): Boolean {
return valueParameters.any { it.type.getClass()?.hasAtJvmInline == true }
}
}

0 comments on commit c9f361f

Please sign in to comment.