Take over the require cache within a test run

CHANGELOG.md

@@ -1,6 +1,12 @@
 # TestEZ Changelog
 
 ## Unreleased Changes
+* Remove the lifecycle hooks from the session tree. This prevents the `[?]` spam from the reporter not recognizing these nodes.
+* Change the way errors are collected to call tostring on them before further processing.
+  * Luau allows non-string errors, but not concatenating non-strings or passing non-strings to `debug.traceback` as a message, so TestRunner needs to do that step. This is a temporary fix as the better solution would be to retain the error in object form for as long as possible to give the reporter more to work with.
+  * This also makes a slight change to what's in the traceback to eliminate the unnecessary line mentioning the error collection function.
+* Add a deprecation notice for uses of extraEnvironment.
+* Guarantee that `init.spec.lua` will run before any `it` or `describe` blocks in the folder under it.
 
 ## 0.3.1 (2020-06-22)
 * Further simplify `beforeAll` handling.

src/TestBootstrap.lua

@@ -54,7 +54,7 @@ function TestBootstrap:getModulesImpl(root, modules, current)
 	current = current or root
 
 	if isSpecScript(current) then
-		local method = require(current)
+		local method = debug.loadmodule(current)
 		local path = getPath(current, root)
 		local pathString = toStringPath(path)
 

src/TestPlan.lua

@@ -8,12 +8,19 @@
 local TestEnum = require(script.Parent.TestEnum)
 local Expectation = require(script.Parent.Expectation)
 
-local function newEnvironment(currentNode, extraEnvironment)
+local REQUIRE_CACHE_KEY = {}
+
+local function newEnvironment(parentEnvironment, currentNode, extraEnvironment)
 	local env = {}
 
+	-- All nodes in a tree currently share a cache
+	local requireCache = parentEnvironment and parentEnvironment[REQUIRE_CACHE_KEY] or {}
+	env[REQUIRE_CACHE_KEY] = requireCache
+
 	if extraEnvironment then
 		if type(extraEnvironment) ~= "table" then
-			error(("Bad argument #2 to newEnvironment. Expected table, got %s"):format(
+			error(("Bad argument #3 to newEnvironment. Expected table, got %s"):format(
+
 				typeof(extraEnvironment)), 2)
 		end
 
@@ -25,9 +32,6 @@ local function newEnvironment(currentNode, extraEnvironment)
 	local function addChild(phrase, callback, nodeType, nodeModifier)
 		local node = currentNode:addChild(phrase, nodeType, nodeModifier)
 		node.callback = callback
-		if nodeType == TestEnum.NodeType.Describe then
-			node:expand()
-		end
 		return node
 	end
 
@@ -107,6 +111,17 @@ local function newEnvironment(currentNode, extraEnvironment)
 
 	env.expect = Expectation.new
 
+	function env.require(module)
+		if not requireCache[module] then
+			local chunk = debug.loadmodule(module)
+			local originalEnv = getfenv(chunk)
+			local newEnv = setmetatable({require = env.require}, {__index = originalEnv})
+			setfenv(chunk, newEnv)
+			requireCache[module] = chunk()
+		end
+		return requireCache[module]
+	end
+
 	return env
 end
 
@@ -118,7 +133,7 @@ TestNode.__index = TestNode
 	and the type of node it is are required. The modifier is optional and will
 	be None if left blank.
 ]]
-function TestNode.new(plan, phrase, nodeType, nodeModifier)
+function TestNode.new(parent, plan, phrase, nodeType, nodeModifier)
 	nodeModifier = nodeModifier or TestEnum.NodeModifier.None
 
 	local node = {
@@ -131,7 +146,8 @@ function TestNode.new(plan, phrase, nodeType, nodeModifier)
 		parent = nil,
 	}
 
-	node.environment = newEnvironment(node, plan.extraEnvironment)
+	local parentEnvironment = parent and parent.Environment
+	node.environment = newEnvironment(parentEnvironment, node, plan.extraEnvironment)
 	return setmetatable(node, TestNode)
 end
 
@@ -157,7 +173,7 @@ function TestNode:addChild(phrase, nodeType, nodeModifier)
 
 	local childName = self:getFullName() .. " " .. phrase
 	nodeModifier = getModifier(childName, self.plan.testNamePattern, nodeModifier)
-	local child = TestNode.new(self.plan, phrase, nodeType, nodeModifier)
+	local child = self:new(self.plan, phrase, nodeType, nodeModifier)
 	child.parent = self
 	table.insert(self.children, child)
 	return child
@@ -177,22 +193,35 @@ function TestNode:getFullName()
 end
 
 --[[
-	Expand a node by setting its callback environment and then calling it. Any
-	further it and describe calls within the callback will be added to the tree.
+	Expand a node by setting its callback environment and then calling it. Only
+	expands this one node.
 ]]
 function TestNode:expand()
+	if not self.callback then
+		return
+	end
+
 	local originalEnv = getfenv(self.callback)
 	local callbackEnv = setmetatable({}, { __index = originalEnv })
 	for key, value in pairs(self.environment) do
 		callbackEnv[key] = value
 	end
 	setfenv(self.callback, callbackEnv)
 
-	local success, result = xpcall(self.callback, debug.traceback)
+	local success, result = xpcall(self.callback, function(message)
+		return debug.traceback(tostring(message), 2)
+	end)
 
 	if not success then
 		self.loadError = result
 	end
+
+	if typeof(result) == "function" then
+		success, result = xpcall(result, debug.traceback)
+		if not success then
+			self.loadError = result
+		end
+	end
 end
 
 local TestPlan = {}
@@ -202,6 +231,14 @@ TestPlan.__index = TestPlan
 	Create a new, empty TestPlan.
 ]]
 function TestPlan.new(testNamePattern, extraEnvironment)
+	if extraEnvironment and next(extraEnvironment) then
+		warn(
+			"extraEnvironment is deprecated and will be removed in the near future. " ..
+			"Please use an init.spec.lua file and the test context to pass in anything " ..
+			"that is currently in extraEnvironment."
+		)
+	end
+
 	local plan = {
 		children = {},
 		testNamePattern = testNamePattern,
@@ -216,7 +253,7 @@ end
 ]]
 function TestPlan:addChild(phrase, nodeType, nodeModifier)
 	nodeModifier = getModifier(phrase, self.testNamePattern, nodeModifier)
-	local child = TestNode.new(self, phrase, nodeType, nodeModifier)
+	local child = TestNode.new(nil, self, phrase, nodeType, nodeModifier)
 	table.insert(self.children, child)
 	return child
 end
@@ -245,7 +282,17 @@ function TestPlan:addRoot(path, method)
 	end
 
 	curNode.callback = method
-	curNode:expand()
+end
+
+--[[
+	Expands all describe nodes, leaving the plan in a runnable state.
+]]
+function TestPlan:finalize()
+	self:visitAllNodes(function(node)
+		if node.type == TestEnum.NodeType.Describe then
+			node:expand()
+		end
+	end)
 end
 
 --[[

src/TestPlanner.lua

@@ -34,6 +34,7 @@ function TestPlanner.createPlan(modulesList, testNamePattern, extraEnvironment)
 		plan:addRoot(module.path, module.method)
 	end
 
+	plan:finalize()
 	return plan
 end
 

src/TestRunner.lua

@@ -67,7 +67,7 @@ function TestRunner.runPlanNode(session, planNode, lifecycleHooks)
 			end
 
 			success = false
-			errorMessage = messagePrefix .. message .. "\n" .. debug.traceback()
+			errorMessage = messagePrefix .. debug.traceback(tostring(message), 2)
 		end
 
 		local context = session:getContext()
@@ -77,7 +77,7 @@ function TestRunner.runPlanNode(session, planNode, lifecycleHooks)
 				callback(context)
 			end,
 			function(message)
-				return messagePrefix .. message .. "\n" .. debug.traceback()
+				return messagePrefix .. debug.traceback(tostring(message), 2)
 			end
 		)
 
@@ -134,9 +134,8 @@ function TestRunner.runPlanNode(session, planNode, lifecycleHooks)
 
 	if not halt then
 		for _, childPlanNode in ipairs(planNode.children) do
-			session:pushNode(childPlanNode)
-
 			if childPlanNode.type == TestEnum.NodeType.It then
+				session:pushNode(childPlanNode)
 				if session:shouldSkip() then
 					session:setSkipped()
 				else
@@ -148,7 +147,9 @@ function TestRunner.runPlanNode(session, planNode, lifecycleHooks)
 						session:setError(errorMessage)
 					end
 				end
+				session:popNode()
 			elseif childPlanNode.type == TestEnum.NodeType.Describe then
+				session:pushNode(childPlanNode)
 				TestRunner.runPlanNode(session, childPlanNode, lifecycleHooks)
 
 				-- Did we have an error trying build a test plan?
@@ -158,9 +159,8 @@ function TestRunner.runPlanNode(session, planNode, lifecycleHooks)
 				else
 					session:setStatusFromChildren()
 				end
+				session:popNode()
 			end
-
-			session:popNode()
 		end
 	end
 

tests/expandOrder.lua

@@ -0,0 +1,88 @@
+-- luacheck: globals it beforeAll afterAll
+
+local TestEZ = require(script.Parent.Parent.TestEZ)
+
+return {
+	["init.spec.lua is run before children are expanded"] = function()
+		local initialized = false
+
+		local plan = TestEZ.TestPlanner.createPlan({
+			{
+				method = function()
+					assert(initialized, "init.spec was not called before bar.spec")
+				end,
+				path = {'bar.spec', 'foo'},
+				pathStringForSorting = "foo bar.spec",
+			},
+			{
+				method = function()
+					initialized = true
+				end,
+				path = {'foo'},
+				pathStringForSorting = "foo",
+			},
+		})
+
+		local results = TestEZ.TestRunner.runPlan(plan)
+		assert(#results.errors == 0, "init test failed: " .. tostring(results.errors[1]))
+	end,
+	["init.spec.lua afterAll can correctly undo changes"] = function()
+		local initialized = false
+
+		local plan = TestEZ.TestPlanner.createPlan({
+			{
+				method = function()
+					it("A", function()
+						assert(not initialized, "initialized was true in foo/a.spec")
+					end)
+				end,
+				path = {'a.spec', 'foo'},
+				pathStringForSorting = "foo a.spec",
+			},
+			{
+				method = function()
+					it("B", function()
+						assert(initialized, "initialized was false in foo/bar/b.spec")
+					end)
+				end,
+				path = {'b.spec', 'bar', 'foo'},
+				pathStringForSorting = "foo bar b.spec",
+			},
+			{
+				method = function()
+					beforeAll(function()
+						initialized = true
+					end)
+
+					afterAll(function()
+						initialized = false
+					end)
+				end,
+				path = {'bar', 'foo'},
+				pathStringForSorting = "foo bar",
+			},
+			{
+				method = function()
+					it("C", function()
+						assert(initialized, "initialized was false in foo/bar/c.spec")
+					end)
+				end,
+				path = {'c.spec', 'bar', 'foo'},
+				pathStringForSorting = "foor bar c.spec",
+			},
+			{
+				method = function()
+					it("D", function()
+						assert(not initialized, "initialized was true in foo/d.spec")
+					end)
+				end,
+				path = {'d.spec', 'foo'},
+				pathStringForSorting = "foo d.spec",
+			},
+		})
+
+		local results = TestEZ.TestRunner.runPlan(plan)
+		assert(#results.errors == 0, "init test failed:\n" ..
+			table.concat(results.errors, "\n"))
+	end,
+}

tests/modules/a.lua

@@ -0,0 +1,3 @@
+local b = require(script.Parent.b)
+
+return b(1) -- 2

tests/modules/b.lua

@@ -0,0 +1,3 @@
+return function(x)
+	return x + 1
+end

tests/passing/requires.spec.lua

@@ -0,0 +1,17 @@
+-- luacheck: globals it expect
+
+return function()
+	local a = require(script.Parent.Parent.modules.a)
+	local b = require(script.Parent.Parent.modules.b)
+
+	it("requires should work properly", function()
+		expect(a).to.equal(2)
+		expect(b(2)).to.equal(3)
+	end)
+
+	it("require cache works under normal circumstances", function()
+		local b2 = require(script.Parent.Parent.modules.b)
+
+		expect(b).to.equal(b2)
+	end)
+end

tests/planning/init.spec.lua

@@ -0,0 +1,4 @@
+-- This should be added to the "planning" node of the tree instead of creating
+-- a new node.
+return function()
+end