From e34c3e8c4f4f4e0ef73a1b4b6a1bcfa51eedf0f6 Mon Sep 17 00:00:00 2001 From: Daniel Chao Date: Tue, 19 May 2026 11:32:51 -0700 Subject: [PATCH] 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. --- docs/modules/pkl-gradle/pages/index.adoc | 4 +- .../org/pkl/commons/cli/CliTestOptions.kt | 2 +- .../org/pkl/commons/cli/TestReporter.kt | 6 ++- .../pkl/commons/cli/commands/TestOptions.kt | 2 +- .../main/java/org/pkl/gradle/PklPlugin.java | 4 +- .../pkl/gradle/spec/ProjectPackageSpec.java | 2 +- .../pkl/gradle/task/ProjectPackageTask.java | 16 +------- .../java/org/pkl/gradle/task/TestTask.java | 18 +-------- .../org/pkl/gradle/utils/PluginUtils.java | 28 ++++++++++++++ .../test/kotlin/org/pkl/gradle/TestsTest.kt | 37 +++++++++++++++++++ 10 files changed, 80 insertions(+), 39 deletions(-) diff --git a/docs/modules/pkl-gradle/pages/index.adoc b/docs/modules/pkl-gradle/pages/index.adoc index e05a6c24..adbfa87b 100644 --- a/docs/modules/pkl-gradle/pages/index.adoc +++ b/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 +.testReporter: Property [%collapsible] ==== Default: `"spec"` + @@ -686,7 +686,7 @@ Default: `false` + Whether to ignore expected example files and generate them again. ==== -.test-reporter: Property +.testReporter: Property [%collapsible] ==== Default: `"spec"` + diff --git a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliTestOptions.kt b/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliTestOptions.kt index c79a0b1f..0aac25f7 100644 --- a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliTestOptions.kt +++ b/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, ) diff --git a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/TestReporter.kt b/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/TestReporter.kt index 54d4534d..6986b3d9 100644 --- a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/TestReporter.kt +++ b/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 + } } diff --git a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/TestOptions.kt b/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/TestOptions.kt index 2de7df94..02c06852 100644 --- a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/TestOptions.kt +++ b/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 { it.name.lowercase() } .single() - .default(TestReporter.SPEC) + .default(TestReporter.default) val cliTestOptions: CliTestOptions by lazy { CliTestOptions( diff --git a/pkl-gradle/src/main/java/org/pkl/gradle/PklPlugin.java b/pkl-gradle/src/main/java/org/pkl/gradle/PklPlugin.java index 10091ae2..ef9b347f 100644 --- a/pkl-gradle/src/main/java/org/pkl/gradle/PklPlugin.java +++ b/pkl-gradle/src/main/java/org/pkl/gradle/PklPlugin.java @@ -100,7 +100,6 @@ public class PklPlugin implements Plugin { 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 @@ public class PklPlugin implements Plugin { 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 @@ public class PklPlugin implements Plugin { configureBaseSpec(project, spec); spec.getOverwrite().convention(false); - spec.getTestReporter().convention("spec"); var testTask = createModulesTask(project, TestTask.class, spec); testTask.configure( diff --git a/pkl-gradle/src/main/java/org/pkl/gradle/spec/ProjectPackageSpec.java b/pkl-gradle/src/main/java/org/pkl/gradle/spec/ProjectPackageSpec.java index 42d4d261..d4b05562 100644 --- a/pkl-gradle/src/main/java/org/pkl/gradle/spec/ProjectPackageSpec.java +++ b/pkl-gradle/src/main/java/org/pkl/gradle/spec/ProjectPackageSpec.java @@ -30,5 +30,5 @@ public interface ProjectPackageSpec extends BasePklSpec { Property getSkipPublishCheck(); - Property getReporter(); + Property getTestReporter(); } diff --git a/pkl-gradle/src/main/java/org/pkl/gradle/task/ProjectPackageTask.java b/pkl-gradle/src/main/java/org/pkl/gradle/task/ProjectPackageTask.java index 5046fec3..fcee3ed9 100644 --- a/pkl-gradle/src/main/java/org/pkl/gradle/task/ProjectPackageTask.java +++ b/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.PathSensitivity; 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 @@ public abstract class ProjectPackageTask extends BasePklTask { 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 @@ public abstract class ProjectPackageTask extends BasePklTask { getOverwrite().get(), getJunitAggregateReports().getOrElse(false), getJunitAggregateSuiteName().get(), - testReporter), + toTestReporter(getTestReporter())), getOutputPath().get().getAsFile().getAbsolutePath(), getSkipPublishCheck().getOrElse(false), new PrintWriter(System.out), diff --git a/pkl-gradle/src/main/java/org/pkl/gradle/task/TestTask.java b/pkl-gradle/src/main/java/org/pkl/gradle/task/TestTask.java index d562806b..78801ad6 100644 --- a/pkl-gradle/src/main/java/org/pkl/gradle/task/TestTask.java +++ b/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.Optional; 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,18 +55,6 @@ public abstract class TestTask extends ModulesTask { @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( @@ -76,7 +62,7 @@ public abstract class TestTask extends ModulesTask { getOverwrite().get(), getJunitAggregateReports().getOrElse(false), getJunitAggregateSuiteName().get(), - testReporter), + toTestReporter(getTestReporter())), new PrintWriter(System.out), new PrintWriter(System.err)) .run(); diff --git a/pkl-gradle/src/main/java/org/pkl/gradle/utils/PluginUtils.java b/pkl-gradle/src/main/java/org/pkl/gradle/utils/PluginUtils.java index 0c9eac65..f9cb1914 100644 --- a/pkl-gradle/src/main/java/org/pkl/gradle/utils/PluginUtils.java +++ b/pkl-gradle/src/main/java/org/pkl/gradle/utils/PluginUtils.java @@ -31,6 +31,7 @@ import org.gradle.api.file.FileSystemLocation; 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 final class PluginUtils { @Nullable T value = provider.getOrNull(); return value == null ? null : f.apply(value); } + + public static TestReporter toTestReporter(Provider 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()); + } + } } diff --git a/pkl-gradle/src/test/kotlin/org/pkl/gradle/TestsTest.kt b/pkl-gradle/src/test/kotlin/org/pkl/gradle/TestsTest.kt index 50538a1f..f721c14b 100644 --- a/pkl-gradle/src/test/kotlin/org/pkl/gradle/TestsTest.kt +++ b/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"] {