From 06f94fd0ac5b5d9a6cad883ef08bd0425d39df77 Mon Sep 17 00:00:00 2001
From: translatenix <119817707+translatenix@users.noreply.github.com>
Date: Thu, 29 Feb 2024 17:42:23 -0800
Subject: [PATCH] Incorporate review feedback

- improve how tests get access to Pkl distributions
---
 gradle/libs.versions.toml                     |  1 +
 pkl-executor/pkl-executor.gradle.kts          | 20 ++++++-------
 .../org/pkl/executor/EmbeddedExecutorTest.kt  | 29 ++++++++++++-------
 3 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
index 62fc47f38..c83f86681 100644
--- a/gradle/libs.versions.toml
+++ b/gradle/libs.versions.toml
@@ -81,6 +81,7 @@ msgpack = { group = "org.msgpack", name = "msgpack-core", version.ref = "msgpack
 nuValidator = { group = "nu.validator", name = "validator", version.ref = "nuValidator" }
 # to be replaced with https://github.com/usethesource/capsule or https://github.com/lacuna/bifurcan
 paguro = { group = "org.organicdesign", name = "Paguro", version.ref = "paguro" }
+pklConfigJavaAll025 = { group = "org.pkl-lang", name = "pkl-config-java-all", version = "0.25.0" }
 shadowPlugin = { group = "gradle.plugin.com.github.johnrengelman", name = "shadow", version.ref = "shadowPlugin" }
 slf4jApi = { group = "org.slf4j", name = "slf4j-api", version.ref = "slf4j" }
 slf4jSimple = { group = "org.slf4j", name = "slf4j-simple", version.ref = "slf4j" }
diff --git a/pkl-executor/pkl-executor.gradle.kts b/pkl-executor/pkl-executor.gradle.kts
index 7ffbbee7a..59fd005d6 100644
--- a/pkl-executor/pkl-executor.gradle.kts
+++ b/pkl-executor/pkl-executor.gradle.kts
@@ -1,5 +1,3 @@
-import de.undercouch.gradle.tasks.download.Download
-
 plugins {
   pklAllProjects
   pklJavaLibrary
@@ -7,14 +5,17 @@ plugins {
   pklKotlinTest
 }
 
-val pklDistribution: Configuration by configurations.creating
+val pklDistributionCurrent: Configuration by configurations.creating
+val pklDistribution025: Configuration by configurations.creating
 
 // Because pkl-executor doesn't depend on other Pkl modules
 // (nor has overlapping dependencies that could cause a version conflict),
 // clients are free to use different versions of pkl-executor and (say) pkl-config-java-all.
 // (Pkl distributions used by EmbeddedExecutor are isolated via class loaders.)
 dependencies {
-  pklDistribution(project(":pkl-config-java", "fatJar"))
+  pklDistributionCurrent(project(":pkl-config-java", "fatJar"))
+  @Suppress("UnstableApiUsage")
+  pklDistribution025(libs.pklConfigJavaAll025)
 
   implementation(libs.slf4jApi)
 
@@ -51,17 +52,14 @@ sourceSets {
   }
 }
 
-val downloadPkl025 by tasks.registering(Download::class) {
-  src("https://repo1.maven.org/maven2/org/pkl-lang/pkl-config-java-all/0.25.0/pkl-config-java-all-0.25.0.jar")
-  dest("build/download/pkl-config-java-all-0.25.0.jar")
-  doFirst { file("build/download").mkdirs() }
-}
-
+// this task could be folded into tasks.test by switching to IntelliJ's Gradle test runner
 val prepareTest by tasks.registering {
   // used by EmbeddedExecutorTest
-  dependsOn(downloadPkl025, pklDistribution)
+  dependsOn(pklDistributionCurrent, pklDistribution025)
 }
 
 tasks.test {
   dependsOn(prepareTest)
+  systemProperty("pklDistributionCurrent", pklDistributionCurrent.singleFile)
+  systemProperty("pklDistribution025", pklDistribution025.singleFile)
 }
diff --git a/pkl-executor/src/test/kotlin/org/pkl/executor/EmbeddedExecutorTest.kt b/pkl-executor/src/test/kotlin/org/pkl/executor/EmbeddedExecutorTest.kt
index 9eed315b1..3658b3ead 100644
--- a/pkl-executor/src/test/kotlin/org/pkl/executor/EmbeddedExecutorTest.kt
+++ b/pkl-executor/src/test/kotlin/org/pkl/executor/EmbeddedExecutorTest.kt
@@ -38,8 +38,8 @@ class EmbeddedExecutorTest {
         ExecutionContext(executor1_1.value, ::convertToOptions1, "Options1, Executor1, Distribution1"),
 
         // This context has a pkl-executor version that is lower than the distribution version.
-        // It can be enabled once there is a distribution that ships (at least) with pkl-executor's SPI classes.
-        //ExecutionContext(::convertToOptions1, executor1_2.value),
+        // It can be enabled once there is a distribution that includes pkl-executor.
+        //ExecutionContext(executor1_2.value, ::convertToOptions1, "Options1, Executor1, Distribution2"),
 
         ExecutionContext(executor2_1.value, ::convertToOptions1, "Options1, Executor2, Distribution1"),
         ExecutionContext(executor2_1.value, ::convertToOptions2, "Options2, Executor2, Distribution1"),
@@ -101,19 +101,28 @@ class EmbeddedExecutorTest {
     
     // a Pkl distribution that supports ExecutorSpiOptions up to v1
     private val pklDistribution1: Path by lazy {
-      FileTestUtils.rootProjectDir
-        .resolve("pkl-executor/build/download/pkl-config-java-all-0.25.0.jar").apply {
-          if (!exists()) throw AssertionError("Missing test fixture. Run `./gradlew :pkl-executor:prepareTest` to create it.")
+      val path = System.getProperty("pklDistribution025")?.toPath() ?:
+        // can get rid of this path by switching to IntelliJ's Gradle test runner
+        System.getProperty("user.home").toPath()
+          .resolve(".gradle/caches/modules-2/files-2.1/org.pkl-lang/pkl-config-java-all/" +
+            "0.25.0/e9451dda554f1659e49ff5bdd30accd26be7bf0f/pkl-config-java-all-0.25.0.jar")
+      path.apply {
+          if (!exists()) throw AssertionError("Missing test fixture. " +
+            "To fix this problem, run `./gradlew :pkl-executor:prepareTest`.")
         }
     }
 
     // a Pkl distribution that supports ExecutorSpiOptions up to v2
     private val pklDistribution2: Path by lazy {
-      val mavenVersion = Release.current().version().withBuild(null).toString().replaceFirst("dev", "SNAPSHOT")
-      FileTestUtils.rootProjectDir
-        .resolve("pkl-config-java/build/libs/pkl-config-java-all-$mavenVersion.jar").apply {
-          if (!exists()) throw AssertionError("Missing test fixture. Run `./gradlew :pkl-executor:prepareTest` to create it.")
-        }
+      val path = System.getProperty("pklDistributionCurrent")?.toPath() ?:
+        // can get rid of this path by switching to IntelliJ's Gradle test runner
+        FileTestUtils.rootProjectDir
+          .resolve("pkl-config-java/build/libs/pkl-config-java-all-" +
+            "${Release.current().version().withBuild(null).toString().replaceFirst("dev", "SNAPSHOT")}.jar")
+      path.apply {
+        if (!exists()) throw AssertionError("Missing test fixture. " +
+          "To fix this problem, run `./gradlew :pkl-executor:prepareTest`.")
+      }
     }
 
     private fun convertToOptions2(options: ExecutorOptions): ExecutorOptions2 =