diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc index 6867c79fd4..0ffd10e6f4 100644 --- a/docs/release_notes.adoc +++ b/docs/release_notes.adoc @@ -6,6 +6,27 @@ include::include.adoc[] == 2.5 (tbd) +=== Breaking Changes + +* It is no longer true that only top-level expression statements are treated as implicit conditions. +Just like within `with { ... }`, `verifyAll { ... }`, `@ConditionBlock` methods, and other places, +each expression statement is now considered an implicit condition, except for statements in unknown +closures. ++ +This enables the usage of `if` and similar while also being able to still safely use `.every { ... }`, +but without the regular confusion why a test is not failing if the condition is inside an `if`. +You can even have interactions in those nested places now. ++ +As now expression statements are considered implicit conditions that previously were not, +this can lead to compilation errors in existing code-bases, because such nested statements were +previously simply ignored by the Spock compiler and now also must be valid conditions. ++ +If those compile errors are not accidentally written assignments that should have been conditions +or similar, you can for example use the <> +to declare an expression statement explicitly as not being an implicit condition. ++ +spockPull:2250[] + === Enhancements * Improve `TooManyInvocationsError` now reports unsatisfied interactions with argument mismatch details, making it easier to diagnose why invocations didn't match expected interactions spockPull:2315[] diff --git a/docs/spock_primer.adoc b/docs/spock_primer.adoc index bd4e4c234f..fb87f9f622 100644 --- a/docs/spock_primer.adoc +++ b/docs/spock_primer.adoc @@ -230,8 +230,9 @@ digestible form. Nice, isn't it? ===== Implicit and explicit conditions Conditions are an essential ingredient of `then` blocks and `expect` blocks. Except for calls to `void` methods and -expressions classified as interactions, all top-level expressions in these blocks are implicitly treated as conditions. -To use conditions in other places, you need to designate them with Groovy's assert keyword: +expressions classified as interactions, all expression statements in these blocks are implicitly treated as conditions +except if they happen inside some unknown closure. +To use conditions in other places, you need to designate them with Groovy's `assert` keyword: [source,groovy] ---- diff --git a/spock-core/src/main/java/org/spockframework/compiler/AbstractDeepBlockRewriter.java b/spock-core/src/main/java/org/spockframework/compiler/AbstractDeepBlockRewriter.java index a1db3340f1..5e5f2797cf 100644 --- a/spock-core/src/main/java/org/spockframework/compiler/AbstractDeepBlockRewriter.java +++ b/spock-core/src/main/java/org/spockframework/compiler/AbstractDeepBlockRewriter.java @@ -151,7 +151,7 @@ public final void visitClosureExpression(ClosureExpression expr) { groupConditionFound = false; ISpecialMethodCall oldSpecialMethodCall = currSpecialMethodCall; if (!currSpecialMethodCall.isMatch(expr)) { - currSpecialMethodCall = NoSpecialMethodCall.INSTANCE; // unrelated closure terminates currSpecialMethodCall scope + currSpecialMethodCall = NoSpecialMethodCall.CLOSURE_INSTANCE; // unrelated closure terminates currSpecialMethodCall scope } try { Statement code = expr.getCode(); diff --git a/spock-core/src/main/java/org/spockframework/compiler/DeepBlockRewriter.java b/spock-core/src/main/java/org/spockframework/compiler/DeepBlockRewriter.java index 3bbe618b30..124c75bb26 100644 --- a/spock-core/src/main/java/org/spockframework/compiler/DeepBlockRewriter.java +++ b/spock-core/src/main/java/org/spockframework/compiler/DeepBlockRewriter.java @@ -201,7 +201,7 @@ private boolean handleInteraction(InteractionRewriter rewriter, ExpressionStatem } private boolean handleImplicitCondition(ExpressionStatement stat) { - if (!(stat == currTopLevelStat && isThenOrExpectOrFilterBlock() + if (!(((currSpecialMethodCall == NoSpecialMethodCall.INSTANCE) && isThenOrExpectOrFilterBlock()) || currSpecialMethodCall.isConditionMethodCall() || currSpecialMethodCall.isConditionBlock() || currSpecialMethodCall.isGroupConditionBlock() diff --git a/spock-core/src/main/java/org/spockframework/compiler/NoSpecialMethodCall.java b/spock-core/src/main/java/org/spockframework/compiler/NoSpecialMethodCall.java index 0e89c842bf..789d4dacaa 100644 --- a/spock-core/src/main/java/org/spockframework/compiler/NoSpecialMethodCall.java +++ b/spock-core/src/main/java/org/spockframework/compiler/NoSpecialMethodCall.java @@ -23,6 +23,7 @@ public class NoSpecialMethodCall implements ISpecialMethodCall { public static final ISpecialMethodCall INSTANCE = new NoSpecialMethodCall(); + public static final ISpecialMethodCall CLOSURE_INSTANCE = new NoSpecialMethodCall(); @Override public boolean isMethodName(String name) { diff --git a/spock-junit4/src/test/groovy/org/spockframework/junit4/junit/JUnitFixtureMethods.groovy b/spock-junit4/src/test/groovy/org/spockframework/junit4/junit/JUnitFixtureMethods.groovy index 717a505744..6059a912ce 100644 --- a/spock-junit4/src/test/groovy/org/spockframework/junit4/junit/JUnitFixtureMethods.groovy +++ b/spock-junit4/src/test/groovy/org/spockframework/junit4/junit/JUnitFixtureMethods.groovy @@ -151,7 +151,7 @@ class JUnitFixtureMethods extends JUnitBaseSpec { then: def e = result.failures[exceptionPos].exception if (suppressed) { - e = e.suppressed[0] + !!(e = e.suppressed[0]) } e instanceof RuntimeException e.message == name diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/condition/ConditionEvaluation.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/condition/ConditionEvaluation.groovy index 66c100b5f5..960832578d 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/condition/ConditionEvaluation.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/condition/ConditionEvaluation.groovy @@ -59,6 +59,19 @@ class ConditionEvaluation extends EmbeddedSpecification { 7 } + @FailsWith(ConditionNotSatisfiedError) + def "failing non-top-level condition"() { + expect: + if (true) { + 2 * 3 == 7 + } + } + + def "failing non-top-level non-condition"() { + expect: + [1, 2, 3].any { it == 2 } + } + def "MethodCallExpression"() { expect: [1, 2, 3].size() == 3 diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/extension/TempDirExtensionSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/extension/TempDirExtensionSpec.groovy index 33d1bd5fb2..b2698a51b1 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/extension/TempDirExtensionSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/extension/TempDirExtensionSpec.groovy @@ -88,12 +88,12 @@ class TempDirExtensionSpec extends EmbeddedSpecification { def tempFile = aaabbb.resolve("tmp.txt") Files.createDirectories(aaabbb) Files.write(tempFile, "ewfwf".bytes) - aaabbb.toFile().writable = false - aaa.toFile().writable = false - tempFile.toFile().writable = false - previousIteration = iterationDir + !!(aaabbb.toFile().writable = false) + !!(aaa.toFile().writable = false) + !!(tempFile.toFile().writable = false) + !!(previousIteration = iterationDir) } else if (i == 1) { - assert !previousIteration.exists() + !previousIteration.exists() } where: diff --git a/spock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/condition/VerifyAllMethodsAstSpec/transforms_conditions_in_private_methods.groovy b/spock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/condition/VerifyAllMethodsAstSpec/transforms_conditions_in_private_methods.groovy index 929c588668..86e8f1f597 100644 --- a/spock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/condition/VerifyAllMethodsAstSpec/transforms_conditions_in_private_methods.groovy +++ b/spock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/condition/VerifyAllMethodsAstSpec/transforms_conditions_in_private_methods.groovy @@ -84,9 +84,15 @@ public void $spock_feature_0_0() { finally { } if (true) { - [true, false].any({ -> - it - }) + try { + org.spockframework.runtime.SpockRuntime.verifyMethodCondition($spock_errorCollector, $spock_valueRecorder.reset(), '[true, false].any { it }', 5, 9, null, $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(2), [$spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(0), true), $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(1), false)]), $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(3), 'any'), new java.lang.Object[]{$spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(4), { -> + it + })}, $spock_valueRecorder.realizeNas(7, false), false, 6) + } + catch (java.lang.Throwable $spock_condition_throwable) { + org.spockframework.runtime.SpockRuntime.conditionFailedWithException($spock_errorCollector, $spock_valueRecorder, '[true, false].any { it }', 5, 9, null, $spock_condition_throwable)} + finally { + } } this.with({ -> org.spockframework.runtime.ValueRecorder $spock_valueRecorder1 = new org.spockframework.runtime.ValueRecorder() diff --git a/spock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/condition/VerifyMethodsAstSpec/transforms_conditions_in_private_methods.groovy b/spock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/condition/VerifyMethodsAstSpec/transforms_conditions_in_private_methods.groovy index ff11d7b6a3..fbacf52038 100644 --- a/spock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/condition/VerifyMethodsAstSpec/transforms_conditions_in_private_methods.groovy +++ b/spock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/condition/VerifyMethodsAstSpec/transforms_conditions_in_private_methods.groovy @@ -80,9 +80,15 @@ public void $spock_feature_0_0() { finally { } if (true) { - [true, false].any({ -> - it - }) + try { + org.spockframework.runtime.SpockRuntime.verifyMethodCondition($spock_errorCollector, $spock_valueRecorder.reset(), '[true, false].any { it }', 5, 9, null, $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(2), [$spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(0), true), $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(1), false)]), $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(3), 'any'), new java.lang.Object[]{$spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(4), { -> + it + })}, $spock_valueRecorder.realizeNas(7, false), false, 6) + } + catch (java.lang.Throwable $spock_condition_throwable) { + org.spockframework.runtime.SpockRuntime.conditionFailedWithException($spock_errorCollector, $spock_valueRecorder, '[true, false].any { it }', 5, 9, null, $spock_condition_throwable)} + finally { + } } this.with({ -> org.spockframework.runtime.ValueRecorder $spock_valueRecorder1 = new org.spockframework.runtime.ValueRecorder()