Merge pull request #26 from greghaskins/aftereach-always-runs

Bugfix: make sure afterEach always runs

src/main/java/com/greghaskins/spectrum/AfterEachBlock.java

@@ -0,0 +1,36 @@
+package com.greghaskins.spectrum;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import com.greghaskins.spectrum.Spectrum.Block;
+
+class AfterEachBlock implements Block {
+
+	private final List<Block> blocks;
+
+	public AfterEachBlock() {
+		this.blocks = new ArrayList<Block>();
+	}
+
+	@Override
+	public void run() throws Throwable {
+		runAllBlocksInReverseOrder(this.blocks.size() - 1);
+	}
+
+	private void runAllBlocksInReverseOrder(final int index) throws Throwable {
+		if (index < 0) {
+			return;
+		}
+		try {
+			this.blocks.get(index).run();
+		} finally {
+			runAllBlocksInReverseOrder(index - 1);
+		}
+	}
+
+	public void addBlock(final Block block) {
+		this.blocks.add(block);
+	}
+
+}

src/main/java/com/greghaskins/spectrum/Suite.java

@@ -18,7 +18,7 @@ class Suite implements Parent, Child {
 	private final CompositeBlock afterAll = new CompositeBlock();
 
 	private final CompositeBlock beforeEach = new CompositeBlock();
-	private final CompositeBlock afterEach = new CompositeBlock();
+	private final AfterEachBlock afterEach = new AfterEachBlock();
 
 	private final List<Child> children = new ArrayList<Child>();
 	private final Set<Child> focusedChildren = new HashSet<Child>();
@@ -47,8 +47,8 @@ public Suite addSuite(final String name) {
 	public Spec addSpec(final String name, final Block block) {
 		final Description specDescription = Description.createTestDescription(this.description.getClassName(), name);
 
-		final CompositeBlock specBlockInContext = new CompositeBlock(
-				Arrays.asList(this.beforeAll, this.beforeEach, block, this.afterEach));
+		final Block specBlockInContext = new TryFinallyBlock(new CompositeBlock(
+				Arrays.asList(this.beforeAll, this.beforeEach, block)), this.afterEach);
 
 		final Spec spec = new Spec(specDescription, specBlockInContext, this);
 		addChild(spec);

src/main/java/com/greghaskins/spectrum/TryFinallyBlock.java

@@ -0,0 +1,25 @@
+package com.greghaskins.spectrum;
+
+import com.greghaskins.spectrum.Spectrum.Block;
+
+class TryFinallyBlock implements Block {
+
+    private final Block tryBlock;
+    private final Block finallyBlock;
+
+    public TryFinallyBlock(final Block tryBlock, final Block finallyBlock) {
+        this.tryBlock = tryBlock;
+        this.finallyBlock = finallyBlock;
+    }
+
+    @Override
+    public void run() throws Throwable {
+        try {
+            tryBlock.run();
+        } finally {
+            finallyBlock.run();
+        }
+
+    }
+
+}

src/test/java/specs/FixturesSpec.java

@@ -10,6 +10,7 @@
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
 
@@ -274,6 +275,54 @@ public class FixturesSpec {{
 
     });
 
+	describe("afterEach blocks", () -> {
+
+		describe("when a spec explodes", () -> {
+
+			it("still run", () -> {
+				final Result result = SpectrumRunner.run(getSuiteWithExplodingSpec());
+				assertThat(result.getFailureCount(), is(1));
+				assertThat(result.getFailures().get(0).getMessage(), containsString("boom"));
+			});
+
+		});
+
+		describe("when another afterEach explodes", () -> {
+
+			it("still run, too", () -> {
+				final Result result = SpectrumRunner.run(getSuiteWithExplodingAndNonExplodingAfterEach());
+				assertThat(result.getFailureCount(), is(1));
+				assertThat(result.getFailures().get(0).getMessage(), containsString("boom"));
+			});
+
+		});
+
+		final ArrayList<String> items = new ArrayList<>();
+
+		describe("in multiples", () -> {
+
+			it("run in reverse order", () -> {
+				assertThat(items, hasSize(0));
+			});
+
+			afterEach(() -> {
+				items.add("after1");
+			});
+			afterEach(() -> {
+				items.add("after2");
+			});
+			afterEach(() -> {
+				items.add("after3");
+			});
+
+		});
+
+		it("run in reverse declaration order", () -> {
+			assertThat(items, contains("after3", "after2", "after1"));
+		});
+
+	});
+
 }
 
 private static Class<?> getSpecWithExplodingBeforeEach(){
@@ -375,11 +424,70 @@ class Spec {{
 
                 });
 
-            });
-        });
+			});
+		});
 
-    }}
-    return Spec.class;
+	}}
+	return Spec.class;
+}
+
+private static Class<?> getSuiteWithExplodingSpec(){
+
+	class Suite {{
+		describe("suite with exploding spec", () -> {
+
+			final ArrayList<String> items = new ArrayList<>();
+
+			describe("boom", () -> {
+				it("explodes", () -> {
+					items.add("foo");
+					throw new Exception("boom");
+				});
+
+				afterEach(()->{
+					items.clear();
+				});
+			});
+
+			it("should still run afterEach blocks", () -> {
+				assertThat(items, hasSize(0));
+			});
+
+
+		});
+	}}
+	return Suite.class;
+}
+
+private static Class<?> getSuiteWithExplodingAndNonExplodingAfterEach(){
+
+	class Suite {{
+		describe("suite with exploding spec", () -> {
+
+			final ArrayList<String> items = new ArrayList<>();
+
+			describe("boom", () -> {
+				it("explodes", () -> {
+					items.add("foo");
+				});
+
+				afterEach(() -> {
+					throw new Exception("boom");
+				});
+
+				afterEach(()->{
+					items.clear();
+				});
+			});
+
+			it("should still run afterEach blocks", () -> {
+				assertThat(items, hasSize(0));
+			});
+
+
+		});
+	}}
+	return Suite.class;
 }
 
 }