Bug 372209 - Capture screenshots before @after clean-ups run

Thomas Wolf · Thomas Wolf · commit 1d99c548d5c2 · 2016-01-26T09:22:42.000+01:00
SWTBotJUnit4Runner takes screenshots after @after methods have run, which frequently leads to not very useful screenshots. Amend this by taking the screenshot as soon as possible after a test method failure, before @after runs. The former mechanism based on a RunListener still exists and will take a screenshot (if none had been taken earlier for a particular test method) if some @After/@before method fails or a rule such as @ExpectedException fails the test. In those cases, the behavior is unchanged: these screenshots are still taken after @after has run. Change-Id: Ifda1e76a5ddd3d29ee0826056b87b88cef31a290 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
diff --git a/org.eclipse.swtbot.junit4_x/META-INF/MANIFEST.MF b/org.eclipse.swtbot.junit4_x/META-INF/MANIFEST.MF
@@ -10,6 +10,8 @@ Import-Package: org.apache.log4j;version="1.2.12",
  org.eclipse.swt.widgets,
  org.eclipse.swtbot.swt.finder,
  org.eclipse.swtbot.swt.finder.utils,
+ org.junit;version="4.8.0",
+ org.junit.runner;version="4.8.0",
  org.junit.runner.notification;version="4.8.0",
  org.junit.runners;version="4.8.0",
  org.junit.runners.model;version="4.8.0"
diff --git a/org.eclipse.swtbot.junit4_x/src/org/eclipse/swtbot/swt/finder/junit/SWTBotJUnit4Suite.java b/org.eclipse.swtbot.junit4_x/src/org/eclipse/swtbot/swt/finder/junit/SWTBotJUnit4Suite.java
@@ -10,7 +10,7 @@
  *******************************************************************************/
 package org.eclipse.swtbot.swt.finder.junit;
 
-import org.junit.runner.notification.RunListener;
+import org.eclipse.swtbot.swt.finder.junit.internal.ScreenshotCaptureNotifier;
 import org.junit.runner.notification.RunNotifier;
 import org.junit.runners.Suite;
 import org.junit.runners.model.RunnerBuilder;
@@ -50,13 +50,11 @@ public SWTBotJUnit4Suite(Class<?> klass, Class<?>[] annotatedClasses) throws Exc
 	 * @see org.junit.runners.Suite#run(RunNotifier)
 	 */
 	public void run(RunNotifier notifier) {
-		RunListener failureSpy = new ScreenshotCaptureListener();
-		notifier.removeListener(failureSpy); // remove existing listeners that could be added by suite or class runners
-		notifier.addListener(failureSpy);
-		try {
+		if (notifier instanceof ScreenshotCaptureNotifier) {
 			super.run(notifier);
-		} finally {
-			notifier.removeListener(failureSpy);
+		} else {
+			RunNotifier wrappedNotifier = new ScreenshotCaptureNotifier(notifier);
+			super.run(wrappedNotifier);
 		}
 	}
 
diff --git a/org.eclipse.swtbot.junit4_x/src/org/eclipse/swtbot/swt/finder/junit/SWTBotJunit4ClassRunner.java b/org.eclipse.swtbot.junit4_x/src/org/eclipse/swtbot/swt/finder/junit/SWTBotJunit4ClassRunner.java
@@ -8,15 +8,24 @@
  * Contributors:
  *     Hans Schwaebli - initial API and implementation (Bug 259787)
  *     Toby Weston  - initial API and implementation (Bug 259787)
+ *     Thomas Wolf - Bug 372209.
  *******************************************************************************/
 package org.eclipse.swtbot.swt.finder.junit;
 
-import org.junit.runner.notification.RunListener;
+import java.lang.reflect.Method;
+
+import org.eclipse.swtbot.swt.finder.junit.internal.ScreenshotCaptureNotifier;
+import org.junit.Test;
+import org.junit.Test.None;
+import org.junit.runner.Description;
+import org.junit.runner.notification.Failure;
 import org.junit.runner.notification.RunNotifier;
 import org.junit.runners.BlockJUnit4ClassRunner;
+import org.junit.runners.model.FrameworkMethod;
 
 /**
- * A runner that captures screenshots on test failures. If you wish to launch your application for your tests use
+ * A runner that captures screenshots on test failures. If you wish to launch
+ * your application for your tests use
  * {@link SWTBotApplicationLauncherClassRunner}. Typical usage is:
  *
  * <pre>
@@ -40,20 +49,114 @@ public class SWTBotJunit4ClassRunner extends BlockJUnit4ClassRunner {
 	/**
 	 * Creates a SWTBotRunner to run {@code klass}
 	 *
-	 * @throws Exception if the test class is malformed.
+	 * @throws Exception
+	 *             if the test class is malformed.
 	 */
 	public SWTBotJunit4ClassRunner(Class<?> klass) throws Exception {
 		super(klass);
 	}
 
+	@Override
 	public void run(RunNotifier notifier) {
-		RunListener failureSpy = new ScreenshotCaptureListener();
-		notifier.removeListener(failureSpy); // remove existing listeners that could be added by suite or class runners
-		notifier.addListener(failureSpy);
-		try {
+		if (notifier instanceof ScreenshotCaptureNotifier) {
 			super.run(notifier);
-		} finally {
-			notifier.removeListener(failureSpy);
+		} else {
+			RunNotifier wrappedNotifier = new ScreenshotCaptureNotifier(notifier);
+			super.run(wrappedNotifier);
+		}
+	}
+
+	@Override
+	protected void runChild(final FrameworkMethod method, RunNotifier notifier) {
+		Description description = describeChild(method);
+		if (isIgnored(method)) {
+			notifier.fireTestIgnored(description);
+		} else {
+			FrameworkMethod toRun = method;
+			if (notifier instanceof ScreenshotCaptureNotifier) {
+				toRun = new CapturingFrameworkMethod(method.getMethod(), description,
+						(ScreenshotCaptureNotifier) notifier);
+			}
+			runLeaf(methodBlock(toRun), description, notifier);
+		}
+	}
+
+	/**
+	 * A {@link FrameworkMethod} that tries to take a screenshot via the given
+	 * {@link ScreenshotCaptureNotifier} as soon as the test method fails,
+	 * before any @After clean-ups run.
+	 */
+	private static class CapturingFrameworkMethod extends FrameworkMethod {
+
+		// We test for this exception using the name because it only became
+		// public recently in JUnit 4.12
+		// TODO: remove once we require JUnit 4.12 or greater
+		private static final String ASSUMPTION_VIOLATED_EXCEPTION_NAME = "AssumptionViolatedException";
+
+		private final Class<? extends Throwable> expectedException;
+		private final Description description;
+		private final ScreenshotCaptureNotifier notifier;
+
+		public CapturingFrameworkMethod(Method method, Description description, ScreenshotCaptureNotifier notifier) {
+			super(method);
+			this.description = description;
+			this.notifier = notifier;
+			// Determine expected exception, if any
+			Test annotation = method.getAnnotation(Test.class);
+			expectedException = getExpectedException(annotation);
+		}
+
+		@Override
+		public Object invokeExplosively(Object target, Object... params) throws Throwable {
+			Object result = null;
+			try {
+				result = super.invokeExplosively(target, params);
+			} catch (Throwable e) {
+				// A timeout will give us an InterruptedException here and make
+				// us capture a screenshot, too.
+				// TODO: test for org.junit.AssumptionViolatedException once
+				// we require JUnit 4.12 or greater
+				if (!ASSUMPTION_VIOLATED_EXCEPTION_NAME.equals(e.getClass().getSimpleName())) {
+					if (expectedException == null || !expectedException.isAssignableFrom(e.getClass())) {
+						// Unexpected exception
+						notifier.captureScreenshot(new Failure(description, e));
+					}
+				}
+				throw e;
+			}
+			if (expectedException != null) {
+				// No exception, but we expected one
+				notifier.captureScreenshot(new Failure(description, null));
+				// No need to raise an exception, an outer statement will do so.
+			}
+			// If the test didn't throw an exception but there is an outer
+			// @ExpectedException rule that would expect one and then fail the
+			// test, ScreenshotCaptureNotifier.fireTestFailed() will eventually
+			// be called and will create the screenshot. In that case, however,
+			// @After clean-ups already have run.
+			return result;
+		}
+
+		private Class<? extends Throwable> getExpectedException(Test annotation) {
+			if (annotation == null || annotation.expected() == None.class) {
+				return null;
+			} else {
+				return annotation.expected();
+			}
+		}
+
+		@Override
+		public boolean equals(Object obj) {
+			// The new fields added in this sub-class shall not influence
+			// equality.
+			return super.equals(obj);
+		}
+
+		@Override
+		public int hashCode() {
+			// The new fields added in this sub-class shall not influence the
+			// hash code.
+			return super.hashCode();
 		}
 	}
 
diff --git a/org.eclipse.swtbot.junit4_x/src/org/eclipse/swtbot/swt/finder/junit/ScreenshotCaptureListener.java b/org.eclipse.swtbot.junit4_x/src/org/eclipse/swtbot/swt/finder/junit/ScreenshotCaptureListener.java
@@ -4,15 +4,19 @@
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
  * http://www.eclipse.org/legal/epl-v10.html
- * 
+ *
  * Contributors:
  *     Hans Schwaebli - initial API and implementation (Bug 259787)
+ *     Thomas Wolf - Bug 372209
  *******************************************************************************/
 package org.eclipse.swtbot.swt.finder.junit;
 
+import java.io.File;
+
 import org.apache.log4j.Logger;
 import org.eclipse.swtbot.swt.finder.utils.SWTBotPreferences;
 import org.eclipse.swtbot.swt.finder.utils.SWTUtils;
+import org.junit.runner.Description;
 import org.junit.runner.notification.Failure;
 import org.junit.runner.notification.RunListener;
 
@@ -25,38 +29,65 @@
  */
 public final class ScreenshotCaptureListener extends RunListener {
 	/** The logger. */
-	private static Logger	log					= Logger.getLogger(SWTBotApplicationLauncherClassRunner.class);
+	private static Logger log = Logger.getLogger(SWTBotApplicationLauncherClassRunner.class);
 
 	/** Counts the screenshots to determine if maximum number is reached. */
-	private static int		screenshotCounter	= 0;
+	private static int screenshotCounter = 0;
 
+	@Override
 	public void testFailure(Failure failure) throws Exception {
 		captureScreenshot(failure);
 	}
 
+	/**
+	 * Removes a previously taken screenshot, if any.
+	 *
+	 * @param description
+	 *            of the test
+	 * @since 2.4
+	 */
+	public void removeScreenshot(Description description) {
+		File file = new File(getFileName(new Failure(description, null)));
+		if (file.exists()) {
+			log.debug("Removing screenshot '" + file.getPath() + '\''); //$NON-NLS-1$
+			if (!file.delete() && file.exists()) {
+				log.warn("Could not remove screenshot " + file.getAbsolutePath()); //$NON-NLS-1$
+			}
+			--screenshotCounter;
+		}
+	}
+
 	private void captureScreenshot(Failure failure) {
 		try {
 			int maximumScreenshots = SWTBotPreferences.MAX_ERROR_SCREENSHOT_COUNT;
-			String fileName = SWTBotPreferences.SCREENSHOTS_DIR + "/" + failure.getTestHeader() + "." + SWTBotPreferences.SCREENSHOT_FORMAT.toLowerCase(); //$NON-NLS-1$
+			String fileName = getFileName(failure);
 			if (++screenshotCounter <= maximumScreenshots) {
 				captureScreenshot(fileName);
 			} else {
-				log.info("No screenshot captured for '" + failure.getTestHeader() + "' because maximum number of screenshots reached: " //$NON-NLS-1$ 
+				log.info("No screenshot captured for '" + failure.getTestHeader() //$NON-NLS-1$
+						+ "' because maximum number of screenshots reached: "
 						+ maximumScreenshots);
 			}
 		} catch (Exception e) {
 			log.warn("Could not capture screenshot", e); //$NON-NLS-1$
 		}
 	}
 
+	private String getFileName(Failure failure) {
+		return SWTBotPreferences.SCREENSHOTS_DIR + "/" + failure.getTestHeader() + "." //$NON-NLS-1$
+				+ SWTBotPreferences.SCREENSHOT_FORMAT.toLowerCase();
+	}
+
 	private boolean captureScreenshot(String fileName) {
 		return SWTUtils.captureScreenshot(fileName);
 	}
-	
+
+	@Override
 	public int hashCode() {
 		return 31;
 	}
 
+	@Override
 	public boolean equals(Object obj) {
 		if (this == obj)
 			return true;
@@ -66,5 +97,5 @@ public boolean equals(Object obj) {
 			return false;
 		return true;
 	}
-	
+
 }
diff --git a/org.eclipse.swtbot.junit4_x/src/org/eclipse/swtbot/swt/finder/junit/internal/ScreenshotCaptureNotifier.java b/org.eclipse.swtbot.junit4_x/src/org/eclipse/swtbot/swt/finder/junit/internal/ScreenshotCaptureNotifier.java
