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

Don't generate a broken test when a constructor has a value class parameter #94

Merged
merged 1 commit into from
Jan 27, 2025
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
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 }
}
}