Test reporter fixes (#1597)

* Fix error message when an invalid test reporter is supplied in Gradle
* Fix Gradle property name in docs
* Fix Gradle property name in tasks
* Introduce `TestReporter.default`, and use it in places where default
is applied
* Remove calls to `convention()`; this is not required because the input
is optional anyways.

Author: bioball

docs/modules/pkl-gradle/pages/index.adoc

@@ -323,7 +323,7 @@ Whether to ignore expected example files and generate them again.
 ====
 
 [[test-reporter]]
-.test-reporter: Property<String>
+.testReporter: Property<String>
 [%collapsible]
 ====
 Default: `"spec"` +
@@ -686,7 +686,7 @@ Default: `false` +
 Whether to ignore expected example files and generate them again.
 ====
 
-.test-reporter: Property<String>
+.testReporter: Property<String>
 [%collapsible]
 ====
 Default: `"spec"` +

pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliTestOptions.kt

@@ -22,5 +22,5 @@ class CliTestOptions(
   val overwrite: Boolean = false,
   val junitAggregateReports: Boolean = false,
   val junitAggregateSuiteName: String = "pkl-tests",
-  val reporter: TestReporter = TestReporter.SPEC,
+  val reporter: TestReporter = TestReporter.default,
 )

pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/TestReporter.kt

@@ -17,5 +17,9 @@ package org.pkl.commons.cli
 
 enum class TestReporter {
   SPEC,
-  MINIMAL,
+  MINIMAL;
+
+  companion object {
+    @JvmStatic val default: TestReporter = SPEC
+  }
 }

pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/TestOptions.kt

@@ -57,7 +57,7 @@ class TestOptions : OptionGroup() {
     option(names = arrayOf("--test-reporter"), help = "Which test reporter to use for CLI output.")
       .enum<TestReporter> { it.name.lowercase() }
       .single()
-      .default(TestReporter.SPEC)
+      .default(TestReporter.default)
 
   val cliTestOptions: CliTestOptions by lazy {
     CliTestOptions(

pkl-gradle/src/main/java/org/pkl/gradle/PklPlugin.java

@@ -100,7 +100,6 @@ private void configureProjectPackageTasks(
           spec.getOutputPath()
               .convention(project.getLayout().getBuildDirectory().dir("generated/pkl/packages"));
           spec.getOverwrite().convention(false);
-          spec.getReporter().convention("spec");
           var packageTask = createTask(project, ProjectPackageTask.class, spec);
           packageTask.configure(
               task -> {
@@ -109,7 +108,7 @@ private void configureProjectPackageTasks(
                 task.getSkipPublishCheck().set(spec.getSkipPublishCheck());
                 task.getJunitReportsDir().set(spec.getJunitReportsDir());
                 task.getOverwrite().set(spec.getOverwrite());
-                task.getTestReporter().set(spec.getReporter());
+                task.getTestReporter().set(spec.getTestReporter());
               });
           project
               .getPluginManager()
@@ -280,7 +279,6 @@ private void configureTestTasks(Project project, NamedDomainObjectContainer<Test
           configureBaseSpec(project, spec);
 
           spec.getOverwrite().convention(false);
-          spec.getTestReporter().convention("spec");
 
           var testTask = createModulesTask(project, TestTask.class, spec);
           testTask.configure(

pkl-gradle/src/main/java/org/pkl/gradle/spec/ProjectPackageSpec.java

@@ -30,5 +30,5 @@ public interface ProjectPackageSpec extends BasePklSpec {
 
   Property<Boolean> getSkipPublishCheck();
 
-  Property<String> getReporter();
+  Property<String> getTestReporter();
 }

pkl-gradle/src/main/java/org/pkl/gradle/task/ProjectPackageTask.java

@@ -16,6 +16,7 @@
 package org.pkl.gradle.task;
 
 import static org.pkl.gradle.utils.PluginUtils.mapAndGetOrNull;
+import static org.pkl.gradle.utils.PluginUtils.toTestReporter;
 
 import java.io.PrintWriter;
 import java.nio.file.Path;
@@ -34,7 +35,6 @@
 import org.gradle.api.tasks.UntrackedTask;
 import org.pkl.cli.CliProjectPackager;
 import org.pkl.commons.cli.CliTestOptions;
-import org.pkl.commons.cli.TestReporter;
 
 @UntrackedTask(because = "Output names are known only after execution")
 public abstract class ProjectPackageTask extends BasePklTask {
@@ -80,18 +80,6 @@ protected void doRunTask() {
     if (projectDirectories.isEmpty()) {
       throw new InvalidUserDataException("No project directories specified.");
     }
-    TestReporter testReporter;
-    try {
-      testReporter = TestReporter.valueOf(getTestReporter().getOrElse("SPEC").toUpperCase());
-    } catch (IllegalArgumentException e) {
-      throw new InvalidUserDataException(
-          "Invalid reporter: '%s'. Valid reporter options: %s"
-              .formatted(
-                  getTestReporter().get(),
-                  TestReporter.getEntries().stream()
-                      .map(it -> it.name().toLowerCase())
-                      .collect(Collectors.joining(", "))));
-    }
 
     new CliProjectPackager(
             getCliBaseOptions(),
@@ -101,7 +89,7 @@ protected void doRunTask() {
                 getOverwrite().get(),
                 getJunitAggregateReports().getOrElse(false),
                 getJunitAggregateSuiteName().get(),
-                testReporter),
+                toTestReporter(getTestReporter())),
             getOutputPath().get().getAsFile().getAbsolutePath(),
             getSkipPublishCheck().getOrElse(false),
             new PrintWriter(System.out),

pkl-gradle/src/main/java/org/pkl/gradle/task/TestTask.java

@@ -16,10 +16,9 @@
 package org.pkl.gradle.task;
 
 import static org.pkl.gradle.utils.PluginUtils.mapAndGetOrNull;
+import static org.pkl.gradle.utils.PluginUtils.toTestReporter;
 
 import java.io.PrintWriter;
-import java.util.stream.Collectors;
-import org.gradle.api.InvalidUserDataException;
 import org.gradle.api.file.DirectoryProperty;
 import org.gradle.api.provider.Property;
 import org.gradle.api.tasks.CacheableTask;
@@ -28,7 +27,6 @@
 import org.gradle.api.tasks.OutputDirectory;
 import org.pkl.cli.CliTestRunner;
 import org.pkl.commons.cli.CliTestOptions;
-import org.pkl.commons.cli.TestReporter;
 
 @CacheableTask
 public abstract class TestTask extends ModulesTask {
@@ -57,26 +55,14 @@ public TestTask() {
 
   @Override
   protected void doRunTask() {
-    TestReporter testReporter;
-    try {
-      testReporter = TestReporter.valueOf(getTestReporter().getOrElse("SPEC").toUpperCase());
-    } catch (IllegalArgumentException e) {
-      throw new InvalidUserDataException(
-          "Invalid reporter: '%s'. Valid reporter options: %s"
-              .formatted(
-                  getTestReporter().get(),
-                  TestReporter.getEntries().stream()
-                      .map(it -> it.name().toLowerCase())
-                      .collect(Collectors.joining(", "))));
-    }
     new CliTestRunner(
             getCliBaseOptions(),
             new CliTestOptions(
                 mapAndGetOrNull(getJunitReportsDir(), it -> it.getAsFile().toPath()),
                 getOverwrite().get(),
                 getJunitAggregateReports().getOrElse(false),
                 getJunitAggregateSuiteName().get(),
-                testReporter),
+                toTestReporter(getTestReporter())),
             new PrintWriter(System.out),
             new PrintWriter(System.err))
         .run();

pkl-gradle/src/main/java/org/pkl/gradle/utils/PluginUtils.java

@@ -31,6 +31,7 @@
 import org.gradle.api.file.RegularFile;
 import org.gradle.api.provider.Provider;
 import org.jspecify.annotations.Nullable;
+import org.pkl.commons.cli.TestReporter;
 import org.pkl.core.ImportGraph;
 import org.pkl.core.util.IoUtils;
 
@@ -175,4 +176,31 @@ public static List<File> parseTransitiveFiles(RegularFile outputFile) {
     @Nullable T value = provider.getOrNull();
     return value == null ? null : f.apply(value);
   }
+
+  public static TestReporter toTestReporter(Provider<String> input) {
+    var inputStr = input.getOrNull();
+    if (inputStr == null) {
+      return TestReporter.getDefault();
+    }
+    try {
+      return TestReporter.valueOf(inputStr.toUpperCase());
+    } catch (IllegalArgumentException e) {
+      var sb = new StringBuilder("Invalid test reporter: '");
+      sb.append(inputStr).append("'. ");
+      sb.append("Valid reporters: ");
+      var isFirst = true;
+      // `enumEntries()` is not available in Kotlin versions below 1.8
+      //noinspection EnumValuesSoftDeprecateInJava
+      for (var value : TestReporter.values()) {
+        if (isFirst) {
+          isFirst = false;
+        } else {
+          sb.append(", ");
+        }
+        sb.append('\'').append(value.toString().toLowerCase()).append('\'');
+      }
+      sb.append(".");
+      throw new InvalidUserDataException(sb.toString());
+    }
+  }
 }

pkl-gradle/src/test/kotlin/org/pkl/gradle/TestsTest.kt

@@ -332,6 +332,43 @@ class TestsTest : AbstractTest() {
       )
   }
 
+  @Test
+  fun `minimal test reporter`() {
+    writeFile(
+      "test.pkl",
+      """
+      amends "pkl:test"
+
+      facts {
+        ["passes"] {
+          true
+        }
+        ["fails"] {
+          false
+        }
+      }
+      """
+        .trimIndent(),
+    )
+    writeFile("test.pkl-expected.pcf", bigTestExpected)
+
+    writeBuildFile("testReporter = 'minimal'")
+
+    runTask("evalTest", expectFailure = true)
+  }
+
+  @Test
+  fun `invalid test reporter`() {
+    writePklFile()
+    writeFile("test.pkl-expected.pcf", bigTestExpected)
+
+    writeBuildFile("testReporter = 'not a reporter'")
+
+    val output = runTask("evalTest", expectFailure = true)
+    assertThat(output.output)
+      .contains("Invalid test reporter: 'not a reporter'. Valid reporters: 'spec', 'minimal'.")
+  }
+
   private val examples =
     """
     ["user 0"] {