basic Gradle integration test scaffolding #789

RBusarow · 2023-11-20T20:00:46Z

No description provided.

JoelWilcox

Partial review

.github/workflows/ci.yml

JoelWilcox · 2023-11-28T22:25:50Z

...nventions/src/main/kotlin/com/squareup/anvil/conventions/IntegrationTestsConventionPlugin.kt

+import org.gradle.language.base.plugins.LifecycleBasePlugin
+import org.gradle.plugins.ide.idea.model.IdeaModel
+
+abstract class IntegrationTestsConventionPlugin : Plugin<Project> {


Can you add a doc comment for this?

Also is there a more specific name that works here? Right now it's a bit confusing having an integrationTest task and the integration-tests module that are both unrelated to each other. Maybe this could be pluginIntegrationTest, gradleIntegrationTest, etc for the task name to differentiate it?

JoelWilcox · 2023-11-28T23:41:11Z

build-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/LibraryPlugin.kt

+    when (target.path) {
+      ":annotations",
+      ":annotations-optional",
+      ":compiler",
+      ":compiler-api",
+      ":compiler-utils",
+      ":gradle-plugin",
+      -> target.setBuildDir()


Conditionally applying by target path/module like this feels unexpected instead of setting the build dir for anyone that has chosen to apply this plugin. What do you think about having a separate plugin to handle the case for simple lib modules that only want the above (kotlin-jvm, ktlint) plugins applied? The simpler plugin could be something like MinimalLibraryPlugin or the other direction where the library plugin with this build dir setup logic could be named around what it's doing differently to indicate which modules should use it.

JoelWilcox · 2023-11-28T23:43:19Z

build-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/utils.kt

+
+internal fun Project.isInMainAnvilBuild() = rootProject.name == "anvil"
+
+internal fun Project.isInDelegateBuild() = rootProject.name == "delegate"


Can you add some high level documentation somewhere around the main build vs delegate build, why we have both, etc?

build-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/BasePlugin.kt

...d-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/KtlintConventionPlugin.kt

...-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/PublishConventionPlugin.kt

JoelWilcox · 2023-12-01T02:48:29Z

...-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/PublishConventionPlugin.kt

+      target.extras["skipDokka"] = true
+    }
+
+    target.rootProject.tasks.named("publishToBuildM2").dependsOn(publishToBuildM2)


Am I reading correctly that this is making sure when we run publish at the root level, that all subprojects applying the plugin get their publish task executed?

Yep. The publish tasks don't chain together the way that build tasks do, so this is a pretty common catch-all.

build-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/BasePlugin.kt

build-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/CompositePlugin.kt

JoelWilcox · 2023-12-01T21:57:52Z

build-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/LibraryPlugin.kt

+  override fun beforeApply(target: Project) {
+    target.plugins.apply(target.libs.plugins.kotlin.jvm.pluginId)
+
+    target.plugins.apply(PublishConventionPlugin::class.java)


Can we keep publishing separate and have libs apply it as needed? Not every library necessarily needs to be published, unless we expect non-published library modules to use a different plugin

For example, the use-case I'm thinking of is that we create a new module like compiler-utils-2 that is only used internally by compiler-utils and does not need to be exposed to anyone else.

JoelWilcox · 2023-12-01T22:02:25Z

build-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/RootPlugin.kt

+
+    target.logVersionInfo()
+
+    if (target.isInMainAnvilBuild() || target.isInDelegateBuild()) {


What are the other options beyond main build and delegate build here?

Once upon a time...

I forgot to clean this up. :)

build-logic/settings/src/main/kotlin/com/squareup/anvil/builds/settings/SettingsPlugin.kt

...logic/conventions/src/main/kotlin/com/squareup/anvil/benchmark/CreateBenchmarkProjectTask.kt

build-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/CompositePlugin.kt

JoelWilcox · 2023-12-05T22:02:47Z

build-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/CompositePlugin.kt

+      val newRequests = oldRequests.map { request ->
+
+        val originalSplit = request.args.splitTaskArgs()
+
+        val taskPaths = originalSplit.mapTo(mutableSetOf()) { it.first() }
+
+        val includedProjects = gradle.includedBuilds
+          .asSequence()
+          .filterIsInstance<IncludedBuildImpl>()
+          .flatMap { impl ->
+
+            val defaultIncludedBuild = impl.target as DefaultIncludedBuild
+
+            defaultIncludedBuild.mutableModel.projectRegistry.allProjects
+          }
+
+        val newSplit = originalSplit.flatMap { taskWithArgs ->
+
+          val taskName = taskWithArgs.first()
+
+          if (taskName.startsWith(':')) {
+            // qualified task names aren't propagated to included builds
+            return@flatMap listOf(taskWithArgs)
+          }
+
+          // Don't propagate help tasks
+          if (taskName == "help") return@flatMap listOf(taskWithArgs)
+
+          val included = includedProjects.mapNotNull { includedProject ->
+
+            val includedPath = "${includedProject.identityPath}:$taskName"
+
+            // Don't include tasks that are already in the task graph
+            if (taskPaths.contains(includedPath)) return@mapNotNull null
+
+            includedProject.tasks.namedOrNull(taskName) ?: return@mapNotNull null
+
+            target.logger.quiet("The task $taskName will delegate to $includedPath")
+
+            buildList<String> {
+              add(includedPath)
+              addAll(taskWithArgs.subList(1, taskWithArgs.size))
+            }
+          }
+
+          buildList {
+
+            // Looks to see if any project in this build has a task with this name.
+            val resolvedInRootBuild = target.allprojects


Could you add some tests around some of this task finagling logic? Could be a follow-up PR in the interest of not further delaying the rest of the changes

...-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/PublishConventionPlugin.kt

JoelWilcox · 2023-12-05T22:29:23Z

build-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/BasePlugin.kt

+  private fun configureTests(target: Project) {
+    target.tasks.withType(Test::class.java).configureEach { task ->
+
+      task.maxParallelForks = Runtime.getRuntime().availableProcessors()


It looks like previously we only did this for local machines. Is there any downside to now setting it on CI?

Nope. It's a pretty common setting.

JoelWilcox · 2023-12-05T22:33:52Z

build-logic/conventions/src/main/kotlin/com/squareup/anvil/conventions/BasePlugin.kt

+      task.maxParallelForks = Runtime.getRuntime().availableProcessors()
+
+      task.testLogging { logging ->
+        logging.events("passed", "skipped", "failed")


Why do we not include the 'standardOut', 'standardError' events now?

Anything that we actually need to see (for a failed test) will still be included in the output as the exception/stacktrace. There's so much noise in the console output that it becomes difficult to find an actual error when you need to.

RBusarow force-pushed the rick/gradle-integration-test-support branch 4 times, most recently from 4632e6f to 8584e97 Compare November 20, 2023 23:23

RBusarow marked this pull request as ready for review November 21, 2023 23:10

RBusarow force-pushed the rick/gradle-integration-test-support branch from 8584e97 to 095e67a Compare November 28, 2023 04:45

JoelWilcox reviewed Nov 28, 2023

View reviewed changes

RBusarow force-pushed the rick/gradle-integration-test-support branch 3 times, most recently from 65704b8 to 078e8ea Compare December 1, 2023 21:05

JoelWilcox reviewed Dec 1, 2023

View reviewed changes

RBusarow force-pushed the rick/gradle-integration-test-support branch 5 times, most recently from 91fae50 to c6b71f3 Compare December 4, 2023 22:17

RBusarow added 14 commits December 5, 2023 14:02

add basic Gradle integration test scaffolding

27ccd61

making a better composite build

39dbe13

fix the plugin-integration-tests job's name

3d323cd

introduce BasePlugin and MinimalPlugin conventions

c6af1dd

Put sample and integration-tests into the delegate build

78afae2

add kdoc to CompositePlugin

fcf5f6b

don't bundle the publishing plugin application into the library plugin

f4f609e

fix project paths in ci workflow

4fd92cb

don't include the benchmark build as part of the main build

df2fa97

remove the explicit task delegation from the ktlint plugin

c50ce91

only apply the benchmark plugin to the main build's root

6604afa

misc cleanup

299753e

fix :gradle-plugin's plugin application order

36b75b9

properly parse --tests parameters in CompositePlugin

f28bd56

RBusarow force-pushed the rick/gradle-integration-test-support branch from c6b71f3 to f28bd56 Compare December 5, 2023 20:02

JoelWilcox approved these changes Dec 5, 2023

View reviewed changes

RBusarow merged commit 9bfb9a1 into main Dec 6, 2023
21 checks passed

RBusarow deleted the rick/gradle-integration-test-support branch December 6, 2023 15:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

basic Gradle integration test scaffolding #789

basic Gradle integration test scaffolding #789

RBusarow commented Nov 20, 2023

JoelWilcox left a comment

JoelWilcox Nov 28, 2023

RBusarow Dec 2, 2023

JoelWilcox Nov 28, 2023

RBusarow Dec 2, 2023

JoelWilcox Nov 28, 2023

JoelWilcox Dec 1, 2023

RBusarow Dec 2, 2023

JoelWilcox Dec 1, 2023

RBusarow Dec 2, 2023

JoelWilcox Dec 1, 2023

RBusarow Dec 1, 2023

JoelWilcox Dec 5, 2023

RBusarow Dec 5, 2023

JoelWilcox Dec 5, 2023

RBusarow Dec 5, 2023

JoelWilcox Dec 5, 2023

RBusarow Dec 5, 2023


		internal fun Project.isInMainAnvilBuild() = rootProject.name == "anvil"

		internal fun Project.isInDelegateBuild() = rootProject.name == "delegate"


		target.logVersionInfo()

		if (target.isInMainAnvilBuild() \|\| target.isInDelegateBuild()) {

basic Gradle integration test scaffolding #789

basic Gradle integration test scaffolding #789

Conversation

RBusarow commented Nov 20, 2023

JoelWilcox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment