From 47328ab76ae620c41f74301bace5d899d65dd229 Mon Sep 17 00:00:00 2001 From: Colin Harrington Date: Wed, 23 Nov 2011 10:46:51 -0600 Subject: [PATCH] GROOVY-5139: GroovyTestCase's shouldFail {...} should return the throwable and not the enclosed message --- src/main/groovy/util/GroovyTestCase.java | 27 +++++++++---------- src/test/groovy/beans/ListenerListASTTest.groovy | 8 +++--- src/test/groovy/bugs/Groovy4098Bug.groovy | 6 ++-- src/test/groovy/json/JsonLexerTest.groovy | 2 +- src/test/groovy/json/JsonSlurperTest.groovy | 2 +- src/test/groovy/swing/SwingBuilderTableTest.groovy | 8 +++--- src/test/groovy/swing/SwingBuilderTest.groovy | 2 +- .../transform/ConditionalInterruptTest.groovy | 4 +- .../groovy/transform/ThreadInterruptTest.groovy | 4 +- .../groovy/transform/TimedInterruptTest.groovy | 16 ++++++------ .../groovy/util/FactoryBuilderSupportTest.groovy | 4 +- src/test/groovy/util/GroovyTestCaseTest.groovy | 22 +++++++++++----- .../builder/AstBuilderFromSpecificationTest.groovy | 8 +++--- .../groovy/runtime/StringAsClassTest.groovy | 4 +- .../groovy/transform/CanonicalTransformTest.groovy | 6 ++-- .../groovy/transform/ImmutableTransformTest.groovy | 4 +- 16 files changed, 67 insertions(+), 60 deletions(-) diff --git a/src/main/groovy/util/GroovyTestCase.java b/src/main/groovy/util/GroovyTestCase.java index 74a2745..19ffcc8 100644 --- a/src/main/groovy/util/GroovyTestCase.java +++ b/src/main/groovy/util/GroovyTestCase.java @@ -213,25 +213,24 @@ public class GroovyTestCase extends TestCase { /** * Asserts that the given code closure fails when it is evaluated * - * @param code - * @return the message of the thrown Throwable + * @param code the closure that should fail + * @return the thrown Throwable */ - protected String shouldFail(Closure code) { - boolean failed = false; - String result = null; + protected Throwable shouldFail(Closure code) { + Throwable th = null; try { code.call(); } catch (GroovyRuntimeException gre) { - failed = true; - result = ScriptBytecodeAdapter.unwrap(gre).getMessage(); + th = ScriptBytecodeAdapter.unwrap(gre); } catch (Throwable e) { - failed = true; - result = e.getMessage(); + th = e; } - assertTrue("Closure " + code + " should have failed", failed); - return result; + if (th == null) { + fail("Closure " + code + " should have failed"); + } + return th; } /** @@ -240,9 +239,9 @@ public class GroovyTestCase extends TestCase { * * @param clazz the class of the expected exception * @param code the closure that should fail - * @return the message of the expected Throwable + * @return the expected Throwable */ - protected String shouldFail(Class clazz, Closure code) { + protected Throwable shouldFail(Class clazz, Closure code) { Throwable th = null; try { code.call(); @@ -257,7 +256,7 @@ public class GroovyTestCase extends TestCase { } else if (!clazz.isInstance(th)) { fail("Closure " + code + " should have failed with an exception of type " + clazz.getName() + ", instead got Exception " + th); } - return th.getMessage(); + return th; } /** diff --git a/src/test/groovy/beans/ListenerListASTTest.groovy b/src/test/groovy/beans/ListenerListASTTest.groovy index 7870cb2..044e010 100644 --- a/src/test/groovy/beans/ListenerListASTTest.groovy +++ b/src/test/groovy/beans/ListenerListASTTest.groovy @@ -133,7 +133,7 @@ class ListenerListASTTest extends GroovyTestCase { Vector listeners2 } """) - } + }?.message assert message.contains('Class b.TestClass already has method fireEventOccurred') } @@ -207,7 +207,7 @@ class ListenerListASTTest extends GroovyTestCase { def listeners } """) - } + }?.message assert message.contains('@groovy.beans.ListenerList can only annotate collection properties') } @@ -221,7 +221,7 @@ class ListenerListASTTest extends GroovyTestCase { List listeners } """) - } + }?.message assert message.contains('@groovy.beans.ListenerList fields must have a generic type') } @@ -235,7 +235,7 @@ class ListenerListASTTest extends GroovyTestCase { List listeners } """) - } + }?.message assert message.contains('@groovy.beans.ListenerList fields with generic wildcards not yet supported') } diff --git a/src/test/groovy/bugs/Groovy4098Bug.groovy b/src/test/groovy/bugs/Groovy4098Bug.groovy index 73dac7b..549eb64 100644 --- a/src/test/groovy/bugs/Groovy4098Bug.groovy +++ b/src/test/groovy/bugs/Groovy4098Bug.groovy @@ -87,7 +87,7 @@ class Groovy4098Bug extends GroovyTestCase { assert metaProperty.getProperty(this) == "five normal" def msg = shouldFail { metaProperty.setProperty(this, "five mop") - } + }?.message assert msg == "Cannot set read-only property: propertyFive" } @@ -98,7 +98,7 @@ class Groovy4098Bug extends GroovyTestCase { assert metaProperty.getProperty(this) == "six normal" def msg = shouldFail { metaProperty.setProperty(this, "six mop") - } + }?.message assert msg == "Cannot set the property 'propertySix' because the backing field is final." } @@ -143,4 +143,4 @@ class Groovy4098Bug extends GroovyTestCase { assert metaProperty.getProperty(p) == "four mop" } -} \ No newline at end of file +} diff --git a/src/test/groovy/json/JsonLexerTest.groovy b/src/test/groovy/json/JsonLexerTest.groovy index bc75854..d7413b1 100644 --- a/src/test/groovy/json/JsonLexerTest.groovy +++ b/src/test/groovy/json/JsonLexerTest.groovy @@ -83,7 +83,7 @@ class JsonLexerTest extends GroovyTestCase { def msg = shouldFail(JsonException) { lexer.nextToken() - } + }?.message assert msg.contains("trua") assert msg.contains("constant 'true'") diff --git a/src/test/groovy/json/JsonSlurperTest.groovy b/src/test/groovy/json/JsonSlurperTest.groovy index 2c1d8b7..127e6b7 100644 --- a/src/test/groovy/json/JsonSlurperTest.groovy +++ b/src/test/groovy/json/JsonSlurperTest.groovy @@ -25,7 +25,7 @@ class JsonSlurperTest extends GroovyTestCase { void testJsonShouldStartWithCurlyOrBracket() { def msg = shouldFail(JsonException) { parser.parseText("true") - } + }?.message assert msg.contains('A JSON payload should start with') } diff --git a/src/test/groovy/swing/SwingBuilderTableTest.groovy b/src/test/groovy/swing/SwingBuilderTableTest.groovy index 36a9be8..78ceae9 100644 --- a/src/test/groovy/swing/SwingBuilderTableTest.groovy +++ b/src/test/groovy/swing/SwingBuilderTableTest.groovy @@ -39,7 +39,7 @@ class SwingBuilderTableTest extends GroovySwingTestCase { def swing = new SwingBuilder() def msg = shouldFail { swing.propertyColumn() - } + }?.message assert msg.contains('propertyColumn must be a child of a tableModel') msg = shouldFail { swing.table { @@ -47,7 +47,7 @@ class SwingBuilderTableTest extends GroovySwingTestCase { propertyColumn() } } - } + }?.message assert msg.contains("Must specify a property for a propertyColumn"): \ "Instead found message: " + msg swing.table { @@ -76,7 +76,7 @@ class SwingBuilderTableTest extends GroovySwingTestCase { def swing = new SwingBuilder() def msg = shouldFail { swing.closureColumn() - } + }?.message assert msg.contains('closureColumn must be a child of a tableModel') msg = shouldFail { swing.table { @@ -84,7 +84,7 @@ class SwingBuilderTableTest extends GroovySwingTestCase { closureColumn() } } - } + }?.message assert msg.contains("Must specify 'read' Closure property for a closureColumn"): \ "Instead found message: " + msg def closure = {x -> x } diff --git a/src/test/groovy/swing/SwingBuilderTest.groovy b/src/test/groovy/swing/SwingBuilderTest.groovy index 3a29e68..1e92c51 100644 --- a/src/test/groovy/swing/SwingBuilderTest.groovy +++ b/src/test/groovy/swing/SwingBuilderTest.groovy @@ -628,7 +628,7 @@ class SwingBuilderTest extends GroovySwingTestCase { def swing = new SwingBuilder() def message = shouldFail{ swing.boxLayout() - } + }?.message assert message.contains('Must be nested inside a Container') // default is X_AXIS swing.panel(id:'panel'){ diff --git a/src/test/groovy/transform/ConditionalInterruptTest.groovy b/src/test/groovy/transform/ConditionalInterruptTest.groovy index 1d16053..5d28111 100644 --- a/src/test/groovy/transform/ConditionalInterruptTest.groovy +++ b/src/test/groovy/transform/ConditionalInterruptTest.groovy @@ -34,7 +34,7 @@ class ConditionalInterruptTest extends GroovyTestCase { def instance = c.newInstance() def message = shouldFail(InterruptedException) { instance.myMethod() - } + }?.message assert message == 'Execution interrupted. The following condition failed: { visited = true }' assert instance.visited } @@ -53,7 +53,7 @@ class ConditionalInterruptTest extends GroovyTestCase { def instance = c.newInstance() def message = shouldFail(CustomException) { instance.myMethod() - } + }?.message assert message == 'Execution interrupted. The following condition failed: { visited = true }' assert instance.visited } diff --git a/src/test/groovy/transform/ThreadInterruptTest.groovy b/src/test/groovy/transform/ThreadInterruptTest.groovy index fe0790f..0bde466 100644 --- a/src/test/groovy/transform/ThreadInterruptTest.groovy +++ b/src/test/groovy/transform/ThreadInterruptTest.groovy @@ -128,7 +128,7 @@ class ThreadInterruptTest extends GroovyTestCase { def mocker = new StubFor(Thread.class) mocker.demand.currentThread(1..Integer.MAX_VALUE) { new InterruptingThread() } mocker.use { - def message = shouldFail(InterruptedException) { c.newInstance().myMethod() } + def message = shouldFail(InterruptedException) { c.newInstance().myMethod() }?.message assert message == 'Execution interrupted. The current thread has been interrupted.' } } @@ -362,4 +362,4 @@ class CustomException extends Exception { public CustomException(final String message) { super(message) } -} \ No newline at end of file +} diff --git a/src/test/groovy/transform/TimedInterruptTest.groovy b/src/test/groovy/transform/TimedInterruptTest.groovy index 1e974a3..4dc04dc 100644 --- a/src/test/groovy/transform/TimedInterruptTest.groovy +++ b/src/test/groovy/transform/TimedInterruptTest.groovy @@ -43,7 +43,7 @@ class TimedInterruptTest extends GroovyTestCase { system.use { def e = shouldFail(TimeoutException) { instance.myMethod() - } + }?.message assert e.contains('Execution timed out after 1 units') } } @@ -81,7 +81,7 @@ class TimedInterruptTest extends GroovyTestCase { system.use { def e = shouldFail(CustomException) { instance.myMethod() - } + }?.message assert e.contains('Execution timed out after 1 units') } } @@ -118,7 +118,7 @@ class TimedInterruptTest extends GroovyTestCase { system.use { def e = shouldFail(TimeoutException) { instance.myMethod() - } + }?.message assert e.contains('Execution timed out after 1 units') } } @@ -186,7 +186,7 @@ class TimedInterruptTest extends GroovyTestCase { system.use { def e = shouldFail(TimeoutException) { instance.myMethod() - } + }?.message assert e.contains('Execution timed out after 1 units') } } @@ -225,7 +225,7 @@ class TimedInterruptTest extends GroovyTestCase { system.use { def e = shouldFail(TimeoutException) { instance.run() - } + }?.message assert e.contains('Execution timed out after 1 units') } } @@ -262,7 +262,7 @@ class TimedInterruptTest extends GroovyTestCase { system.use { def e = shouldFail(TimeoutException) { instance.run() - } + }?.message assert e.contains('Execution timed out after 1 units') } } @@ -299,7 +299,7 @@ class TimedInterruptTest extends GroovyTestCase { system.use { def e = shouldFail(TimeoutException) { instance.run() - } + }?.message assert e.contains('Execution timed out after 1 units') } } @@ -364,7 +364,7 @@ class TimedInterruptTest extends GroovyTestCase { system.use { def e = shouldFail(TimeoutException) { instance.myMethod() - } + }?.message assert e.contains('Execution timed out after 18000000 units') } } diff --git a/src/test/groovy/util/FactoryBuilderSupportTest.groovy b/src/test/groovy/util/FactoryBuilderSupportTest.groovy index 775be6b..b18f78f 100644 --- a/src/test/groovy/util/FactoryBuilderSupportTest.groovy +++ b/src/test/groovy/util/FactoryBuilderSupportTest.groovy @@ -747,7 +747,7 @@ class FactoryBuilderSupportTest extends GroovyTestCase { void testErrorMessage_checkValueIsType() { def msg = shouldFail(Exception) { FactoryBuilderSupport.checkValueIsType('message', 'prop', Integer) - } + }?.message assert msg.contains('prop') assert msg.contains('Integer') assert msg.contains('String') @@ -756,7 +756,7 @@ class FactoryBuilderSupportTest extends GroovyTestCase { void testErrorMessage_checkValueIsTypeNotString() { def msg = shouldFail(Exception) { FactoryBuilderSupport.checkValueIsTypeNotString(123G, 'prop', Integer) - } + }?.message assert msg.contains('prop') assert msg.contains('Integer') assert msg.contains('String') diff --git a/src/test/groovy/util/GroovyTestCaseTest.groovy b/src/test/groovy/util/GroovyTestCaseTest.groovy index d3c9a77..746489a 100644 --- a/src/test/groovy/util/GroovyTestCaseTest.groovy +++ b/src/test/groovy/util/GroovyTestCaseTest.groovy @@ -30,14 +30,13 @@ class GroovyTestCaseTest extends GroovyTestCase { // ---------------- - void testShouldFailWithMessage() { - def msg = shouldFail { throw new RuntimeException('x') } - assertEquals 'x', msg + void testShouldFailReturnsException() { + def throwable = shouldFail { throw new MyException(new NullPointerException()) } + assertEquals MyException, throwable?.class } - void testShouldFailWithMessageForClass() { - def msg = shouldFail(RuntimeException.class) { throw new RuntimeException('x') } - println msg - assertEquals 'x', msg + void testShouldFailForClassReturnsException() { + def throwable = shouldFail(RuntimeException.class) { throw new MyException(new RuntimeException('x')) } + assertEquals MyException, throwable?.class } void testShouldFail() { @@ -51,6 +50,15 @@ class GroovyTestCaseTest extends GroovyTestCase { new Foo().createBarWithNestedException() } } + + void testShouldFailOnUnexpectedExceptionShouldFail() { + shouldFail { + shouldFail(MyException) { + throw new NullPointerException() + } + } + } + } class Foo { diff --git a/src/test/org/codehaus/groovy/ast/builder/AstBuilderFromSpecificationTest.groovy b/src/test/org/codehaus/groovy/ast/builder/AstBuilderFromSpecificationTest.groovy index 541dbcf..41574ab 100644 --- a/src/test/org/codehaus/groovy/ast/builder/AstBuilderFromSpecificationTest.groovy +++ b/src/test/org/codehaus/groovy/ast/builder/AstBuilderFromSpecificationTest.groovy @@ -47,7 +47,7 @@ public class AstBuilderFromSpecificationTest extends GroovyTestCase { constant "this method" } } - } + }?.message assertTrue("Unhelpful error message: $message", message.contains('methodCall could not be invoked')) assertTrue("Unhelpful error message: $message", message.contains('Expected to receive')) assertTrue("Unhelpful error message: $message", message.contains('but found')) @@ -65,7 +65,7 @@ public class AstBuilderFromSpecificationTest extends GroovyTestCase { empty() } } - } + }?.message assertTrue("Unhelpful error message: $message", message.contains('methodCall could not be invoked')) assertTrue("Unhelpful error message: $message", message.contains('Expected to receive')) assertTrue("Unhelpful error message: $message", message.contains('but found')) @@ -1762,7 +1762,7 @@ public class AstBuilderFromSpecificationTest extends GroovyTestCase { constant "illegal value" } } - } + }?.message assertEquals("Wrong exception message", "methodCall could not be invoked. Expected to receive parameters [class org.codehaus.groovy.ast.expr.Expression, class org.codehaus.groovy.ast.expr.Expression, class org.codehaus.groovy.ast.expr.Expression] but found [class org.codehaus.groovy.ast.expr.VariableExpression, class org.codehaus.groovy.ast.expr.ConstantExpression, class org.codehaus.groovy.ast.expr.ArgumentListExpression, class org.codehaus.groovy.ast.expr.ConstantExpression]", msg) @@ -1778,7 +1778,7 @@ public class AstBuilderFromSpecificationTest extends GroovyTestCase { // missing argument list! } } - } + }?.message assertEquals("Wrong exception message", "methodCall could not be invoked. Expected to receive parameters [class org.codehaus.groovy.ast.expr.Expression, class org.codehaus.groovy.ast.expr.Expression, class org.codehaus.groovy.ast.expr.Expression] but found [class org.codehaus.groovy.ast.expr.VariableExpression, class org.codehaus.groovy.ast.expr.ConstantExpression]", msg) diff --git a/src/test/org/codehaus/groovy/runtime/StringAsClassTest.groovy b/src/test/org/codehaus/groovy/runtime/StringAsClassTest.groovy index cc9a5f9..0730d3b 100644 --- a/src/test/org/codehaus/groovy/runtime/StringAsClassTest.groovy +++ b/src/test/org/codehaus/groovy/runtime/StringAsClassTest.groovy @@ -27,7 +27,7 @@ class StringAsClassTest extends GroovyTestCase{ void testFails () { def message = shouldFail { assertEquals "NOSUCHCLASS" as Class, ArrayList - } + }?.message assert message.contains('java.lang.ClassNotFoundException: NOSUCHCLASS') } -} \ No newline at end of file +} diff --git a/src/test/org/codehaus/groovy/transform/CanonicalTransformTest.groovy b/src/test/org/codehaus/groovy/transform/CanonicalTransformTest.groovy index a56282f..5aef547 100644 --- a/src/test/org/codehaus/groovy/transform/CanonicalTransformTest.groovy +++ b/src/test/org/codehaus/groovy/transform/CanonicalTransformTest.groovy @@ -45,7 +45,7 @@ class CanonicalTransformTest extends GroovyShellTestCase { String bar } """ - } + }?.message assert msg.contains("@Canonical class 'Foo' can't also be @Immutable") } @@ -72,7 +72,7 @@ class CanonicalTransformTest extends GroovyShellTestCase { // Fail here new Foo('a', 'b', 'c') """ - } + }?.message assert msg.contains('Could not find matching constructor') } @@ -359,7 +359,7 @@ class CanonicalTransformTest extends GroovyShellTestCase { @groovy.transform.Canonical class Simple { } new Simple(missing:'Name') """ - } + }?.message assert msg.contains('No such property: missing for class: Simple') } diff --git a/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy b/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy index d3aca51..e229be9 100644 --- a/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy +++ b/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy @@ -84,7 +84,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { String bar } """ - } + }?.message assert msg.contains("@Canonical class 'Foo' can't also be @Immutable") } @@ -146,7 +146,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { @Immutable class Simple { } new Simple(missing:'Name') """ - } + }?.message assert msg.contains('No such property: missing for class: Simple') } -- 1.7.1