snazy commented on code in PR #2283:
URL: https://github.com/apache/polaris/pull/2283#discussion_r2259381321


##########
build-logic/src/main/kotlin/polaris-runtime.gradle.kts:
##########
@@ -52,6 +52,14 @@ testing {
       }
       sources { java.srcDirs(tasks.named("quarkusGenerateCodeTests")) }
     }
+    register<JvmTestSuite>("cloudTest") {
+      targets.all {
+        tasks.named("compileCloudTestJava").configure {
+          dependsOn(tasks.named("compileQuarkusTestGeneratedSourcesJava"))
+        }
+      }

Review Comment:
   ```suggestion
         tasks.named("compileCloudTestJava").configure {
           dependsOn("compileQuarkusTestGeneratedSourcesJava")
         }
   ```
   (`tasks` is `Project.tasks`, not part of `JvmTestSuite` - not to confuse w/ 
`JvmTestSuite.testTask` )
   
   Same change can be applied above.



##########
runtime/service/build.gradle.kts:
##########
@@ -212,50 +212,55 @@ tasks.named<Test>("test").configure {
   jvmArgs("-Xshare:off")
 }
 
-tasks.named<Test>("intTest").configure {
-  maxParallelForks = 1
-
-  val logsDir = project.layout.buildDirectory.get().asFile.resolve("logs")
-
-  // JVM arguments provider does not interfere with Gradle's cache keys
-  jvmArgumentProviders.add(
-    CommandLineArgumentProvider {
-      // Same issue as above: allow a java security manager after Java 21
-      // (this setting is for the application under test, while the setting 
above is for test code).
-      val securityManagerAllow = "-Djava.security.manager=allow"
-
-      val args = mutableListOf<String>()
-
-      // Example: to attach a debugger to the spawned JVM running Quarkus, add
-      // 
-Dquarkus.test.arg-line=-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005
-      // to your test configuration.
-      val explicitQuarkusTestArgLine = 
System.getProperty("quarkus.test.arg-line")
-      var quarkusTestArgLine =
-        if (explicitQuarkusTestArgLine != null) "$explicitQuarkusTestArgLine 
$securityManagerAllow"
-        else securityManagerAllow
-
-      args.add("-Dquarkus.test.arg-line=$quarkusTestArgLine")
-      // This property is not honored in a per-profile application.properties 
file,
-      // so we need to set it here.
-      
args.add("-Dquarkus.log.file.path=${logsDir.resolve("polaris.log").absolutePath}")
-
-      // Add `quarkus.*` system properties, other than the ones explicitly set 
above
-      System.getProperties()
-        .filter {
-          it.key.toString().startsWith("quarkus.") &&
-            !"quarkus.test.arg-line".equals(it.key) &&
-            !"quarkus.log.file.path".equals(it.key)
-        }
-        .forEach { args.add("${it.key}=${it.value}") }
-
-      args
+tasks
+  .withType<Test>()
+  .matching { it.name in listOf("intTest", "cloudTest") }
+  .configureEach {

Review Comment:
   ```suggestion
   listOf("intTest", "cloudTest").map { tasks.named<Test>(it) }.forEach { 
it.configure {
   ```
   
   The `.matching { it.name` causes an eager configuration of all test tasks.
   



##########
build-logic/src/main/kotlin/polaris-runtime.gradle.kts:
##########
@@ -52,6 +52,14 @@ testing {
       }
       sources { java.srcDirs(tasks.named("quarkusGenerateCodeTests")) }
     }
+    register<JvmTestSuite>("cloudTest") {
+      targets.all {
+        tasks.named("compileCloudTestJava").configure {

Review Comment:
   ```suggestion
         tasks.named(sources.compileJavaTaskName).configure {
   ```
   
   should work as well



##########
build-logic/src/main/kotlin/polaris-runtime.gradle.kts:
##########
@@ -78,7 +86,14 @@ configurations.named("intTestImplementation").configure {
   extendsFrom(configurations.getByName("testImplementation"))
 }
 
-dependencies { add("intTestImplementation", 
java.sourceSets.getByName("test").output.dirs) }
+configurations.named("cloudTestImplementation").configure {
+  extendsFrom(configurations.getByName("testImplementation"))
+}
+
+dependencies {
+  add("intTestImplementation", java.sourceSets.getByName("test").output.dirs)
+  add("cloudTestImplementation", java.sourceSets.getByName("test").output.dirs)

Review Comment:
   Can probably moved into the test suites `.register` closure as
   ``` kotlin
         dependencies {
           implementation(java.sourceSets.getByName("test").output.dirs)
         }
   ```



##########
build-logic/src/main/kotlin/polaris-runtime.gradle.kts:
##########
@@ -52,6 +52,14 @@ testing {
       }
       sources { java.srcDirs(tasks.named("quarkusGenerateCodeTests")) }
     }
+    register<JvmTestSuite>("cloudTest") {

Review Comment:
   I _think_ with all the suggestions below, it could be unified as:
   ```kotlin
       fun intTestSuiteConfigure(testSuite: JvmTestSuite) = testSuite.run {
         targets.all { 
           testTask.configure {
   // TODO configuration from lines 37..42 went here
             // For Quarkus...
             //
             // 
io.quarkus.test.junit.IntegrationTestUtil.determineBuildOutputDirectory(java.net.URL)
             // is not smart enough :(
             systemProperty("build.output.directory", 
layout.buildDirectory.asFile.get())
             dependsOn(tasks.named("quarkusBuild"))
           }
         }
         tasks.named(sources.compileJavaTaskName).configure {
           dependsOn("compileQuarkusTestGeneratedSourcesJava")
         }
         configurations.named(sources.runtimeOnlyConfigurationName).configure {
           extendsFrom(configurations.getByName("testRuntimeOnly"))
         }
         
configurations.named(sources.implementationConfigurationName).configure {
   // Let the test's implementation config extend testImplementation, so it 
also inherits the
   // project's "main" implementation dependencies (not just the "api" 
configuration)
           extendsFrom(configurations.getByName("testImplementation"))
         }
         sources { java.srcDirs(tasks.named("quarkusGenerateCodeTests")) }
       }
       listOf("intTest", "cloudTest").forEach { 
register<JvmTestSuite>(it).configure { intTestSuiteConfigure(this) } }
   ```
   



##########
build-logic/src/main/kotlin/polaris-runtime.gradle.kts:
##########
@@ -78,7 +86,14 @@ configurations.named("intTestImplementation").configure {
   extendsFrom(configurations.getByName("testImplementation"))
 }
 
-dependencies { add("intTestImplementation", 
java.sourceSets.getByName("test").output.dirs) }
+configurations.named("cloudTestImplementation").configure {
+  extendsFrom(configurations.getByName("testImplementation"))
+}

Review Comment:
   Can probably moved into the test suites `.register` closure as
   ```kotlin
         
configurations.named(sources.implementationConfigurationName).configure {
           extendsFrom(configurations.get("testImplementation"))
         }
   ```
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to