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

Add support for snapshots for android multiplatform #1649

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

geoff-powell
Copy link
Collaborator

@geoff-powell geoff-powell commented Oct 21, 2024

Adds source set support for Android Multiplatform projects.

Closes #595.

@geoff-powell geoff-powell force-pushed the gpowell/multiplatform-snapshots branch 2 times, most recently from e6defe1 to f71a4cb Compare October 21, 2024 14:36
@@ -89,7 +90,30 @@ public class PaparazziPlugin @Inject constructor(
project.addTestDependency()
val layoutlibNativeRuntimeFileCollection = project.setupLayoutlibRuntimeDependency()
val layoutlibResourcesFileCollection = project.setupLayoutlibResourcesDependency()
val snapshotOutputDir = project.layout.projectDirectory.dir("src/test/snapshots")
val testSourceSetProvider = project.objects.directoryProperty()
testSourceSetProvider.set(project.layout.projectDirectory.dir("src/test"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

there should be a way to query Gradle for the test source set, vs hardcoding them

Comment on lines 8 to 16
androidTarget {
publishLibraryVariants('release')
}

sourceSets {
commonTest.dependencies {

}
commonMain.dependencies {

}
androidMain {
dependencies {
implementation 'androidx.appcompat:appcompat:1.7.0'
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
androidTarget {
publishLibraryVariants('release')
}
sourceSets {
commonTest.dependencies {
}
commonMain.dependencies {
}
androidMain {
dependencies {
implementation 'androidx.appcompat:appcompat:1.7.0'
}
}
}
androidTarget()

none of this KMP boilerplate affects the test purpose

Comment on lines 45 to 47
dependencies {
testImplementation libs.junit
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
dependencies {
testImplementation libs.junit
}


@Test
fun test() {
paparazzi.snapshot(HelloPaparazzi(paparazzi.context))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
paparazzi.snapshot(HelloPaparazzi(paparazzi.context))
paparazzi.snapshot(TextView(paparazzi.context).apply { text = "Hello Paparazzi!" })

Copy link
Collaborator

Choose a reason for hiding this comment

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

delete

Copy link
Collaborator

Choose a reason for hiding this comment

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

delete

kotlinOptions {
jvmTarget = libs.versions.javaTarget.get()
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

interesting that KMP requires this, but the other projects don't!

project.plugins.withId("org.jetbrains.kotlin.multiplatform") {
val kmpExtension = project.extensions.getByType(KotlinMultiplatformExtension::class.java)
kmpExtension.sourceSets.all { sourceSet ->
if (sourceSet.name == "androidUnitTest" || sourceSet.name == "androidTest") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

similar, is there a way for the plugin to tell us test source sets?

Copy link
Collaborator

Choose a reason for hiding this comment

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

while i agree that hardcoded values aren't great, this is still an improvement over our current setup which breaks the directory structure for multiplatform projects.

@geoff-powell geoff-powell force-pushed the gpowell/multiplatform-snapshots branch from f71a4cb to b4d6694 Compare October 23, 2024 16:28
@tonyradz
Copy link

hey, any updates on this? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Snapshots should be placed into src/{sourceSetName}/snapshots/ for MPP plugin
4 participants