Configure tasks lazily (#99)

* Configure tasks lazily This fixes an issue where applying OkReplay plugin would cause all tasks in the project to be configured eagerly. * Replace plugins.withId with plugins.withType * Make a comment clearer
airbnb · Jan 23, 2020 · 42d4d6a · 42d4d6a
1 parent 02271d1
commit 42d4d6a
Show file tree

Hide file tree

Showing 11 changed files with 172 additions and 130 deletions.
diff --git a/okreplay-gradle-plugin/build.gradle b/okreplay-gradle-plugin/build.gradle
@@ -22,4 +22,8 @@ test {
   testLogging.showStandardStreams = isCi
 }
 
+validateTaskProperties {
+  failOnWarning = true
+}
+
 apply from: rootProject.file('gradle/gradle-mvn-push.gradle')
diff --git a/okreplay-gradle-plugin/src/main/kotlin/okreplay/ClearTapesTask.kt b/okreplay-gradle-plugin/src/main/kotlin/okreplay/ClearTapesTask.kt
@@ -2,29 +2,25 @@ package okreplay
 
 import okreplay.OkReplayPlugin.Companion.REMOTE_TAPES_DIR
 import org.gradle.api.DefaultTask
-import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.TaskAction
-import javax.inject.Inject
-
-open class ClearTapesTask : DefaultTask(), TapeTask {
-  @get:Input override var deviceBridge: DeviceBridge? = null
-  @get:Input override var packageName: String? = null
 
+abstract class ClearTapesTask : DefaultTask(), TapeTask {
   init {
     description = "Remove OkReplay tapes from the device"
-    group = "okreplay"
+    group = "OkReplay"
   }
 
   @Suppress("unused")
   @TaskAction
   internal fun clearTapes() {
-    deviceBridge!!.devices().forEach {
-      val externalStorage = it.externalStorageDir()
+    val deviceBridge = DeviceBridgeProvider.get(adbPath.get(), adbTimeout.get(), logger)
+    deviceBridge.devices().forEach { device ->
+      val externalStorage = device.externalStorageDir()
       try {
-        it.deleteDirectory("$externalStorage/$REMOTE_TAPES_DIR/$packageName/")
+        device.deleteDirectory("$externalStorage/$REMOTE_TAPES_DIR/${packageName.get()}/")
       } catch (e: RuntimeException) {
-        project.logger.error("ADB Command failed: ${e.message}")
+        logger.error("ADB Command failed: ${e.message}")
       }
     }
   }
-}
+}
diff --git a/okreplay-gradle-plugin/src/main/kotlin/okreplay/DeviceBridgeProvider.kt b/okreplay-gradle-plugin/src/main/kotlin/okreplay/DeviceBridgeProvider.kt
@@ -1,22 +1,22 @@
 package okreplay
 
 import com.android.annotations.VisibleForTesting
-import org.gradle.api.Project
+import org.gradle.api.logging.Logger
 import java.io.File
 
 internal class DeviceBridgeProvider {
   companion object {
     private var instance: DeviceBridge? = null
 
-    internal fun get(adbPath: File, adbTimeoutMs: Int, project: Project): DeviceBridge =
+    internal fun get(adbPath: File, adbTimeoutMs: Int, logger: Logger): DeviceBridge =
         if (instance != null) {
           instance as DeviceBridge
         } else {
-          DeviceBridge(adbPath, adbTimeoutMs, project.logger)
+          DeviceBridge(adbPath, adbTimeoutMs, logger)
         }
 
     @VisibleForTesting internal fun setInstance(deviceBridge: DeviceBridge) {
       instance = deviceBridge
     }
   }
-}
+}
diff --git a/okreplay-gradle-plugin/src/main/kotlin/okreplay/OkReplayPlugin.kt b/okreplay-gradle-plugin/src/main/kotlin/okreplay/OkReplayPlugin.kt
@@ -1,77 +1,69 @@
 package okreplay
 
-import com.android.build.gradle.*
+import com.android.build.gradle.AppExtension
+import com.android.build.gradle.AppPlugin
+import com.android.build.gradle.BaseExtension
+import com.android.build.gradle.DynamicFeaturePlugin
+import com.android.build.gradle.LibraryExtension
+import com.android.build.gradle.LibraryPlugin
 import com.android.build.gradle.api.BaseVariant
+import com.android.build.gradle.internal.api.TestedVariant
+import org.gradle.api.DomainObjectSet
 import org.gradle.api.Plugin
 import org.gradle.api.Project
-import org.gradle.api.Task
-import org.gradle.api.UnknownTaskException
-import org.gradle.api.internal.DefaultDomainObjectSet
-import javax.inject.Inject
 
-class OkReplayPlugin
-@Inject constructor() : Plugin<Project> {
-  private lateinit var project: Project
+class OkReplayPlugin : Plugin<Project> {
 
   override fun apply(project: Project) {
-    this.project = project
-    if (project.plugins.hasPlugin(AppPlugin::class.java)
-        || project.plugins.hasPlugin(LibraryPlugin::class.java)) {
-      applyPlugin()
-    } else {
-      throw IllegalArgumentException("OkReplay plugin couldn't be applied. "
-          + "The Android or Library plugin must be configured first.")
+    project.plugins.withType(AppPlugin::class.java) {
+      project.registerTasks()
+    }
+    project.plugins.withType(LibraryPlugin::class.java) {
+      project.registerTasks()
+    }
+    project.plugins.withType(DynamicFeaturePlugin::class.java) {
+      project.registerTasks()
     }
   }
 
-  private fun Task.runBefore(dependentTaskName: String) {
-    try {
-      val taskToFind = project.tasks.getByName(dependentTaskName)
-      taskToFind.dependsOn(this)
-    } catch (e: UnknownTaskException) {
-      project.tasks.whenTaskAdded { dependentTask ->
-        if (dependentTask.name == dependentTaskName) {
-          dependentTask.dependsOn(this)
-        }
+  private fun Project.registerTasks() {
+    getVariants().all { variant ->
+      // Only variants with build type matching android.testBuildType will have a test variant
+      val testVariant = (variant as TestedVariant).testVariant ?: return@all
+
+      val androidConfig = androidConfig()
+      val adbPath = androidConfig.adbExecutable
+      val adbTimeoutMs = androidConfig.adbOptions.timeOutInMs
+
+      val targetName = variant.name.capitalize()
+      val pullTapesTask = tasks.register("pull${targetName}OkReplayTapes", PullTapesTask::class.java) {
+        it.adbPath.set(adbPath)
+        it.adbTimeout.set(adbTimeoutMs)
+        it.packageName.set(testVariant.applicationId)
+        it.outputDir.set(file(LOCAL_TAPES_DIR))
+      }
+      val clearTapesTask = tasks.register("clear${targetName}OkReplayTapes", ClearTapesTask::class.java) {
+        it.adbPath.set(adbPath)
+        it.adbTimeout.set(adbTimeoutMs)
+        it.packageName.set(testVariant.applicationId)
       }
-    }
-  }
 
-  private fun Task.runAfter(dependentTaskName: String) {
-    try {
-      val taskToFind = project.tasks.getByName(dependentTaskName)
-      taskToFind.finalizedBy(this)
-    } catch (e: UnknownTaskException) {
-      project.tasks.whenTaskAdded { dependentTask ->
-        if (dependentTask.name == dependentTaskName) {
-          dependentTask.finalizedBy(this)
-        }
+      testVariant.connectedInstrumentTestProvider.configure { task ->
+        task.dependsOn(clearTapesTask)
+        task.finalizedBy(pullTapesTask)
       }
     }
   }
 
-  private fun applyPlugin() {
-    project.afterEvaluate {
-      it.getVariants().all {
-        val flavorNameCapitalized = it.flavorName.capitalize()
-        val buildNameCapitalized = it.buildType.name.capitalize()
-        val targetName = "$flavorNameCapitalized$buildNameCapitalized"
-        val pullTapesTask: TapeTask =
-            project.tasks.create("pull${targetName}OkReplayTapes", PullTapesTask::class.java)
-        val clearTapesTask: TapeTask =
-            project.tasks.create("clear${targetName}OkReplayTapes", ClearTapesTask::class.java)
-        val extension = project.extensions.getByType(BaseExtension::class.java)
-        val adbPath = extension.adbExecutable
-        val adbTimeoutMs = extension.adbOptions.timeOutInMs
-        val testApplicationId = project.testApplicationId()
-        val deviceBridge = DeviceBridgeProvider.get(adbPath, adbTimeoutMs, project)
-        listOf(pullTapesTask, clearTapesTask).forEach {
-          it.deviceBridge = deviceBridge
-          it.packageName = testApplicationId
-        }
-        clearTapesTask.runBefore("connected${targetName}AndroidTest")
-        pullTapesTask.runAfter("connected${targetName}AndroidTest")
-      }
+  private fun Project.androidConfig(): BaseExtension {
+    return extensions.getByType(BaseExtension::class.java)
+  }
+
+  private fun Project.getVariants(): DomainObjectSet<out BaseVariant> {
+    return when (val androidConfig = androidConfig()) {
+      is AppExtension -> androidConfig.applicationVariants
+      is LibraryExtension -> androidConfig.libraryVariants
+      else -> throw IllegalStateException("Invalid project type")
     }
   }
 
@@ -81,32 +73,5 @@ class OkReplayPlugin
     // This is also hardcoded in AndroidTapeRoot#getSdcardDir()
     // Need to use the same value in both places
     const val REMOTE_TAPES_DIR = "okreplay/tapes"
-
-    private fun Project.androidConfig(): AndroidConfig {
-      return extensions.getByName("android") as BaseExtension
-    }
-
-    private fun Project.testApplicationId(): String {
-      val androidConfig = androidConfig()
-      return if (androidConfig is AppExtension || androidConfig is LibraryExtension) {
-        if (!(androidConfig as TestedExtension).testVariants.isEmpty()) {
-          androidConfig.testVariants.first().applicationId
-        } else {
-          ""
-        }
-      } else {
-        throw IllegalStateException("Invalid project type")
-      }
-    }
-
-    private fun Project.getVariants(): DefaultDomainObjectSet<out BaseVariant> {
-      val androidConfig = androidConfig()
-      when (androidConfig) {
-        is AppExtension -> @Suppress("UNCHECKED_CAST")
-        return androidConfig.applicationVariants as DefaultDomainObjectSet<BaseVariant>
-        is LibraryExtension -> return androidConfig.libraryVariants
-        else -> throw IllegalStateException("Invalid project type")
-      }
-    }
   }
 }
diff --git a/okreplay-gradle-plugin/src/main/kotlin/okreplay/PullTapesTask.kt b/okreplay-gradle-plugin/src/main/kotlin/okreplay/PullTapesTask.kt
@@ -1,42 +1,41 @@
 package okreplay
 
-import okreplay.OkReplayPlugin.Companion.LOCAL_TAPES_DIR
 import okreplay.OkReplayPlugin.Companion.REMOTE_TAPES_DIR
 import org.apache.commons.io.FileUtils
 import org.gradle.api.DefaultTask
-import org.gradle.api.tasks.Input
+import org.gradle.api.provider.Property
 import org.gradle.api.tasks.OutputDirectory
 import org.gradle.api.tasks.TaskAction
 import org.gradle.api.tasks.TaskExecutionException
 import java.io.File
-import javax.inject.Inject
 
-open class PullTapesTask : DefaultTask(), TapeTask {
-  @get:OutputDirectory private var outputDir: File? = null
-  @get:Input override var packageName: String? = null
-  @get:Input override var deviceBridge: DeviceBridge? = null
+abstract class PullTapesTask : DefaultTask(), TapeTask {
+  @get:OutputDirectory
+  abstract val outputDir: Property<File>
 
   init {
-    description = "Pull OkReplay tapes from the Device SD Card"
-    group = "okreplay"
+    description = "Pull OkReplay tapes from the device"
+    group = "OkReplay"
+    outputs.upToDateWhen { false }
   }
 
   @Suppress("unused")
   @TaskAction
   fun pullTapes() {
-    outputDir = project.file(LOCAL_TAPES_DIR)
-    val localDir = outputDir!!.absolutePath
-    FileUtils.forceMkdir(outputDir)
-    deviceBridge!!.devices().forEach {
-      val externalStorage = it.externalStorageDir()
+    val localDir = outputDir.get()
+    FileUtils.forceMkdir(localDir)
+
+    val deviceBridge = DeviceBridgeProvider.get(adbPath.get(), adbTimeout.get(), logger)
+    deviceBridge.devices().forEach { device ->
+      val externalStorage = device.externalStorageDir()
       if (externalStorage.isNullOrBlank()) {
         throw TaskExecutionException(this,
             RuntimeException("Failed to retrieve the device external storage dir."))
       }
       try {
-        it.pullDirectory(localDir, "$externalStorage/$REMOTE_TAPES_DIR/$packageName/")
+        device.pullDirectory(localDir.absolutePath, "$externalStorage/$REMOTE_TAPES_DIR/${packageName.get()}/")
       } catch (e: RuntimeException) {
-        project.logger.error("ADB Command failed: ${e.message}")
+        logger.error("ADB Command failed: ${e.message}")
       }
     }
   }

diff --git a/okreplay-gradle-plugin/src/main/kotlin/okreplay/TapeTask.kt b/okreplay-gradle-plugin/src/main/kotlin/okreplay/TapeTask.kt
@@ -1,9 +1,13 @@
 package okreplay
 
 import org.gradle.api.Task
+import org.gradle.api.provider.Property
 import org.gradle.api.tasks.Input
+import org.gradle.api.tasks.Internal
+import java.io.File
 
 interface TapeTask : Task {
-  @get:Input var packageName: String?
-  @get:Input var deviceBridge: DeviceBridge?
-}
+  @get:Input val packageName: Property<String>
+  @get:Internal val adbPath: Property<File>
+  @get:Internal val adbTimeout: Property<Int>
+}
diff --git a/okreplay-gradle-plugin/src/test/kotlin/okreplay/DeviceTest.kt b/okreplay-gradle-plugin/src/test/kotlin/okreplay/DeviceTest.kt
@@ -3,17 +3,21 @@ package okreplay
 import org.gradle.api.logging.Logger
 import org.junit.Test
 import org.mockito.Mockito.*
-import okreplay.DeviceInterface
 
 class DeviceTest {
-  @Test fun deleteDirectory() {
-    val deviceInterface = mock(DeviceInterface::class.java)
-    val logger = mock(Logger::class.java)
-    val device = Device(deviceInterface, logger)
+  private val deviceInterface = mock(DeviceInterface::class.java)
+  private val logger = mock(Logger::class.java)
+  private val device = Device(deviceInterface, logger)
+
+  @Test fun pullDirectory() {
+    device.pullDirectory("/local/dir", "/test/dir")
 
+    verify(deviceInterface, only()).pull("/test/dir", "/local/dir")
+  }
+
+  @Test fun deleteDirectory() {
     device.deleteDirectory("/test/dir")
 
-    verify(deviceInterface).delete("/test/dir")
-    verifyNoMoreInteractions(deviceInterface)
+    verify(deviceInterface, only()).delete("/test/dir")
   }
-}
+}