HHH-4822 - Add @FailureExpected annotation to annotations and entitymananger modules to allow the skipping of tests

git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@18638 1b8cb986-b30d-0410-93ca-fae66ebed9b2

annotations/src/test/java/org/hibernate/test/annotations/cid/CompositeIdTest.java

@@ -9,11 +9,10 @@ import org.hibernate.Query;
 import org.hibernate.Session;
 import org.hibernate.Transaction;
 import org.hibernate.Criteria;
-import org.hibernate.dialect.HSQLDialect;
 import org.hibernate.criterion.Disjunction;
 import org.hibernate.criterion.Restrictions;
+import org.hibernate.junit.SkipForDialect;
 import org.hibernate.test.annotations.TestCase;
-import org.hibernate.test.annotations.SkipForDialect;
 
 /**
  * test some composite id functionalities

annotations/src/test/java/org/hibernate/test/annotations/idclass/xml/IdClassXmlTest.java

@@ -22,20 +22,17 @@
  */
 package org.hibernate.test.annotations.idclass.xml;
 
-import org.hibernate.Query;
-import org.hibernate.Session;
-import org.hibernate.Transaction;
-import org.hibernate.test.annotations.FailureExpected;
+import org.hibernate.junit.FailureExpected;
 import org.hibernate.test.annotations.TestCase;
 
 /**
- * HHH-4282
+ * A test for HHH-4282
  *
  * @author Hardy Ferentschik
  */
+@FailureExpected( jiraKey = "HHH-4282" )
 public class IdClassXmlTest extends TestCase {
 
-	@FailureExpected
 	public void testEntityMappningPropertiesAreNotIgnored() {
 		throw new RuntimeException();
 //		Session s = openSession();

annotations/src/test/java/org/hibernate/test/annotations/idclassgeneratedvalue/IdClassGeneratedValueTest.java

@@ -27,7 +27,7 @@ package org.hibernate.test.annotations.idclassgeneratedvalue;
 import java.util.List;
 
 import org.hibernate.Session;
-import org.hibernate.test.annotations.FailureExpected;
+import org.hibernate.junit.FailureExpected;
 import org.hibernate.test.annotations.TestCase;
 
 /**
@@ -60,7 +60,7 @@ public class IdClassGeneratedValueTest extends TestCase {
 		s.close();
 	}
 
-	@FailureExpected(message = "Not yet implemented", issueNumber = "HHH-4552")
+	@FailureExpected(message = "Not yet implemented", jiraKey = "HHH-4552")
 	@SuppressWarnings({ "unchecked" })
 	public void testSingleGeneratedValue() {
 		Session s = openSession();
@@ -85,7 +85,7 @@ public class IdClassGeneratedValueTest extends TestCase {
 		s.close();
 	}
 
-	@FailureExpected(message = "Not yet implemented", issueNumber = "HHH-4552")
+	@FailureExpected(message = "Not yet implemented", jiraKey = "HHH-4552")
 	@SuppressWarnings({ "unchecked" })
 	public void testMultipleGeneratedValue() {
 		Session s = openSession();

annotations/src/test/java/org/hibernate/test/annotations/indexcoll/IndexedCollectionTest.java

@@ -12,9 +12,9 @@ import org.hibernate.Hibernate;
 import org.hibernate.Session;
 import org.hibernate.Transaction;
 import org.hibernate.dialect.HSQLDialect;
+import org.hibernate.junit.RequiresDialect;
 import org.hibernate.mapping.Collection;
 import org.hibernate.mapping.Column;
-import org.hibernate.test.annotations.RequiresDialect;
 import org.hibernate.test.annotations.TestCase;
 
 /**

annotations/src/test/java/org/hibernate/test/annotations/lob/ImageTest.java

@@ -32,7 +32,7 @@ import org.hibernate.dialect.SQLServerDialect;
 import org.hibernate.dialect.Sybase11Dialect;
 import org.hibernate.dialect.SybaseASE15Dialect;
 import org.hibernate.dialect.SybaseDialect;
-import org.hibernate.test.annotations.RequiresDialect;
+import org.hibernate.junit.RequiresDialect;
 import org.hibernate.test.annotations.TestCase;
 import org.hibernate.util.ArrayHelper;
 

annotations/src/test/java/org/hibernate/test/annotations/lob/TextTest.java

@@ -32,7 +32,7 @@ import org.hibernate.dialect.SQLServerDialect;
 import org.hibernate.dialect.Sybase11Dialect;
 import org.hibernate.dialect.SybaseASE15Dialect;
 import org.hibernate.dialect.SybaseDialect;
-import org.hibernate.test.annotations.RequiresDialect;
+import org.hibernate.junit.RequiresDialect;
 import org.hibernate.test.annotations.TestCase;
 import org.hibernate.util.ArrayHelper;
 

annotations/src/test/java/org/hibernate/test/annotations/manytoonewithformula/ManyToOneWithFormulaTest.java

@@ -29,7 +29,7 @@ package org.hibernate.test.annotations.manytoonewithformula;
 import org.hibernate.Session;
 import org.hibernate.Transaction;
 import org.hibernate.dialect.HSQLDialect;
-import org.hibernate.test.annotations.SkipForDialect;
+import org.hibernate.junit.SkipForDialect;
 import org.hibernate.test.annotations.TestCase;
 
 /**

annotations/src/test/java/org/hibernate/test/annotations/onetomany/OneToManyTest.java

@@ -12,11 +12,9 @@ import org.hibernate.Hibernate;
 import org.hibernate.HibernateException;
 import org.hibernate.Session;
 import org.hibernate.Transaction;
-import org.hibernate.dialect.HSQLDialect;
 import org.hibernate.test.annotations.Customer;
 import org.hibernate.test.annotations.Discount;
 import org.hibernate.test.annotations.Passport;
-import org.hibernate.test.annotations.RequiresDialect;
 import org.hibernate.test.annotations.TestCase;
 import org.hibernate.test.annotations.Ticket;
 import org.hibernate.test.annotations.TicketComparator;

entitymanager/src/test/java/org/hibernate/ejb/test/callbacks/CallbacksTest.java

@@ -10,7 +10,7 @@ import javax.persistence.EntityManager;
 import org.hibernate.ejb.test.Cat;
 import org.hibernate.ejb.test.Kitten;
 import org.hibernate.ejb.test.TestCase;
-import org.hibernate.test.annotations.FailureExpected;
+import org.hibernate.junit.FailureExpected;
 
 /**
  * @author Emmanuel Bernard
@@ -168,7 +168,7 @@ public class CallbacksTest extends TestCase {
 		em.close();
 	}
 	
-	@FailureExpected(message = "collection change does not trigger an event", issueNumber = "EJB-288")
+	@FailureExpected(message = "collection change does not trigger an event", jiraKey = "EJB-288")
 	public void testPostUpdateCollection() throws Exception {
 		// create a cat
 		EntityManager em = getOrCreateEntityManager();

entitymanager/src/test/java/org/hibernate/ejb/test/lob/BlobTest.java

@@ -12,7 +12,6 @@ import javax.persistence.EntityManager;
 import org.hibernate.Hibernate;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.ejb.test.TestCase;
-import org.hibernate.test.annotations.FailureExpected;
 
 /**
  * @author Emmanuel Bernard

testing/src/main/java/org/hibernate/junit/FailureExpected.java

@@ -1,4 +1,3 @@
-// $Id$
 /*
  * Hibernate, Relational Persistence for Idiomatic Java
  *
@@ -22,7 +21,7 @@
  * 51 Franklin Street, Fifth Floor
  * Boston, MA  02110-1301  USA
  */
-package org.hibernate.test.annotations;
+package org.hibernate.junit;
 
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
@@ -30,13 +29,23 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 /**
- * Annotations used to mark a expected test failure.
+ * Annotations used to mark a test as an expected failure.
  *
  * @author Hardy Ferentschik
+ * @author Steve Ebersole
  */
-@Target({ ElementType.METHOD, ElementType.TYPE })
 @Retention(RetentionPolicy.RUNTIME)
+@Target({ ElementType.METHOD, ElementType.TYPE })
 public @interface FailureExpected {
+	/**
+	 * The key of a JIRA issue which covers this expected failure.
+	 * @return The jira issue key
+	 */
+	String jiraKey();
+
+	/**
+	 * A message explaining the reason for the expected failure.  Optional.
+	 * @return The reason
+	 */
 	String message() default "";
-	String issueNumber() default "";
 }

testing/src/main/java/org/hibernate/junit/RequiresDialect.java

@@ -1,4 +1,3 @@
-// $Id$
 /*
  * Hibernate, Relational Persistence for Idiomatic Java
  *
@@ -22,7 +21,7 @@
  * 51 Franklin Street, Fifth Floor
  * Boston, MA  02110-1301  USA
  */
-package org.hibernate.test.annotations;
+package org.hibernate.junit;
 
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
@@ -32,12 +31,24 @@ import java.lang.annotation.Target;
 import org.hibernate.dialect.Dialect;
 
 /**
- * Annotations used to mark a test to be specific to a given dialect.
+ * Annotation used to indicate that a test should be run only when run against the
+ * indicated dialects.
  * 
  * @author Hardy Ferentschik
  */
 @Target({ElementType.METHOD, ElementType.TYPE})
 @Retention(RetentionPolicy.RUNTIME)
 public @interface RequiresDialect {
+	/**
+	 * The dialects against which to run the test
+	 * @return The dialects
+	 */
 	Class<? extends Dialect>[] value();
+
+	/**
+	 * Used to indicate if the dialects should be matched strictly (classes equal) or
+	 * non-strictly (instanceof).
+	 * @return Should strict matching be used?
+	 */
+	boolean strictMatching() default false;
 }

testing/src/main/java/org/hibernate/junit/SkipForDialect.java

@@ -1,4 +1,3 @@
-// $Id$
 /*
  * Hibernate, Relational Persistence for Idiomatic Java
  *
@@ -22,7 +21,7 @@
  * 51 Franklin Street, Fifth Floor
  * Boston, MA  02110-1301  USA
  */
-package org.hibernate.test.annotations;
+package org.hibernate.junit;
 
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
@@ -32,14 +31,38 @@ import java.lang.annotation.Target;
 import org.hibernate.dialect.Dialect;
 
 /**
- * Annotations used to mark a test to be specific to a given dialect.
+ * Annotation used to indicate that a test should be skipped when run against the
+ * indicated dialects.
  *
  * @author Hardy Ferentschik
+ * @author Steve Ebersole
  */
-@Target({ ElementType.METHOD, ElementType.TYPE })
 @Retention(RetentionPolicy.RUNTIME)
+@Target({ ElementType.METHOD, ElementType.TYPE })
 public @interface SkipForDialect {
+	/**
+	 * The dialects against which to skip the test
+	 * @return The dialects
+	 */
 	Class<? extends Dialect>[] value();
 
-	String comment();
+	/**
+	 * Used to indicate if the dialects should be matched strictly (classes equal) or
+	 * non-strictly (instanceof).
+	 * @return Should strict matching be used?
+	 */
+	boolean strictMatching() default false;
+
+	/**
+	 * Comment describing the reason for the skip.
+	 * @return The comment
+	 */
+	String comment() default "";
+
+	/**
+	 * The key of a JIRA issue which covers the reason for this skip.  Eventually we should make this
+	 * a requirement.
+	 * @return The jira issue key
+	 */
+	String jiraKey() default "";
 }

testing/src/main/java/org/hibernate/test/annotations/HibernateTestCase.java

@@ -1,4 +1,3 @@
-// $Id:$
 /*
  * Hibernate, Relational Persistence for Idiomatic Java
  *
@@ -24,6 +23,7 @@
  */
 package org.hibernate.test.annotations;
 
+import java.lang.annotation.Annotation;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
@@ -40,6 +40,9 @@ import org.slf4j.LoggerFactory;
 import org.hibernate.cfg.Configuration;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.jdbc.Work;
+import org.hibernate.junit.FailureExpected;
+import org.hibernate.junit.RequiresDialect;
+import org.hibernate.junit.SkipForDialect;
 import org.hibernate.tool.hbm2ddl.SchemaExport;
 import org.hibernate.util.StringHelper;
 
@@ -57,11 +60,6 @@ public abstract class HibernateTestCase extends TestCase {
 	private static Class<?> lastTestClass;
 
 
-	/**
-	 * The test method.
-	 */
-	private Method runMethod = null;
-
 	/**
 	 * Flag indicating whether the test should be run or skipped.
 	 */
@@ -87,78 +85,162 @@ public abstract class HibernateTestCase extends TestCase {
 	}
 
 	@Override
-	protected void setUp() throws Exception {
-		runMethod = findTestMethod();
-		setRunTestFlag( runMethod );
-		if ( runTest ) {
-			if ( cfg == null || lastTestClass != getClass() ) {
-				buildConfiguration();
-				lastTestClass = getClass();
+	public void runBare() throws Throwable {
+		Method runMethod = findTestMethod();
+
+		final Skip skip = determineSkipByDialect( Dialect.getDialect(), runMethod );
+		if ( skip != null ) {
+			reportSkip( skip );
+			return;
+		}
+
+		setUp();
+		try {
+			runTest();
+		}
+		finally {
+			tearDown();
+		}
+	}
+
+	@Override
+	protected void runTest() throws Throwable {
+		Method runMethod = findTestMethod();
+		FailureExpected failureExpected = locateAnnotation( FailureExpected.class, runMethod );
+		try {
+			super.runTest();
+			if ( failureExpected != null ) {
+				throw new FailureExpectedTestPassedException();
+			}
+		}
+		catch ( FailureExpectedTestPassedException t ) {
+			closeResources();
+			throw t;
+		}
+		catch ( Throwable t ) {
+			if ( t instanceof InvocationTargetException ) {
+				t = ( ( InvocationTargetException ) t ).getTargetException();
+			}
+			if ( t instanceof IllegalAccessException ) {
+				t.fillInStackTrace();
+			}
+			closeResources();
+			if ( failureExpected != null) {
+				StringBuilder builder = new StringBuilder();
+				if ( StringHelper.isNotEmpty( failureExpected.message() ) ) {
+					builder.append( failureExpected.message() );
+				}
+				else {
+					builder.append( "ignoring @FailureExpected test" );
+				}
+				builder.append( " (" )
+						.append( failureExpected.jiraKey() )
+						.append( ")" );
+				reportSkip( "Failed with: " + t.toString(), builder.toString()  );
 			}
 			else {
-				runSchemaGeneration();
+				throw t;
 			}
 		}
 	}
 
+	@Override
+	protected void setUp() throws Exception {
+		if ( cfg == null || lastTestClass != getClass() ) {
+			buildConfiguration();
+			lastTestClass = getClass();
+		}
+		else {
+			runSchemaGeneration();
+		}
+	}
+
 	@Override
 	protected void tearDown() throws Exception {
 		runSchemaDrop();
 		handleUnclosedResources();
 	}
 
-	protected void runTest() throws Throwable {
-		if ( runTest ) {
-			runTestMethod( runMethod );
+	protected static class Skip {
+		private final String reason;
+		private final String testDescription;
+
+		public Skip(String reason, String testDescription) {
+			this.reason = reason;
+			this.testDescription = testDescription;
 		}
 	}
 
-	private void setRunTestFlag(Method runMethod) {
-		updateRequiredDialectList( runMethod );
-		updateSkipForDialectList( runMethod );
+	protected final Skip determineSkipByDialect(Dialect dialect, Method runMethod) {
+		// skips have precedence, so check them first
+		SkipForDialect skipForDialectAnn = locateAnnotation( SkipForDialect.class, runMethod );
+		if ( skipForDialectAnn != null ) {
+			for ( Class<? extends Dialect> dialectClass : skipForDialectAnn.value() ) {
+				if ( skipForDialectAnn.strictMatching() ) {
+					if ( dialectClass.equals( dialect.getClass() ) ) {
+						return buildSkip( dialect, skipForDialectAnn.comment(), skipForDialectAnn.jiraKey() );
+					}
+				}
+				else {
+					if ( dialectClass.isInstance( dialect ) ) {
+						return buildSkip( dialect, skipForDialectAnn.comment(), skipForDialectAnn.jiraKey() );
+					}
+				}
+			}
+		}
 
-		if ( runForCurrentDialect() ) {
-			runTest = true;
-		}
-		else {
-			log.warn(
-					"Skipping test {}, because test does not apply for dialect {}", runMethod.getName(), Dialect
-							.getDialect().getClass()
-			);
-			runTest = false;
+		// then check against the requires
+		RequiresDialect requiresDialectAnn = locateAnnotation( RequiresDialect.class, runMethod );
+		if ( requiresDialectAnn != null ) {
+			for ( Class<? extends Dialect> dialectClass : requiresDialectAnn.value() ) {
+				if ( requiresDialectAnn.strictMatching() ) {
+					if ( dialectClass.equals( dialect.getClass() ) ) {
+						return buildSkip( dialect, null, null );
+					}
+				}
+				else {
+					if ( dialectClass.isInstance( dialect ) ) {
+						return buildSkip( dialect, null, null );
+					}
+				}
+			}
 		}
+
+		return null;
 	}
 
-	private void updateRequiredDialectList(Method runMethod) {
-		requiredDialectList.clear();
-
-		RequiresDialect requiresDialectMethodAnn = runMethod.getAnnotation( RequiresDialect.class );
-		if ( requiresDialectMethodAnn != null ) {
-			Class<? extends Dialect>[] requiredDialects = requiresDialectMethodAnn.value();
-			requiredDialectList.addAll( Arrays.asList( requiredDialects ) );
+	protected <T extends Annotation> T locateAnnotation(Class<T> annotationClass, Method runMethod) {
+		T annotation = runMethod.getAnnotation( annotationClass );
+		if ( annotation == null ) {
+			annotation = getClass().getAnnotation( annotationClass );
 		}
-
-		RequiresDialect requiresDialectClassAnn = getClass().getAnnotation( RequiresDialect.class );
-		if ( requiresDialectClassAnn != null ) {
-			Class<? extends Dialect>[] requiredDialects = requiresDialectClassAnn.value();
-			requiredDialectList.addAll( Arrays.asList( requiredDialects ) );
+		if ( annotation == null ) {
+			annotation = runMethod.getDeclaringClass().getAnnotation( annotationClass );
 		}
+		return annotation;
 	}
 
-	private void updateSkipForDialectList(Method runMethod) {
-		skipForDialectList.clear();
+	protected Skip buildSkip(Dialect dialect, String comment, String jiraKey) {
+		StringBuilder buffer = new StringBuilder();
+		buffer.append( "skipping database-specific test [" );
+		buffer.append( fullTestName() );
+		buffer.append( "] for dialect [" );
+		buffer.append( dialect.getClass().getName() );
+		buffer.append( ']' );
 
-		SkipForDialect skipForDialectMethodAnn = runMethod.getAnnotation( SkipForDialect.class );
-		if ( skipForDialectMethodAnn != null ) {
-			Class<? extends Dialect>[] skipDialects = skipForDialectMethodAnn.value();
-			skipForDialectList.addAll( Arrays.asList( skipDialects ) );
+		if ( StringHelper.isNotEmpty( comment ) ) {
+			buffer.append( "; " ).append( comment );
 		}
 
-		SkipForDialect skipForDialectClassAnn = getClass().getAnnotation( SkipForDialect.class );
-		if ( skipForDialectClassAnn != null ) {
-			Class<? extends Dialect>[] skipDialects = skipForDialectClassAnn.value();
-			skipForDialectList.addAll( Arrays.asList( skipDialects ) );
+		if ( StringHelper.isNotEmpty( jiraKey ) ) {
+			buffer.append( " (" ).append( jiraKey ).append( ')' );
 		}
+
+		return new Skip( buffer.toString(), null );
+	}
+
+	public String fullTestName() {
+		return this.getClass().getName() + "#" + this.getName();
 	}
 
 	protected boolean runForCurrentDialect() {
@@ -185,48 +267,6 @@ public abstract class HibernateTestCase extends TestCase {
 		return runTestForCurrentDialect;
 	}
 
-	private void runTestMethod(Method runMethod) throws Throwable {
-		boolean failureExpected = runMethod.getAnnotation( FailureExpected.class ) != null;
-		try {
-			runMethod.invoke( this, new Class[0] );
-			if ( failureExpected ) {
-				throw new FailureExpectedTestPassedException();
-			}
-		}
-		catch ( FailureExpectedTestPassedException t ) {
-			closeResources();
-			throw t;
-		}
-		catch ( Throwable t ) {
-			if ( t instanceof InvocationTargetException ) {
-				t = ( ( InvocationTargetException ) t ).getTargetException();
-			}
-			if ( t instanceof IllegalAccessException ) {
-				t.fillInStackTrace();
-			}
-			closeResources();
-			if ( failureExpected ) {
-				FailureExpected ann = runMethod.getAnnotation( FailureExpected.class );
-				StringBuilder builder = new StringBuilder();
-				if ( StringHelper.isNotEmpty( ann.message() ) ) {
-					builder.append( ann.message() );
-				}
-				else {
-					builder.append( "ignoring test methods annoated with @FailureExpected" );
-				}
-				if ( StringHelper.isNotEmpty( ann.issueNumber() ) ) {
-					builder.append( " (" );
-					builder.append( ann.issueNumber() );
-					builder.append( ")" );
-				}
-				reportSkip( builder.toString(), "Failed with: " + t.toString() );
-			}
-			else {
-				throw t;
-			}
-		}
-	}
-
 	private Method findTestMethod() {
 		String fName = getName();
 		assertNotNull( fName );
@@ -288,12 +328,14 @@ public abstract class HibernateTestCase extends TestCase {
 		export.drop( true, true );
 	}
 
+	private void reportSkip(Skip skip) {
+		reportSkip( skip.reason, skip.testDescription );
+	}
+
 	protected void reportSkip(String reason, String testDescription) {
 		StringBuilder builder = new StringBuilder();
 		builder.append( "*** skipping test [" );
-		builder.append( runMethod.getDeclaringClass().getName() );
-		builder.append( "." );
-		builder.append( runMethod.getName() );
+		builder.append( fullTestName() );
 		builder.append( "] - " );
 		builder.append( testDescription );
 		builder.append( " : " );