You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/04/04 04:35:15 UTC

[groovy] branch master updated: GROOVY-9492: Relax groovy.test.NotYetImplemented dependency on JUnit 4's AssertionFailedError

This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new e9e2cec  GROOVY-9492: Relax groovy.test.NotYetImplemented dependency on JUnit 4's AssertionFailedError
e9e2cec is described below

commit e9e2cec6af194cc1cec5541799d3d0dedae92964
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Mar 31 13:02:49 2020 +1000

    GROOVY-9492: Relax groovy.test.NotYetImplemented dependency on JUnit 4's AssertionFailedError
---
 .../main/java/groovy/test/NotYetImplemented.java   | 15 +++++++++----
 .../NotYetImplementedASTTransformation.java        | 26 +++++++++++++---------
 .../NotYetImplementedTransformTest.groovy          | 25 ++++++++++-----------
 3 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/subprojects/groovy-test/src/main/java/groovy/test/NotYetImplemented.java b/subprojects/groovy-test/src/main/java/groovy/test/NotYetImplemented.java
index ad21c52..712b852 100644
--- a/subprojects/groovy-test/src/main/java/groovy/test/NotYetImplemented.java
+++ b/subprojects/groovy-test/src/main/java/groovy/test/NotYetImplemented.java
@@ -26,14 +26,14 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 /**
- * Method annotation used to invert test case results. If a JUnit 3/4 test case method is
- * annotated with {@code @NotYetImplemented} the test will fail if no test failure occurs and it will pass
- * if a test failure occurs.
+ * Method annotation used to invert test case results. If a JUnit 3/4/5 test case method is
+ * annotated with {@code @NotYetImplemented}, the test will fail if no test failure occurs
+ * and it will pass if a test failure occurs.
  * <p>
  * This is helpful for tests that don't currently work but should work one day,
  * when the tested functionality has been implemented.
  * <p>
- * The idea for this AST transformation originated in {@link groovy.test.GroovyTestCase#notYetImplemented()}.
+ * Note: JUnit 3 users should use the optional {@code exception} attribute, e.g. {@code @NotYetImplemented(exception=junit.framework.AssertionFailedError)}.
  *
  * @since 3.0.0
  */
@@ -42,4 +42,11 @@ import java.lang.annotation.Target;
 @Target({ElementType.METHOD})
 @GroovyASTTransformationClass("org.apache.groovy.test.transform.NotYetImplementedASTTransformation")
 public @interface NotYetImplemented {
+    /**
+     * If defined, tests which unexpectedly pass will throw this exception.
+     * The supplied exception class should have a constructor variant accepting a single String error message.
+     *
+     * @since 3.0.3
+     */
+    Class<? extends AssertionError> exception() default AssertionError.class;
 }
diff --git a/subprojects/groovy-test/src/main/java/org/apache/groovy/test/transform/NotYetImplementedASTTransformation.java b/subprojects/groovy-test/src/main/java/org/apache/groovy/test/transform/NotYetImplementedASTTransformation.java
index 7c6250c..c6f7f70 100644
--- a/subprojects/groovy-test/src/main/java/org/apache/groovy/test/transform/NotYetImplementedASTTransformation.java
+++ b/subprojects/groovy-test/src/main/java/org/apache/groovy/test/transform/NotYetImplementedASTTransformation.java
@@ -22,7 +22,9 @@ import org.codehaus.groovy.ast.ASTNode;
 import org.codehaus.groovy.ast.AnnotationNode;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.ConstructorNode;
 import org.codehaus.groovy.ast.MethodNode;
+import org.codehaus.groovy.ast.Parameter;
 import org.codehaus.groovy.ast.stmt.BlockStatement;
 import org.codehaus.groovy.ast.stmt.ReturnStatement;
 import org.codehaus.groovy.ast.stmt.ThrowStatement;
@@ -32,9 +34,6 @@ import org.codehaus.groovy.control.SourceUnit;
 import org.codehaus.groovy.transform.AbstractASTTransformation;
 import org.codehaus.groovy.transform.GroovyASTTransformation;
 
-import java.util.Arrays;
-import junit.framework.AssertionFailedError;
-
 import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.block;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.catchS;
@@ -53,22 +52,29 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.tryCatchS;
 public class NotYetImplementedASTTransformation extends AbstractASTTransformation {
 
     private static final ClassNode CATCH_TYPE = ClassHelper.make(Throwable.class);
-    private static final ClassNode THROW_TYPE = ClassHelper.make(AssertionFailedError.class); // TODO: java.lang.AssertionError
+    private static final ClassNode DEFAULT_THROW_TYPE = ClassHelper.make(AssertionError.class);
 
     public void visit(ASTNode[] nodes, SourceUnit source) {
-        if (!(nodes.length == 2 && nodes[0] instanceof AnnotationNode && nodes[1] instanceof MethodNode)) {
-            throw new RuntimeException("Internal error: expecting [AnnotationNode, MethodNode] but got: " + Arrays.toString(nodes));
-        }
-
+        init(nodes, source);
+        AnnotationNode anno = (AnnotationNode) nodes[0];
         MethodNode methodNode = (MethodNode) nodes[1];
 
-        if (methodNode.getCode() instanceof BlockStatement && !((BlockStatement) methodNode.getCode()).isEmpty()) {
+        ClassNode exception = getMemberClassValue(anno, "exception");
+        if (exception == null) {
+            exception = DEFAULT_THROW_TYPE;
+        }
+        ConstructorNode cons = exception.getDeclaredConstructor(new Parameter[]{new Parameter(ClassHelper.STRING_TYPE, "dummy")});
+        if (cons == null) {
+            addError("Error during @NotYetImplemented processing: supplied exception " + exception.getNameWithoutPackage() + " doesn't have expected String constructor", methodNode);
+        }
+
+        if (methodNode.getCode() instanceof BlockStatement && !methodNode.getCode().isEmpty()) {
             // wrap code in try/catch with return for failure path followed by throws for success path
 
             TryCatchStatement tryCatchStatement = tryCatchS(methodNode.getCode());
             tryCatchStatement.addCatch(catchS(param(CATCH_TYPE, "ignore"), ReturnStatement.RETURN_NULL_OR_VOID));
 
-            ThrowStatement throwStatement = throwS(ctorX(THROW_TYPE, args(constX("Method is marked with @NotYetImplemented but passes unexpectedly"))));
+            ThrowStatement throwStatement = throwS(ctorX(exception, args(constX("Method is marked with @NotYetImplemented but passes unexpectedly"))));
 
             methodNode.setCode(block(tryCatchStatement, throwStatement));
         }
diff --git a/subprojects/groovy-test/src/test/groovy/org/apache/groovy/test/transform/NotYetImplementedTransformTest.groovy b/subprojects/groovy-test/src/test/groovy/org/apache/groovy/test/transform/NotYetImplementedTransformTest.groovy
index f6eef26..b4fbad5 100644
--- a/subprojects/groovy-test/src/test/groovy/org/apache/groovy/test/transform/NotYetImplementedTransformTest.groovy
+++ b/subprojects/groovy-test/src/test/groovy/org/apache/groovy/test/transform/NotYetImplementedTransformTest.groovy
@@ -18,7 +18,6 @@
  */
 package org.apache.groovy.test.transform
 
-import junit.framework.AssertionFailedError
 import org.junit.Test
 
 final class NotYetImplementedTransformTest {
@@ -40,7 +39,7 @@ final class NotYetImplementedTransformTest {
             junit.textui.TestRunner.run(new junit.framework.TestSuite(MyTests))
         ''')
 
-        assert output.wasSuccessful() : 'test method marked with @NotYetImplemented must NOT throw an AssertionFailedError'
+        assert output.wasSuccessful() : 'failing test method marked with @NotYetImplemented must NOT throw an AssertionError'
     }
 
     @Test
@@ -58,7 +57,7 @@ final class NotYetImplementedTransformTest {
             junit.textui.TestRunner.run(new junit.framework.TestSuite(MyTests))
         ''')
 
-        assert output.wasSuccessful() : 'test method marked with @NotYetImplemented must NOT throw an AssertionFailedError'
+        assert output.wasSuccessful() : 'test method throwing exception marked with @NotYetImplemented must NOT throw an AssertionError'
     }
 
     @Test
@@ -68,7 +67,7 @@ final class NotYetImplementedTransformTest {
             import groovy.test.NotYetImplemented
 
             class MyTests extends GroovyTestCase {
-                @NotYetImplemented void testThatPasses()  {
+                @NotYetImplemented(exception=junit.framework.AssertionFailedError) void testThatPasses()  {
                     assertTrue(true)
                 }
             }
@@ -76,8 +75,8 @@ final class NotYetImplementedTransformTest {
             junit.textui.TestRunner.run(new junit.framework.TestSuite(MyTests))
         ''')
 
-        assert output.failureCount() == 1 : 'test method marked with @NotYetImplemented must throw an AssertionFailedError'
-        assert output.failures().nextElement().thrownException() instanceof AssertionFailedError : 'test method marked with @NotYetImplemented must throw an AssertionFailedError'
+        assert output.failureCount() == 1 : 'succeeding test method marked with @NotYetImplemented must throw an AssertionError'
+        assert output.failures().nextElement().thrownException() instanceof AssertionError : 'succeeding test method marked with @NotYetImplemented must throw an AssertionError'
     }
 
     @Test
@@ -94,7 +93,7 @@ final class NotYetImplementedTransformTest {
             junit.textui.TestRunner.run(new junit.framework.TestSuite(MyTests))
         ''')
 
-        assert output.wasSuccessful() : 'empty test method must not throw an AssertionFailedError'
+        assert output.wasSuccessful() : 'empty test method must not throw an AssertionError'
     }
 
     @Test
@@ -114,7 +113,7 @@ final class NotYetImplementedTransformTest {
             new JUnitCore().run(MyTests)
         ''')
 
-        assert output.wasSuccessful() : '@Test method marked with @NotYetImplemented must NOT throw an AssertionFailedError'
+        assert output.wasSuccessful() : 'failing @Test method marked with @NotYetImplemented must NOT throw an AssertionError'
     }
 
     @Test // GROOVY-8457
@@ -135,7 +134,7 @@ final class NotYetImplementedTransformTest {
             new JUnitCore().run(MyTests)
         ''')
 
-        assert output.wasSuccessful() : '@Test method marked with @CompileStatic and @NotYetImplemented must NOT throw an AssertionFailedError'
+        assert output.wasSuccessful() : 'failing @Test method marked with @CompileStatic and @NotYetImplemented must NOT throw an AssertionError'
     }
 
     @Test
@@ -155,8 +154,8 @@ final class NotYetImplementedTransformTest {
             new JUnitCore().run(MyTests)
         ''')
 
-        assert output.failureCount == 1 : '@Test method marked with @NotYetImplemented must throw an AssertionFailedError'
-        assert output.failures.first().exception instanceof AssertionFailedError : '@Test method marked with @NotYetImplemented must throw an AssertionFailedError'
+        assert output.failureCount == 1 : 'succeeding @Test method marked with @NotYetImplemented must throw an AssertionError'
+        assert output.failures.first().exception instanceof AssertionError : 'succeeding @Test method marked with @NotYetImplemented must throw an AssertionError'
     }
 
     @Test // GROOVY-8457
@@ -177,7 +176,7 @@ final class NotYetImplementedTransformTest {
             new JUnitCore().run(MyTests)
         ''')
 
-        assert output.failureCount == 1 : '@Test method marked with @CompileStatic and @NotYetImplemented must throw an AssertionFailedError'
-        assert output.failures.first().exception instanceof AssertionFailedError : '@Test method marked with @CompileStatic and @NotYetImplemented must throw an AssertionFailedError'
+        assert output.failureCount == 1 : 'succeeding @Test method marked with @CompileStatic and @NotYetImplemented must throw an AssertionError'
+        assert output.failures.first().exception instanceof AssertionError : 'succeeding @Test method marked with @CompileStatic and @NotYetImplemented must throw an AssertionError'
     }
 }