From 6d4d3f11ff0c65f35c51f83709522829516ca322 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Fri, 27 Feb 2026 11:38:33 +0100 Subject: [PATCH 1/8] Skip recipe validation for imperative recipes in assertRecipesConfigure() Recipes with optional parameters and a custom validate() requiring at least one to be present fail assertRecipesConfigure() because they are loaded with no arguments, leaving all optional params null. Declarative recipes (from YAML) should still be validated since their options are explicitly configured. Imperative recipes loaded with no user-provided arguments are inherently unconfigured and validation failures are expected. Add a recipeValidation flag to RecipeSpec (following the existing serializationValidation pattern) and gate validateAll() on it. In assertRecipesConfigure(), disable validation for non-DeclarativeRecipe instances. Fixes #6840 --- .../java/org/openrewrite/test/RecipeSpec.java | 7 +++++ .../org/openrewrite/test/RewriteTest.java | 26 ++++++++++++++----- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/rewrite-test/src/main/java/org/openrewrite/test/RecipeSpec.java b/rewrite-test/src/main/java/org/openrewrite/test/RecipeSpec.java index cf434817b8..73517c7805 100644 --- a/rewrite-test/src/main/java/org/openrewrite/test/RecipeSpec.java +++ b/rewrite-test/src/main/java/org/openrewrite/test/RecipeSpec.java @@ -92,6 +92,8 @@ public static RecipeSpec defaults() { boolean serializationValidation = true; + boolean recipeValidation = true; + PrintOutputCapture.@Nullable MarkerPrinter markerPrinter; List>> beforeRecipes = new ArrayList<>(); @@ -258,6 +260,11 @@ public RecipeSpec validateRecipeSerialization(boolean validate) { return this; } + public RecipeSpec validateRecipe(boolean validate) { + this.recipeValidation = validate; + return this; + } + public RecipeSpec sourceSet(Function, LargeSourceSet> sourceSetBuilder) { this.sourceSet = sourceSetBuilder; return this; diff --git a/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java b/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java index f3c85b2d09..fd9be49134 100644 --- a/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java +++ b/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java @@ -19,6 +19,7 @@ import org.jspecify.annotations.Nullable; import org.openrewrite.*; import org.openrewrite.config.CompositeRecipe; +import org.openrewrite.config.DeclarativeRecipe; import org.openrewrite.config.Environment; import org.openrewrite.config.OptionDescriptor; import org.openrewrite.internal.*; @@ -90,10 +91,21 @@ default void assertRecipesConfigure(String packageName) { // scanRuntimeClasspath picks up all recipes in META-INF/rewrite regardless of whether their // names start with the package we intend to filter on here if (recipe.getName().startsWith(packageName)) { + // Imperative recipes are loaded with no user-provided arguments, so all optional + // parameters are null. Skip recipe validation for these since custom validate() + // methods (e.g. requiring at least one of several optional parameters) would + // fail on an unconfigured instance. Declarative recipes have their options + // configured from YAML and should still be validated. + boolean skipValidation = !(recipe instanceof DeclarativeRecipe); softly.assertThatCode(() -> { try { rewriteRun( - spec -> spec.recipe(recipe), + spec -> { + spec.recipe(recipe); + if (skipValidation) { + spec.validateRecipe(false); + } + }, new SourceSpecs[0] ); } catch (Throwable t) { @@ -226,11 +238,13 @@ default void rewriteRun(Consumer spec, SourceSpec... sourceSpecs) for (SourceSpec s : sourceSpecs) { s.customizeExecutionContext.accept(ctx); } - List> validations = new ArrayList<>(); - recipe.validateAll(ctx, validations); - assertThat(validations.stream().filter(Validated::isInvalid)) - .as("Recipe validation must have no failures") - .isEmpty(); + if (testClassSpec.recipeValidation && testMethodSpec.recipeValidation) { + List> validations = new ArrayList<>(); + recipe.validateAll(ctx, validations); + assertThat(validations.stream().filter(Validated::isInvalid)) + .as("Recipe validation must have no failures") + .isEmpty(); + } Map>> sourceSpecsByParser = new HashMap<>(); List methodSpecParsers = testMethodSpec.parsers; From 16f2126a86238935464b0c4e7eb33a349b19d206 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Fri, 27 Feb 2026 11:48:15 +0100 Subject: [PATCH 2/8] Add tests for validateRecipe flag with optional-OR validation Three tests demonstrate the intention: - rejectUnconfiguredRecipeWithOptionalOrValidation: recipe with two optional params and validate() requiring at least one fails when constructed with no args (both null) and default validation - acceptUnconfiguredRecipeWithOptionalOrValidationWhenSkipped: same recipe succeeds with validateRecipe(false), as assertRecipesConfigure now does for imperative recipes - rejectConfiguredRecipeWithOptionalOrValidationStillValidated: declarative recipes still get validated (broken YAML still caught) --- .../test/internal/RewriteTestTest.java | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java b/rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java index 814d42f597..55a6a2a9a7 100644 --- a/rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java +++ b/rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java @@ -16,6 +16,7 @@ package org.openrewrite.test.internal; import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.Value; @@ -24,6 +25,7 @@ import org.junit.jupiter.api.Test; import org.openrewrite.*; import org.openrewrite.test.RewriteTest; +import org.openrewrite.test.SourceSpecs; import org.openrewrite.test.TypeValidation; import org.openrewrite.text.PlainText; import org.openrewrite.text.PlainTextVisitor; @@ -152,6 +154,47 @@ void allowScannerEdit() { text("foo") ); } + + @Test + void rejectUnconfiguredRecipeWithOptionalOrValidation() { + // A recipe with optional params and validate() requiring at least one + // fails when loaded with no arguments and default validation is enabled + assertThrows(AssertionError.class, () -> + rewriteRun( + spec -> spec.recipe(new RecipeWithOptionalOrValidation(null, null)), + new SourceSpecs[0] + )); + } + + @Test + void acceptUnconfiguredRecipeWithOptionalOrValidationWhenSkipped() { + // The same recipe succeeds when validation is disabled, as + // assertRecipesConfigure() does for imperative recipes + rewriteRun( + spec -> spec + .recipe(new RecipeWithOptionalOrValidation(null, null)) + .validateRecipe(false), + new SourceSpecs[0] + ); + } + + @Test + void rejectConfiguredRecipeWithOptionalOrValidationStillValidated() { + // When the recipe IS configured (e.g. from YAML), validation should + // still catch real problems like referring to non-existent sub-recipes + assertThrows(AssertionError.class, () -> + rewriteRun( + spec -> spec.recipeFromYaml(""" + type: specs.openrewrite.org/v1beta/recipe + name: org.openrewrite.test.internal.StillValidated + displayName: Still validated + description: Declarative recipe with a non-existent sub-recipe should still fail validation. + recipeList: + - org.openrewrite.DoesNotExist + + """, "org.openrewrite.test.internal.StillValidated") + )); + } } @EqualsAndHashCode(callSuper = false) @@ -353,3 +396,47 @@ public Collection generate(AtomicBoolean acc, ExecutionCon .build()); } } + +@EqualsAndHashCode(callSuper = false) +class RecipeWithOptionalOrValidation extends Recipe { + + @Getter + final String displayName = "Recipe with optional OR validation"; + + @Getter + final String description = "Has two optional parameters where at least one must be set."; + + @Option(displayName = "Option A", + description = "First optional parameter.", + example = "valueA", + required = false) + @Nullable + final String optionA; + + @Option(displayName = "Option B", + description = "Second optional parameter.", + example = "valueB", + required = false) + @Nullable + final String optionB; + + @JsonCreator + RecipeWithOptionalOrValidation( + @JsonProperty("optionA") @Nullable String optionA, + @JsonProperty("optionB") @Nullable String optionB) { + this.optionA = optionA; + this.optionB = optionB; + } + + @Override + public Validated validate() { + return super.validate().and( + Validated.required("optionA", optionA) + .or(Validated.required("optionB", optionB))); + } + + @Override + public TreeVisitor getVisitor() { + return TreeVisitor.noop(); + } +} From ff78507cbea6954a25efdf14045bb53221445c51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Mon, 2 Mar 2026 12:16:40 +0100 Subject: [PATCH 3/8] Apply suggestion from Review Co-authored-by: Tim te Beek --- .../src/main/java/org/openrewrite/test/RewriteTest.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java b/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java index fd9be49134..e46878ea3f 100644 --- a/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java +++ b/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java @@ -100,12 +100,7 @@ default void assertRecipesConfigure(String packageName) { softly.assertThatCode(() -> { try { rewriteRun( - spec -> { - spec.recipe(recipe); - if (skipValidation) { - spec.validateRecipe(false); - } - }, + spec -> spec.recipe(recipe).validateRecipe(recipe instanceof DeclarativeRecipe), new SourceSpecs[0] ); } catch (Throwable t) { From 67fc0cbfe5ab4f0577e48eb899ead80b1405dd6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Mon, 2 Mar 2026 13:10:27 +0100 Subject: [PATCH 4/8] Remove unused skipValidation variable --- rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java b/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java index e46878ea3f..a3837fd99f 100644 --- a/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java +++ b/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java @@ -96,7 +96,6 @@ default void assertRecipesConfigure(String packageName) { // methods (e.g. requiring at least one of several optional parameters) would // fail on an unconfigured instance. Declarative recipes have their options // configured from YAML and should still be validated. - boolean skipValidation = !(recipe instanceof DeclarativeRecipe); softly.assertThatCode(() -> { try { rewriteRun( From 730f917be0950a0416f8310f7f59888dbad7ba03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Tue, 3 Mar 2026 11:51:48 +0100 Subject: [PATCH 5/8] minimize surface by branching in Recipe information --- .../main/java/org/openrewrite/test/RecipeSpec.java | 7 ------- .../java/org/openrewrite/test/RewriteTest.java | 14 +++++++------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/rewrite-test/src/main/java/org/openrewrite/test/RecipeSpec.java b/rewrite-test/src/main/java/org/openrewrite/test/RecipeSpec.java index 73517c7805..cf434817b8 100644 --- a/rewrite-test/src/main/java/org/openrewrite/test/RecipeSpec.java +++ b/rewrite-test/src/main/java/org/openrewrite/test/RecipeSpec.java @@ -92,8 +92,6 @@ public static RecipeSpec defaults() { boolean serializationValidation = true; - boolean recipeValidation = true; - PrintOutputCapture.@Nullable MarkerPrinter markerPrinter; List>> beforeRecipes = new ArrayList<>(); @@ -260,11 +258,6 @@ public RecipeSpec validateRecipeSerialization(boolean validate) { return this; } - public RecipeSpec validateRecipe(boolean validate) { - this.recipeValidation = validate; - return this; - } - public RecipeSpec sourceSet(Function, LargeSourceSet> sourceSetBuilder) { this.sourceSet = sourceSetBuilder; return this; diff --git a/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java b/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java index a3837fd99f..abf7125ed8 100644 --- a/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java +++ b/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java @@ -91,15 +91,10 @@ default void assertRecipesConfigure(String packageName) { // scanRuntimeClasspath picks up all recipes in META-INF/rewrite regardless of whether their // names start with the package we intend to filter on here if (recipe.getName().startsWith(packageName)) { - // Imperative recipes are loaded with no user-provided arguments, so all optional - // parameters are null. Skip recipe validation for these since custom validate() - // methods (e.g. requiring at least one of several optional parameters) would - // fail on an unconfigured instance. Declarative recipes have their options - // configured from YAML and should still be validated. softly.assertThatCode(() -> { try { rewriteRun( - spec -> spec.recipe(recipe).validateRecipe(recipe instanceof DeclarativeRecipe), + spec -> spec.recipe(recipe), new SourceSpecs[0] ); } catch (Throwable t) { @@ -232,7 +227,12 @@ default void rewriteRun(Consumer spec, SourceSpec... sourceSpecs) for (SourceSpec s : sourceSpecs) { s.customizeExecutionContext.accept(ctx); } - if (testClassSpec.recipeValidation && testMethodSpec.recipeValidation) { + + // Imperative recipes are loaded with no user-provided arguments, so all optional parameters are null. + // Skip recipe validation for these since custom validate() methods (e.g. requiring at least one of several + // optional parameters) would fail on an unconfigured instance. + if (recipe instanceof DeclarativeRecipe + || recipe.getDescriptor().getOptions().stream().anyMatch(opt -> !opt.isRequired())) { List> validations = new ArrayList<>(); recipe.validateAll(ctx, validations); assertThat(validations.stream().filter(Validated::isInvalid)) From 230891241dfece6bbd5d8d765cafbf1df0d732c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Tue, 3 Mar 2026 11:59:33 +0100 Subject: [PATCH 6/8] Fix validation condition and update test to match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The condition for skipping validation was inverted — it ran validation for recipes WITH optional params instead of skipping them. Fixed to allMatch(isRequired) so validation is only performed for declarative recipes or recipes with no optional parameters. Updated the test to verify that an unconfigured recipe with optional params and custom validate() now succeeds instead of failing. --- .../org/openrewrite/test/RewriteTest.java | 2 +- .../test/internal/RewriteTestTest.java | 20 ++++--------------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java b/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java index abf7125ed8..4908e10c35 100644 --- a/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java +++ b/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java @@ -232,7 +232,7 @@ default void rewriteRun(Consumer spec, SourceSpec... sourceSpecs) // Skip recipe validation for these since custom validate() methods (e.g. requiring at least one of several // optional parameters) would fail on an unconfigured instance. if (recipe instanceof DeclarativeRecipe - || recipe.getDescriptor().getOptions().stream().anyMatch(opt -> !opt.isRequired())) { + || recipe.getDescriptor().getOptions().stream().allMatch(OptionDescriptor::isRequired)) { List> validations = new ArrayList<>(); recipe.validateAll(ctx, validations); assertThat(validations.stream().filter(Validated::isInvalid)) diff --git a/rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java b/rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java index 55a6a2a9a7..cb32542fc4 100644 --- a/rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java +++ b/rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java @@ -156,24 +156,12 @@ void allowScannerEdit() { } @Test - void rejectUnconfiguredRecipeWithOptionalOrValidation() { + void acceptUnconfiguredRecipeWithOptionalOrValidation() { // A recipe with optional params and validate() requiring at least one - // fails when loaded with no arguments and default validation is enabled - assertThrows(AssertionError.class, () -> - rewriteRun( - spec -> spec.recipe(new RecipeWithOptionalOrValidation(null, null)), - new SourceSpecs[0] - )); - } - - @Test - void acceptUnconfiguredRecipeWithOptionalOrValidationWhenSkipped() { - // The same recipe succeeds when validation is disabled, as - // assertRecipesConfigure() does for imperative recipes + // should not fail when loaded with no arguments, because validation is + // skipped for imperative recipes that have optional parameters rewriteRun( - spec -> spec - .recipe(new RecipeWithOptionalOrValidation(null, null)) - .validateRecipe(false), + spec -> spec.recipe(new RecipeWithOptionalOrValidation(null, null)), new SourceSpecs[0] ); } From addbe2a3f4ca989c0861b2b611265c985e8bf864 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Tue, 3 Mar 2026 12:21:34 +0100 Subject: [PATCH 7/8] clear test and complete coverage --- .../test/internal/RewriteTestTest.java | 163 +++++++++++------- 1 file changed, 99 insertions(+), 64 deletions(-) diff --git a/rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java b/rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java index cb32542fc4..eb3ae7609d 100644 --- a/rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java +++ b/rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java @@ -16,12 +16,12 @@ package org.openrewrite.test.internal; import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonProperty; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.Value; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.openrewrite.*; import org.openrewrite.test.RewriteTest; @@ -37,6 +37,7 @@ import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.openrewrite.test.SourceSpecs.text; @@ -123,7 +124,7 @@ void rejectRecipeValidationFailure() { description: Deliberately has a non-existent recipe in its recipe list to trigger a validation failure. recipeList: - org.openrewrite.DoesNotExist - + """, "org.openrewrite.RefersToNonExistentRecipe") )); } @@ -155,33 +156,57 @@ void allowScannerEdit() { ); } - @Test - void acceptUnconfiguredRecipeWithOptionalOrValidation() { - // A recipe with optional params and validate() requiring at least one - // should not fail when loaded with no arguments, because validation is - // skipped for imperative recipes that have optional parameters - rewriteRun( - spec -> spec.recipe(new RecipeWithOptionalOrValidation(null, null)), - new SourceSpecs[0] - ); - } + @Nested + class RecipeValidation { - @Test - void rejectConfiguredRecipeWithOptionalOrValidationStillValidated() { - // When the recipe IS configured (e.g. from YAML), validation should - // still catch real problems like referring to non-existent sub-recipes - assertThrows(AssertionError.class, () -> - rewriteRun( - spec -> spec.recipeFromYaml(""" - type: specs.openrewrite.org/v1beta/recipe - name: org.openrewrite.test.internal.StillValidated - displayName: Still validated - description: Declarative recipe with a non-existent sub-recipe should still fail validation. - recipeList: - - org.openrewrite.DoesNotExist + @Test + void validateRecipeWithNoOptions() { + rewriteRun( + spec -> spec.recipe(new RecipeWithNoOptions()), + new SourceSpecs[0] + ); + } - """, "org.openrewrite.test.internal.StillValidated") - )); + @Test + void validateRecipeWithOnlyRequiredOptionsPositive() { + rewriteRun( + spec -> spec.recipe(new RecipeWithRequiredOptionValidateNoBlank("Test")), + new SourceSpecs[0] + ); + } + + @Test + void validateRecipeWithOnlyRequiredOptionsNegative() { + assertThatThrownBy(() -> + rewriteRun( + spec -> spec.recipe(new RecipeWithRequiredOptionValidateNoBlank("")), + new SourceSpecs[0] + ) + ); + } + + @Test + void skipValidationForRecipeWithOptionalOptions() { + rewriteRun( + spec -> spec.recipe(new RecipeWithOptionalOrValidation(null, null)), + new SourceSpecs[0] + ); + } + + @Test + void validateDeclarativeRecipe() { + assertThrows(AssertionError.class, () -> + rewriteRun( + spec -> spec.recipeFromYaml(""" + type: specs.openrewrite.org/v1beta/recipe + name: org.openrewrite.test.internal.StillValidated + displayName: Still validated + description: Declarative recipe with a non-existent sub-recipe should still fail validation. + recipeList: + - org.openrewrite.DoesNotExist + """, "org.openrewrite.test.internal.StillValidated") + )); + } } } @@ -261,7 +286,7 @@ class CreatesTwoFilesSamePath extends ScanningRecipe { String displayName = "Creates two source files with the same path"; String description = "A source file's path must be unique. " + - "This recipe creates two source files with the same path to show that the test framework helps protect against this mistake."; + "This recipe creates two source files with the same path to show that the test framework helps protect against this mistake."; @Override public AtomicBoolean getInitialValue(ExecutionContext ctx) { @@ -327,10 +352,10 @@ class RecipeWithDescriptionListOfLinks extends Recipe { @Getter final String description = """ - A fancy description. - For more information, see: - - [link 1](https://example.com/link1) - - [link 2](https://example.com/link2)"""; + A fancy description. + For more information, see: + - [link 1](https://example.com/link1) + - [link 2](https://example.com/link2)"""; } @NullMarked @@ -341,10 +366,10 @@ class RecipeWithDescriptionListOfDescribedLinks extends Recipe { @Getter final String description = """ - A fancy description. - For more information, see: - - First Resource [link 1](https://example.com/link1). - - Second Resource [link 2](https://example.com/link2)."""; + A fancy description. + For more information, see: + - First Resource [link 1](https://example.com/link1). + - Second Resource [link 2](https://example.com/link2)."""; } @NullMarked @@ -385,46 +410,56 @@ public Collection generate(AtomicBoolean acc, ExecutionCon } } +@Value @EqualsAndHashCode(callSuper = false) -class RecipeWithOptionalOrValidation extends Recipe { +class RecipeWithNoOptions extends Recipe { + String displayName = "Recipe with no options"; + String description = "Has no configurable options at all."; +} - @Getter - final String displayName = "Recipe with optional OR validation"; +@Value +@NullMarked +@EqualsAndHashCode(callSuper = false) +class RecipeWithRequiredOptionValidateNoBlank extends Recipe { + String displayName = "Recipe with required option"; + String description = "Has a single required parameter."; - @Getter - final String description = "Has two optional parameters where at least one must be set."; + @Option(displayName = "Required param", + description = "A required parameter.") + String requiredParam; + + @Override + public Validated validate() { + return super.validate() + .and(Validated.notBlank("Required param", requiredParam)); + } +} + +@Value +@NullMarked +@EqualsAndHashCode(callSuper = false) +class RecipeWithOptionalOrValidation extends Recipe { + String displayName = "Recipe with optional OR validation"; + String description = "Has two optional parameters where at least one must be set."; @Option(displayName = "Option A", - description = "First optional parameter.", - example = "valueA", - required = false) + description = "First optional parameter.", + example = "valueA", + required = false) @Nullable - final String optionA; + String optionA; @Option(displayName = "Option B", - description = "Second optional parameter.", - example = "valueB", - required = false) + description = "Second optional parameter.", + example = "valueB", + required = false) @Nullable - final String optionB; - - @JsonCreator - RecipeWithOptionalOrValidation( - @JsonProperty("optionA") @Nullable String optionA, - @JsonProperty("optionB") @Nullable String optionB) { - this.optionA = optionA; - this.optionB = optionB; - } + String optionB; @Override public Validated validate() { return super.validate().and( - Validated.required("optionA", optionA) - .or(Validated.required("optionB", optionB))); - } - - @Override - public TreeVisitor getVisitor() { - return TreeVisitor.noop(); + Validated.required("optionA", optionA) + .or(Validated.required("optionB", optionB))); } } From bcc994f2e1a4d87445268b6a05798df0fa0c892e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Wed, 4 Mar 2026 12:05:37 +0100 Subject: [PATCH 8/8] Test validation directly instead of expecting AssertionError from rewriteRun --- ...ngeDependencyGroupIdAndArtifactIdTest.java | 30 +++------- ...gedDependencyGroupIdAndArtifactIdTest.java | 15 ++--- .../org/openrewrite/yaml/MergeYamlTest.java | 60 +++++++++---------- 3 files changed, 40 insertions(+), 65 deletions(-) diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java index 35d5ce03ce..a6290d9cbc 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java @@ -19,11 +19,11 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.Issue; +import org.openrewrite.Validated; import org.openrewrite.test.RewriteTest; import org.openrewrite.test.SourceSpec; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.openrewrite.java.Assertions.mavenProject; import static org.openrewrite.maven.Assertions.pomXml; @@ -765,33 +765,17 @@ void changeProfileManagedDependencyGroupIdAndArtifactId() { @Issue("https://github.com/openrewrite/rewrite-java-dependencies/issues/55") @Test void requireNewGroupIdOrNewArtifactId() { - assertThatExceptionOfType(AssertionError.class) - .isThrownBy(() -> rewriteRun( - spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId( - "javax.activation", - "javax.activation-api", - null, - null, - null, - null - )) - )).withMessageContaining("newGroupId OR newArtifactId must be different from before"); + ChangeDependencyGroupIdAndArtifactId recipe = new ChangeDependencyGroupIdAndArtifactId("javax.activation", "javax.activation-api", null, null, null, null);; + assertThat(recipe.validate().failures()).extracting(Validated.Invalid::getMessage) + .contains("newGroupId OR newArtifactId must be different from before"); } @Issue("https://github.com/openrewrite/rewrite-java-dependencies/issues/55") @Test void requireNewGroupIdOrNewArtifactIdToBeDifferentFromBefore() { - assertThatExceptionOfType(AssertionError.class) - .isThrownBy(() -> rewriteRun( - spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId( - "javax.activation", - "javax.activation-api", - "javax.activation", - null, - null, - null - )) - )).withMessageContaining("newGroupId OR newArtifactId must be different from before"); + ChangeDependencyGroupIdAndArtifactId recipe = new ChangeDependencyGroupIdAndArtifactId("javax.activation", "javax.activation-api", "javax.activation", null, null, null); + assertThat(recipe.validate().failures()).extracting(Validated.Invalid::getMessage) + .contains("newGroupId OR newArtifactId must be different from before"); } @Test diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactIdTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactIdTest.java index a9eb5d0f45..83e696b9fa 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactIdTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactIdTest.java @@ -18,10 +18,10 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.Issue; +import org.openrewrite.Validated; import org.openrewrite.test.RewriteTest; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.openrewrite.maven.Assertions.pomXml; class ChangeManagedDependencyGroupIdAndArtifactIdTest implements RewriteTest { @@ -79,16 +79,9 @@ void changeManagedDependencyGroupIdAndArtifactId() { @Issue("https://github.com/openrewrite/rewrite-java-dependencies/issues/55") @Test void requireNewGroupIdOrNewArtifactIdToBeDifferentFromBefore() { - assertThatExceptionOfType(AssertionError.class) - .isThrownBy(() -> rewriteRun( - spec -> spec.recipe(new ChangeManagedDependencyGroupIdAndArtifactId( - "javax.activation", - "javax.activation-api", - "javax.activation", - "javax.activation-api", - null - )) - )).withMessageContaining("newGroupId OR newArtifactId must be different from before"); + ChangeManagedDependencyGroupIdAndArtifactId recipe = new ChangeManagedDependencyGroupIdAndArtifactId("javax.activation", "javax.activation-api", "javax.activation", "javax.activation-api", null); + assertThat(recipe.validate().failures()).extracting(Validated.Invalid::getMessage) + .contains("newGroupId OR newArtifactId must be different from before"); } @Test diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 1d8c919474..c8d2f9e7d0 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -18,9 +18,10 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.Issue; +import org.openrewrite.Validated; import org.openrewrite.test.RewriteTest; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.yaml.Assertions.yaml; import static org.openrewrite.yaml.MergeYaml.InsertMode.After; import static org.openrewrite.yaml.MergeYaml.InsertMode.Before; @@ -3059,22 +3060,21 @@ void preventKeysToBeAppendedToPreviousCommentIfManyLineBreaks() { @Test void invalidYaml() { - assertThrows(AssertionError.class, () -> rewriteRun( - spec -> spec - .recipe(new MergeYaml( - "$.some.object", - //language=text - """ - script: |-ParseError - """, - false, - "name", - null, - null, - null, - null - )) - )); + MergeYaml recipe = new MergeYaml( + "$.some.object", + //language=text + """ + script: |-ParseError + """, + false, + "name", + null, + null, + null, + null + ); + assertThat(recipe.validate().failures()).extracting(Validated.Invalid::getMessage) + .contains("Must be valid YAML"); } @Test @@ -3144,20 +3144,18 @@ void createNewKeysFalse() { @SuppressWarnings("DataFlowIssue") @Test void sourceNull() { - assertThrows(AssertionError.class, () -> - rewriteRun( - spec -> spec - .recipe(new MergeYaml( - "$.some.object", - null, - false, - "name", - null, - null, - null, - null - )) - )); + MergeYaml recipe = new MergeYaml( + "$.some.object", + null, + false, + "name", + null, + null, + null, + null + ); + assertThat(recipe.validate().failures()).extracting(Validated.Invalid::getMessage) + .contains("Must be valid YAML"); } @Test