Improve error handling when execution blocks throw exceptions

- Fixes [Exception in Context -> Tests Ignored #6](#6)
paulcwarren · Mar 16, 2017 · ffde0af · ffde0af
1 parent 0f2ac2e
commit ffde0af
Show file tree

Hide file tree

Showing 11 changed files with 419 additions and 59 deletions.
diff --git a/src/main/java/com/github/paulcwarren/ginkgo4j/ExecutableBlock.java b/src/main/java/com/github/paulcwarren/ginkgo4j/ExecutableBlock.java
@@ -1,5 +1,5 @@
 package com.github.paulcwarren.ginkgo4j;
 
 public interface ExecutableBlock {
-	void invoke() throws Throwable;
+	void invoke() throws Exception;
 }
diff --git a/src/main/java/com/github/paulcwarren/ginkgo4j/Ginkgo4jDSL.java b/src/main/java/com/github/paulcwarren/ginkgo4j/Ginkgo4jDSL.java
@@ -1,87 +1,138 @@
 package com.github.paulcwarren.ginkgo4j;
 
+import java.util.Stack;
+
 import impl.com.github.paulcwarren.ginkgo4j.builder.TestVisitor; 
 
 public class Ginkgo4jDSL {
 
-	private static TestVisitor visitor = null;
+	private static Stack<TestVisitor> stack = new Stack<>();
 
 	public static synchronized void setVisitor(TestVisitor b) {
-		visitor = b;
+		stack.push(b);
 	}
 
 	public static synchronized void unsetVisitor(TestVisitor b) {
-		if (b != visitor) {
+		if (b != stack.peek()) {
 			throw new IllegalStateException("Mismatched set/unset builder calls");
 		}
-		visitor = null;
+		stack.pop();
 	}
 
 	public static void Describe(String text, ExecutableBlock block) {
 		assertVisitor("Describe");
-		if (visitor != null) {
-			visitor.describe(text, block, false);
+		if (stack.peek() != null) {
+			try {
+				stack.peek().describe(text, block, false);
+			} catch (Exception e) {
+				if (e instanceof RuntimeException) {
+					throw (RuntimeException)e;
+				}
+				throw new RuntimeException("Unable to execute test", e);
+			}
 		}
  	}
 
 	public static void FDescribe(String text, ExecutableBlock block) {
 		assertVisitor("FDescribe");
-		if (visitor != null) {
-			visitor.describe(text, block, true);
+		if (stack.peek() != null) {
+			try {
+				stack.peek().describe(text, block, true);
+			} catch (Throwable e) {
+				if (e instanceof RuntimeException) {
+					throw (RuntimeException)e;
+				}
+				throw new RuntimeException("Unable to execute test", e);
+			}
 		}
  	}
 
 	public static void Context(String text, ExecutableBlock block) {
 		assertVisitor("Context");
-		if (visitor != null) {
-			visitor.context(text, block, false);
+		if (stack.peek() != null) {
+			try {
+				stack.peek().context(text, block, false);
+			} catch (Throwable e) {
+				if (e instanceof RuntimeException) {
+					throw (RuntimeException)e;
+				}
+				throw new RuntimeException("Unable to execute test", e);
+			}
 		}
 	}
 
 	public static void FContext(String text, ExecutableBlock block) {
 		assertVisitor("FContext");
-		if (visitor != null) {
-			visitor.context(text, block, true);
+		if (stack.peek() != null) {
+			try {
+				stack.peek().context(text, block, true);
+			} catch (Exception e) {
+				if (e instanceof RuntimeException) {
+					throw (RuntimeException)e;
+				}
+				throw new RuntimeException("Unable to execute test", e);
+			}
 		}
 	}
 
 	public static void BeforeEach(ExecutableBlock block) {
 		assertVisitor("BeforeEach");
-		if (visitor != null) {
-			visitor.beforeEach(block);
+		if (stack.peek() != null) {
+			try {
+				stack.peek().beforeEach(block);
+			} catch (Exception e) {
+				if (e instanceof RuntimeException) {
+					throw (RuntimeException)e;
+				}
+				throw new RuntimeException("Unable to execute test", e);
+			}
 		}
 	}
 
 	public static void JustBeforeEach(ExecutableBlock block) {
 		assertVisitor("JustBeforeEach");
-		if (visitor != null) {
-			visitor.justBeforeEach(block);
+		if (stack.peek() != null) {
+			try {
+				stack.peek().justBeforeEach(block);
+			} catch (Exception e) {
+				if (e instanceof RuntimeException) {
+					throw (RuntimeException)e;
+				}
+				throw new RuntimeException("Unable to execute test", e);
+			}
 		}
 	}
 
 	public static void It(String text, ExecutableBlock block) {
 		assertVisitor("It");
-		if (visitor != null) {
-			visitor.it(text, block, false);
+		if (stack.peek() != null) {
+			stack.peek().it(text, block, false);
 		}
 	}
 
 	public static void FIt(String text, ExecutableBlock block) {
 		assertVisitor("FIt");
-		if (visitor != null) {
-			visitor.it(text, block, true);
+		if (stack.peek() != null) {
+			stack.peek().it(text, block, true);
 		}
 	}
 
 	public static void AfterEach(ExecutableBlock block) {
 		assertVisitor("AfterEach");
-		if (visitor != null) {
-			visitor.afterEach(block);
+		if (stack.peek() != null) {
+			try {
+				stack.peek().afterEach(block);
+			} catch (Exception e) {
+				if (e instanceof RuntimeException) {
+					throw (RuntimeException)e;
+				}
+				throw new RuntimeException("Unable to execute test", e);
+			}
 		}
 	}
 
 	private static void assertVisitor(String string) {
-		if (visitor == null) {
+		if (stack.peek() == null) {
 			throw new IllegalStateException(string);
 		}
 	}

diff --git a/src/main/java/com/github/paulcwarren/ginkgo4j/Ginkgo4jRunner.java b/src/main/java/com/github/paulcwarren/ginkgo4j/Ginkgo4jRunner.java
@@ -10,6 +10,7 @@
 
 import org.junit.runner.Description;
 import org.junit.runner.Runner;
+import org.junit.runner.notification.Failure;
 import org.junit.runner.notification.RunNotifier;
 import org.junit.runners.model.InitializationError;
 
@@ -40,8 +41,12 @@ public Description getDescription() {
 			description = Description.createSuiteDescription(testClass.getName(), (Annotation[])null);
 
 			JunitDescriptionsCollector descCollector = new JunitDescriptionsCollector(description);
-			new TestWalker(testClass).walk(descCollector);
-			descriptions = descCollector.getDescriptions();
+			// collect as many descriptions as we can
+			try {
+				new TestWalker(testClass).walk(descCollector);
+			} finally {
+				descriptions = descCollector.getDescriptions();
+			}
 		}
 
         return description;
@@ -53,12 +58,20 @@ public void run(RunNotifier notifier) {
 		// to ensure setup happens
 		this.getDescription();
 
-		List<ExecutableChain> chains = calculateExecutionChains(testClass);
-
-		RunnerListener listener = new JunitRunnerListener(notifier, descriptions);
-		List<Runnable> runners = calculateWorkerThreads(chains, listener);
-
-		threadExecute(runners, getThreads(testClass));
+		try {
+			notifier.fireTestStarted(description);
+
+			List<ExecutableChain> chains = calculateExecutionChains(testClass);
+
+			RunnerListener listener = new JunitRunnerListener(notifier, descriptions);
+			List<Runnable> runners = calculateWorkerThreads(chains, listener);
+
+			threadExecute(runners, getThreads(testClass));
+		} catch (Exception e) {
+			notifier.fireTestFailure(new Failure(description, e));
+		} finally {
+			notifier.fireTestFinished(description);
+		}
    	}
 
 	static List<ExecutableChain> calculateExecutionChains(Class<?> testClass) {

diff --git a/src/main/java/impl/com/github/paulcwarren/ginkgo4j/builder/TestVisitor.java b/src/main/java/impl/com/github/paulcwarren/ginkgo4j/builder/TestVisitor.java
@@ -5,11 +5,11 @@
 public interface TestVisitor {
 
 	void test(Object test);
-	void describe(String text, ExecutableBlock block, boolean isFocused);
-	void context(String text, ExecutableBlock block, boolean isFocused);
-	void beforeEach(ExecutableBlock block);
-	void justBeforeEach(ExecutableBlock block);
+	void describe(String text, ExecutableBlock block, boolean isFocused) throws Exception;
+	void context(String text, ExecutableBlock block, boolean isFocused) throws Exception;
+	void beforeEach(ExecutableBlock block) throws Exception;
+	void justBeforeEach(ExecutableBlock block) throws Exception;
 	void it(String text, ExecutableBlock block, boolean isFocused);
-	void afterEach(ExecutableBlock block);
+	void afterEach(ExecutableBlock block) throws Exception;
 
 }
diff --git a/src/main/java/impl/com/github/paulcwarren/ginkgo4j/builder/TestWalker.java b/src/main/java/impl/com/github/paulcwarren/ginkgo4j/builder/TestWalker.java
@@ -53,28 +53,28 @@ public void walk(TestVisitor...visitors) {
 	}
 
 	@Override
-	public void describe(String text, ExecutableBlock block, boolean isFocused) {
+	public void describe(String text, ExecutableBlock block, boolean isFocused) throws Exception {
 		for (TestVisitor visitor : visitors) {
 			visitor.describe(text, block, isFocused);
 		}
 	}
 
 	@Override
-	public void context(String text, ExecutableBlock block, boolean isFocused) {
+	public void context(String text, ExecutableBlock block, boolean isFocused) throws Exception {
 		for (TestVisitor visitor : visitors) {
 			visitor.context(text, block, isFocused);
 		}
 	}
 
 	@Override
-	public void beforeEach(ExecutableBlock block) {
+	public void beforeEach(ExecutableBlock block) throws Exception {
 		for (TestVisitor visitor : visitors) {
 			visitor.beforeEach(block);
 		}
 	}
 
 	@Override
-	public void justBeforeEach(ExecutableBlock block) {
+	public void justBeforeEach(ExecutableBlock block) throws Exception {
 		for (TestVisitor visitor : visitors) {
 			visitor.justBeforeEach(block);
 		}
@@ -88,7 +88,7 @@ public void it(String text, ExecutableBlock block, boolean isFocused) {
 	}
 
 	@Override
-	public void afterEach(ExecutableBlock block) {
+	public void afterEach(ExecutableBlock block) throws Exception {
 		for (TestVisitor visitor : visitors) {
 			visitor.afterEach(block);
 		}

diff --git a/src/main/java/impl/com/github/paulcwarren/ginkgo4j/chains/SpecsCollector.java b/src/main/java/impl/com/github/paulcwarren/ginkgo4j/chains/SpecsCollector.java
@@ -19,36 +19,32 @@ public List<Spec> getSpecs() {
 		return specs;
 	}
 
-	public void describe(String text, ExecutableBlock block, boolean isFocused) {
+	public void describe(String text, ExecutableBlock block, boolean isFocused) throws Exception {
 		text = IdBuilder.id(text);
 
 		context.push(text);
 		try {
 			block.invoke();
-		} catch (Throwable e) {
-			e.printStackTrace();
 		} finally {
 			context.pop();
 		}
 	}
 
-	public void context(String text, ExecutableBlock block, boolean isFocused) {
+	public void context(String text, ExecutableBlock block, boolean isFocused) throws Exception {
 		text = IdBuilder.id(text);
 
 		context.push(text);
 		try {
 			block.invoke();
-		} catch (Throwable e) {
-			e.printStackTrace();
 		} finally {
 			context.pop();
 		}
 	}
 
-	public void beforeEach(ExecutableBlock block) {
+	public void beforeEach(ExecutableBlock block) throws Exception {
 	}
 
-	public void justBeforeEach(ExecutableBlock block) {
+	public void justBeforeEach(ExecutableBlock block) throws Exception {
 	}
 
 	public void it(String text, ExecutableBlock block, boolean isFocused) {
@@ -58,7 +54,7 @@ public void it(String text, ExecutableBlock block, boolean isFocused) {
 		specs.add(spec);
 	}
 
-	public void afterEach(ExecutableBlock block) {
+	public void afterEach(ExecutableBlock block) throws Exception {
 	}
 
 	@Override

diff --git a/src/test/java/com/github/paulcwarren/ginkgo4j/ExampleTests.java b/src/test/java/com/github/paulcwarren/ginkgo4j/ExampleTests.java
@@ -14,22 +14,33 @@
 import junit.framework.Assert;
 
 @Ignore
+@SuppressWarnings("deprecation")
 @RunWith(Ginkgo4jRunner.class)
 //@Ginkgo4jConfiguration(threads=1)
 public class ExampleTests {
 
-	private Example example;
+	private  String something;
 
+	private Example example;
 	{
-		Describe("", () -> {
-			Context("", () -> {
-				It("", () -> {
+		Describe("describe", () -> {
+			Context("context", () -> {
+				something.length();
+				It("a test", () -> {
 					assertThat(true, is(true));
 				});
 			});
-		}); 		
-
-		Describe("A describe", () -> {
+		});
+
+        Describe("", () -> {
+            Context("", () -> {
+                It("", () -> {
+                    assertThat(true, is(true));
+                });
+            });
+        });
+
+        Describe("A describe", () -> {
 
 			BeforeEach(() -> {
 				example = new Example();