Simplify beforeAll handling.

CHANGELOG.md

@@ -1,6 +1,10 @@
 # TestEZ Changelog
 
 ## Unreleased Changes
+* Further simplify `beforeAll` handling.
+  * `beforeAll` now runs on entering the block, rather than on the first `it` encountered after entering the block. The major difference for the moment is that a `beforeAll` will now run even if there are no `it` blocks under it, which is now consistent with how `afterAll` worked.
+  * `beforeAll` and `afterAll` now report errors by creating a dummy node in the results to contain the error. Previously, errors in `afterAll` were not reported.
+  * A failure in a `beforeAll` block will now halt all further test execution within its enclosing `describe` block except for any remaining `beforeAll` blocks and any `afterAll` blocks. Multiple `beforeAll` or `afterAll` blocks within one `describe` block should not count on running in any specific order. `afterAll` blocks should account for the possibility of a partially setup state when cleaning up.
 
 ## 0.3.0 (2020-06-12)
 * Remove the `try` node type.

src/LifecycleHooks.lua

@@ -61,44 +61,23 @@ function LifecycleHooks:pushHooksFrom(planNode)
 end
 
 --[[
-	Get all currently uncalled beforeAll hooks, and remove them from the stack.
+	Get the beforeAll hooks from the current level.
 ]]
-function LifecycleHooks:getPendingBeforeAllHooks()
-	local key = TestEnum.NodeType.BeforeAll
-	local hooks = {}
-
-	for _, level in ipairs(self._stack) do
-		for _, hook in ipairs(level[key]) do
-			table.insert(hooks, hook)
-		end
-		level[key] = {}
-	end
-
-	return hooks
+function LifecycleHooks:getBeforeAllHooks()
+	return self._stack[#self._stack][TestEnum.NodeType.BeforeAll]
 end
 
 --[[
-	Get all uncalled afterAll hooks from the back of the stack and remove them.
+	Get the afterAll hooks from the current level.
 ]]
 function LifecycleHooks:getAfterAllHooks()
-	local key = TestEnum.NodeType.AfterAll
-	local hooks = {}
-
-	local currentBack = self._stack[#self._stack]
-	if currentBack then
-		for _, hook in pairs(currentBack[key]) do
-			table.insert(hooks, hook)
-		end
-		currentBack[key] = {}
-	end
-
-	return hooks
+	return self._stack[#self._stack][TestEnum.NodeType.AfterAll]
 end
 
 function LifecycleHooks:_getHooksOfType(nodes, key)
 	local hooks = {}
 
-	for _, node in pairs(nodes) do
+	for _, node in ipairs(nodes) do
 		if node.type == key then
 			table.insert(hooks, node.callback)
 		end

src/TestRunner.lua

@@ -90,14 +90,7 @@ function TestRunner.runPlanNode(session, planNode, lifecycleHooks)
 		-- Errors can be set either via `error` propagating upwards or
 		-- by a test calling fail([message]).
 
-		for _, hook in pairs(lifecycleHooks:getPendingBeforeAllHooks()) do
-			local success, errorMessage = runCallback(hook, "beforeAll hook: ")
-			if not success then
-				return false, errorMessage
-			end
-		end
-
-		for _, hook in pairs(lifecycleHooks:getBeforeEachHooks()) do
+		for _, hook in ipairs(lifecycleHooks:getBeforeEachHooks()) do
 			local success, errorMessage = runCallback(hook, "beforeEach hook: ")
 			if not success then
 				return false, errorMessage
@@ -111,7 +104,7 @@ function TestRunner.runPlanNode(session, planNode, lifecycleHooks)
 			end
 		end
 
-		for _, hook in pairs(lifecycleHooks:getAfterEachHooks()) do
+		for _, hook in ipairs(lifecycleHooks:getAfterEachHooks()) do
 			local success, errorMessage = runCallback(hook, "afterEach hook: ")
 			if not success then
 				return false, errorMessage
@@ -123,40 +116,52 @@ function TestRunner.runPlanNode(session, planNode, lifecycleHooks)
 
 	lifecycleHooks:pushHooksFrom(planNode)
 
-	for _, childPlanNode in ipairs(planNode.children) do
-		session:pushNode(childPlanNode)
+	local halt = false
+	for _, hook in ipairs(lifecycleHooks:getBeforeAllHooks()) do
+		local success, errorMessage = runCallback(hook, "beforeAll hook: ")
+		if not success then
+			session:addDummyError("beforeAll", errorMessage)
+			halt = true
+		end
+	end
 
-		if childPlanNode.type == TestEnum.NodeType.It then
-			if session:shouldSkip() then
-				session:setSkipped()
-			else
-				local success, errorMessage = runNode(childPlanNode)
+	if not halt then
+		for _, childPlanNode in ipairs(planNode.children) do
+			session:pushNode(childPlanNode)
 
-				if success then
-					session:setSuccess()
+			if childPlanNode.type == TestEnum.NodeType.It then
+				if session:shouldSkip() then
+					session:setSkipped()
 				else
-					session:setError(errorMessage)
+					local success, errorMessage = runNode(childPlanNode)
+
+					if success then
+						session:setSuccess()
+					else
+						session:setError(errorMessage)
+					end
+				end
+			elseif childPlanNode.type == TestEnum.NodeType.Describe then
+				TestRunner.runPlanNode(session, childPlanNode, lifecycleHooks)
+
+				-- Did we have an error trying build a test plan?
+				if childPlanNode.loadError then
+					local message = "Error during planning: " .. childPlanNode.loadError
+					session:setError(message)
+				else
+					session:setStatusFromChildren()
 				end
 			end
-		elseif childPlanNode.type == TestEnum.NodeType.Describe then
-			TestRunner.runPlanNode(session, childPlanNode, lifecycleHooks)
-
-			-- Did we have an error trying build a test plan?
-			if childPlanNode.loadError then
-				local message = "Error during planning: " .. childPlanNode.loadError
-				session:setError(message)
-			else
-				session:setStatusFromChildren()
-			end
-		end
 
-		session:popNode()
+			session:popNode()
+		end
 	end
 
-	for _, hook in pairs(lifecycleHooks:getAfterAllHooks()) do
-		runCallback(hook, "afterAll hook: ")
-		-- errors in an afterAll hook are currently not caught
-		-- or attributed to a set of child nodes
+	for _, hook in ipairs(lifecycleHooks:getAfterAllHooks()) do
+		local success, errorMessage = runCallback(hook, "afterAll hook: ")
+		if not success then
+			session:addDummyError("afterAll", errorMessage)
+		end
 	end
 
 	lifecycleHooks:popHooks()

src/TestSession.lua

@@ -171,6 +171,16 @@ function TestSession:setError(message)
 	table.insert(last.errors, message)
 end
 
+--[[
+	Add a dummy node below the current one to hold an error message.
+]]
+function TestSession:addDummyError(phrase, message)
+	self:pushNode({type = TestEnum.NodeType.It, phrase = phrase})
+	self:setError(message)
+	self:popNode()
+	self.nodeStack[#self.nodeStack].status = TestEnum.TestStatus.Failure
+end
+
 --[[
 	Set the current node's status based on that of its children. If all children
 	are skipped, mark it as skipped. If any are fails, mark it as failed.

tests/lifecycleHooks.lua

@@ -142,6 +142,10 @@ return {
 					beforeAll(function()
 						insertLifecycleEvent("3 - beforeAll")
 					end)
+
+					afterAll(function()
+						insertLifecycleEvent("3 - afterAll")
+					end)
 				end)
 			end)
 
@@ -161,6 +165,8 @@ return {
 			"2 - test",
 			"2 - afterEach",
 			"1 - afterEach",
+			"3 - beforeAll",
+			"3 - afterAll",
 			"2 - afterAll",
 			"1 - beforeEach",
 			"1 - another test",
@@ -248,14 +254,6 @@ return {
 		failLifecycleCase("beforeAll")
 		failLifecycleCase("beforeEach")
 		failLifecycleCase("afterEach")
-		-- `afterAll` failure case is intentionally missing.
-		-- Currently it is not easy to attach an afterAll failure to
-		-- a particular set of childNodes without some refactoring.
-		-- Additionally, when jest afterAll hooks fail, it fails the test suite
-		-- and not any particular node which is a different flavor of failure
-		-- that TestEZ does not offer right now
-		-- Consult the following:
-		-- https://github.com/facebook/jest/issues/3266
-		-- https://github.com/facebook/jest/pull/5884
+		failLifecycleCase("afterAll")
 	end,
 }