Skip to content
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
4 changes: 2 additions & 2 deletions Plugins/Gradle/gradle/libs.versions.toml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Plugins/Gradle/settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pluginManagement {
}
}
plugins {
id 'org.jetbrains.kotlin.jvm' version '1.9.24'
id 'org.jetbrains.kotlin.jvm' version '2.2.0'
id 'org.jetbrains.dokka' version '1.8.10'
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,9 @@ internal data class TestifySettings(
val android = project.android
val extension = project.getTestifyExtension()

val assetsSet = android.sourceSets.getByName("""androidTest""").assets
val baselineSourceDir = extension.baselineSourceDir ?: assetsSet.srcDirs.first().path
val baselineSourceDir = extension.baselineSourceDir
?: project.android.sourceSets.findByName("androidTest")?.assets?.directories?.firstOrNull()
?: "src/androidTest/assets"
val testRunner = extension.testRunner ?: android.defaultConfig.testInstrumentationRunner ?: "unknown"
val pullWaitTime = extension.pullWaitTime ?: 0L
val testPackageId = extension.testPackageId ?: project.inferredDefaultTestVariantId
Expand Down
8 changes: 4 additions & 4 deletions Plugins/Gradle/src/main/kotlin/dev/testify/TestifyPlugin.kt
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ class TestifyPlugin : Plugin<Project> {
val isRecordMode = settings.isRecordMode.toString()
val parallelThreads = settings.parallelThreads.toString()
android.defaultConfig {
it.resValue("string", "testifyDestination", destination)
it.resValue("string", "testifyModule", module)
it.resValue("string", "isRecordMode", isRecordMode)
it.resValue("string", "parallelThreads", parallelThreads)
resValue("string", "testifyDestination", destination)
resValue("string", "testifyModule", module)
resValue("string", "isRecordMode", isRecordMode)
resValue("string", "parallelThreads", parallelThreads)
}
}

Expand Down
19 changes: 16 additions & 3 deletions Plugins/Gradle/src/main/kotlin/dev/testify/internal/Adb.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

package dev.testify.internal

import com.android.build.api.variant.ApplicationAndroidComponentsExtension
import com.android.build.api.variant.LibraryAndroidComponentsExtension
import dev.testify.internal.StreamData.BufferedStream
import dev.testify.internal.Style.Description
import org.gradle.api.GradleException
Expand Down Expand Up @@ -121,18 +123,29 @@ class Adb {
}

companion object {
private lateinit var adbPath: String
private var adbPathProvider: (() -> String)? = null
private var verbose: Boolean = false
var forcedUser: Int? = null
private var deviceTargetIndex: Int = 0

fun init(project: Project) {
adbPath = project.android.adbExecutable.absolutePath
?: throw GradleException("adb not found. Have you defined an `android` block?")
adbPathProvider = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I was verifying the changes and found that Plugin:test is failing when run against an emulator. I noticed that the CI verification fails to launch an emulator and so is an incomplete test. This allows for CI to pass, but hides a problem.

The ConfigurationCacheTest fails for all but the first test with the error:

   Execution failed for task ':LegacySample:hidePasswords'.                                                                                                                                         
   > Failed to create service 'com.android.build.gradle.internal.SdkComponentsBuildService_3e57380e-18fd-4cba-a41e-a52928287d47'.                                                                   
      > Could not create an instance of type com.android.build.gradle.internal.SdkComponentsBuildService.                                                                                           
         > Build-scoped services has been closed.    

It seems like the adbPathProvider is capturing a stale project and breaking the configuration cache.

I'll try to fix it

Copy link
Contributor

Choose a reason for hiding this comment

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

I have a fix here (#274) but I need your changes on main before I can open my PR. So, I'm going to approve and merge your changes.

val androidComponents = project.extensions.findByType(
ApplicationAndroidComponentsExtension::class.java
) ?: project.extensions.findByType(
LibraryAndroidComponentsExtension::class.java
)
androidComponents?.sdkComponents?.adb?.get()?.asFile?.absolutePath
?: throw GradleException("adb not found via androidComponents.sdkComponents")
}
deviceTargetIndex = (project.properties["device"] as? String)?.toInt() ?: 0
verbose = project.isVerbose
forcedUser = project.user
}

private val adbPath: String
get() = adbPathProvider?.invoke()
?: throw GradleException("Adb.init() must be called before using Adb")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@

package dev.testify.internal

import com.android.build.gradle.AppExtension
import com.android.build.gradle.TestedExtension
import com.android.build.api.dsl.ApplicationExtension
import com.android.build.api.dsl.CommonExtension
import com.android.build.api.dsl.LibraryExtension
import org.gradle.api.GradleException
import org.gradle.api.Project

val Project.android: TestedExtension
get() = this.properties["android"] as? TestedExtension
val Project.android: CommonExtension<*, *, *, *, *, *>
get() = this.extensions.findByType(ApplicationExtension::class.java)
?: this.extensions.findByType(LibraryExtension::class.java)
?: throw GradleException("Gradle project must contain an `android` closure")

val Project.isVerbose: Boolean
Expand Down Expand Up @@ -59,30 +61,32 @@ val Project.inferredAndroidTestInstallTask: String?

val Project.inferredDefaultTestVariantId: String
get() {
val testVariant = this.android.testVariants.sortedBy { it.testedVariant.flavorName }.firstOrNull()
return try {
testVariant?.applicationId
} catch (e: Throwable) {
this.applicationTargetPackageId?.let { "$it.test" } ?: ""
} ?: ""
return this.applicationTargetPackageId?.let { "$it.test" } ?: ""
}

val Project.applicationTargetPackageId: String?
get() {
var targetPackageId: String? = null
val appExtension = this.extensions.findByType(ApplicationExtension::class.java) ?: return null
return try {
val baseApplicationId = appExtension.defaultConfig.applicationId ?: return null

// Prefer the debug variant
if (this.android is AppExtension) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this might be the opportunity I've been looking for to support all of the variants properly. Instead of trying to infer/guess a single task to extend, it's probably overdue that we evolve to the more modern approach. Handling onVariant and building separate tasks (as appropriate) for each variant is pretty standard in most gradle plugins and I've always felt it kind of weird for the way Testify used to insist on supporting only a single variant.

I haven't really looked at your chained PR, but I'm thinking we just go with a breaking change and modify the plugin to support multiple variants

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm already pretty near the edge of my level of understanding for Gradle development, so I'm not sure I've wrapped my head around how to approach coding that.

If we do want to use onVariants to support multiple variants, I think we'd want to do it starting with my stacked PR dfabulich#1

What are you imagining we'd do with this PR? Options include:

  1. Merge it as-is
  2. Merge my stacked PR refactor to fetch variants/packageIds from an async store dfabulich/android-testify#1 to my PR branch and then merge the whole thing, which does include onVariants support, but only to maintain the status quo
  3. Don't merge anything until Testify can support multiple variants. (Would you work on that? I'm not sure I'm going to do it…)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll definitely help with accomplishing this. I think the best strategy is to merge this pr, then you can open the stacked PR against main.
Then, I can pull that down and help work on the multi-variant support

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't mind have main in an intermediate state where it's not ready for publishing, but is something we can work on collaboratively. Once it's in a good state, I'll bump the versions and release as a 5.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, it sounds like the next step is for you to do the merge. When that happens, I'll retarget my PR. (LMK if you need anything from me.)

val appExtension = this.android as AppExtension
val allDebugVariants = appExtension.applicationVariants.filter {
it.name == "debug" || it.name.endsWith("Debug")
}.sortedBy { it.name }
targetPackageId = allDebugVariants.firstOrNull()?.applicationId
}
// Prefer debug build type suffix (most common for testing), fall back to any build type
val debugBuildType = appExtension.buildTypes.findByName("debug")
val buildType = debugBuildType ?: appExtension.buildTypes.firstOrNull()

// For apks without a debug variant, use the default applicationId
if (targetPackageId.isNullOrEmpty()) {
targetPackageId = this.android.defaultConfig.applicationId
val suffix = buildType?.applicationIdSuffix
if (suffix != null && suffix.isNotEmpty()) {
// Remove leading dot if present, then append with dot
val cleanSuffix = suffix.removePrefix(".")
"$baseApplicationId.$cleanSuffix"
} else {
baseApplicationId
}
} catch (e: Throwable) {
try {
appExtension.defaultConfig.applicationId
} catch (e2: Throwable) {
null
}
}
return targetPackageId
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,23 @@ open class ScreenshotPullTask : TestifyDefaultTask() {
private fun String.toLocalPath(): String {
val src = screenshotDirectory
val dst = destinationImageDirectory
val dstFile = if (File(dst).isAbsolute) {
File(dst)
} else {
File(project.projectDir, dst)
Copy link
Contributor

Choose a reason for hiding this comment

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

I tracked down the configuration break to this line. In order to support gradle configuration caching, the project can only be accessed in the provideInput() method. I've fixed this in my follow-up PR here: b900551

}
val key = this.removePrefix("$src/").replace('/', File.separatorChar)
return "$dst${File.separatorChar}$SCREENSHOT_DIR${File.separatorChar}$key"
return File(dstFile, "$SCREENSHOT_DIR${File.separatorChar}$key").path
}

private fun pullScreenshots() {
val dst = destinationImageDirectory
File(dst).assurePath()
val dstFile = if (File(dst).isAbsolute) {
File(dst)
} else {
File(project.projectDir, dst)
}
dstFile.assurePath()

val failedScreenshots = listFailedScreenshotsWithPath(
src = screenshotDirectory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ open class ScreenshotRecordTask : TestifyDefaultTask() {

ScreenshotTestTask.setDependencies(taskNameProvider, project)

screenshotClearTask.mustRunAfter(getInstallDebugAndroidTestTask(project))
screenshotPullTask.mustRunAfter(getInstallDebugAndroidTestTask(project))
getInstallDebugAndroidTestTask(project)?.let { installDebugAndroidTestTask ->
screenshotClearTask.mustRunAfter(installDebugAndroidTestTask)
screenshotPullTask.mustRunAfter(installDebugAndroidTestTask)
}
}

override fun taskName() = "screenshotRecord"
Expand Down
43 changes: 32 additions & 11 deletions Plugins/Gradle/src/test/kotlin/dev/testify/internal/AdbTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,20 @@
*/
package dev.testify.internal

import com.android.build.gradle.TestedExtension
import com.android.build.api.dsl.SdkComponents
import com.android.build.api.variant.ApplicationAndroidComponentsExtension
import com.android.build.api.variant.LibraryAndroidComponentsExtension
import com.google.common.truth.Truth.assertThat
import dev.testify.test.BaseTest
import io.mockk.every
import io.mockk.impl.annotations.RelaxedMockK
import io.mockk.mockkObject
import io.mockk.mockkStatic
import io.mockk.slot
import io.mockk.unmockkStatic
import io.mockk.verify
import org.gradle.api.GradleException
import org.gradle.api.Project
import org.gradle.api.file.RegularFile
import org.gradle.api.provider.Provider
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows
Expand All @@ -45,7 +47,19 @@ class AdbTest : BaseTest() {
lateinit var project: Project

@RelaxedMockK
lateinit var extension: TestedExtension
lateinit var extensions: org.gradle.api.plugins.ExtensionContainer

@RelaxedMockK
lateinit var androidComponents: ApplicationAndroidComponentsExtension

@RelaxedMockK
lateinit var sdkComponents: SdkComponents

@RelaxedMockK
lateinit var adbProvider: Provider<RegularFile>

@RelaxedMockK
lateinit var regularFile: RegularFile

@RelaxedMockK
lateinit var adbExecutable: File
Expand All @@ -64,17 +78,21 @@ class AdbTest : BaseTest() {
override fun setUp() {
super.setUp()

every { extension.adbExecutable } returns adbExecutable
every { project.extensions } returns extensions
every { extensions.findByType(ApplicationAndroidComponentsExtension::class.java) } returns androidComponents
every { extensions.findByType(LibraryAndroidComponentsExtension::class.java) } returns null
every { androidComponents.sdkComponents } returns sdkComponents
every { sdkComponents.adb } returns adbProvider
every { adbProvider.get() } returns regularFile
every { regularFile.asFile } returns adbExecutable
every { adbExecutable.absolutePath } returns "/usr/bin/adb"

mockkStatic("dev.testify.internal.ClientUtilitiesKt")
mockkStatic(Project::android)
mockkStatic(Project::isVerbose)
mockkStatic(Project::user)
mockkStatic(::println)
mockkStatic(::runProcess)
mockkObject(Device)

every { any<Project>().android } returns extension
every { any<Project>().isVerbose } returns false
every { any<Project>().user } returns null
every { println(any(), any()) } returns Unit
Expand All @@ -97,9 +115,11 @@ class AdbTest : BaseTest() {

@Test
fun `WHEN init AND no android closure THEN throw exception`() {
unmockkStatic(Project::android)
every { extensions.findByType(ApplicationAndroidComponentsExtension::class.java) } returns null
every { extensions.findByType(LibraryAndroidComponentsExtension::class.java) } returns null
Adb.init(project)
assertThrows<GradleException> {
Adb.init(project)
Adb().argument("test").execute()
}
}

Expand All @@ -112,8 +132,9 @@ class AdbTest : BaseTest() {
fun `WHEN init AND no adb path THEN throw exception`() {
@Suppress("NULLABILITY_MISMATCH_BASED_ON_JAVA_ANNOTATIONS")
every { adbExecutable.absolutePath } returns null
Adb.init(project)
assertThrows<GradleException> {
Adb.init(project)
Adb().argument("test").execute()
}
}

Expand Down