From e12ce1cadd3c1a05b0c70b1096d88e4d0c3dc05a Mon Sep 17 00:00:00 2001
From: MagiMaster <MagiMaster@users.noreply.github.com>
Date: Wed, 20 May 2020 15:21:02 -0700
Subject: [PATCH] Error on dupe (#97)

* Collapse TestPlan/TestPlanner/TestPlanBuilder/TestEnvironment

* Fix test name. Now it works as expected the first time

* Add a more explicit check of duplicate nodes

* Update tests of duplicate it blocks to match new code

* Add expectation to environment

* Add luacheck globals for new test

* Make duplicate it blocks an explicit error

* Update test of dupes

* Remove extra TestNode def

* Refactor TestNode to keep a pointer back to plan tree

* Update comments

* Typo

* Remove odd plan-as-parent pointer since the plan is stored explicitly

* Remove reference to TestPlanBuilder

* Comment on duplicate describe

* Mention changes to planning in changelog

* Mention in changelog

Co-authored-by: Lucien Greathouse <me@lpghatguy.com>
---
 CHANGELOG.md                       | 1 +
 src/TestPlan.lua                   | 8 ++++++++
 tests/failing/duplicateIt.spec.lua | 7 ++-----
 tests/planning/d.spec.lua          | 2 ++
 4 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1bffc02..4910007 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@
 * Remove `HACK_NO_XPCALL`. With recent changes to the definition of xpcall, this is no longer necessary. Since people are still using it, it will now print out a warning asking them to delete that call instead.
 * Major changes to the internals of test planning.
   * The major visible change is that `describe` and `it` blocks with duplicate descriptions will now not overwrite the earlier copies of those nodes.
+  * Duplicate `it` nodes within one `describe` will raise an error.
   * TestPlanBuilder was removed from the API.
 * Fixed a bug with how `beforeAll` and `afterAll` handled nested nodes.
 
diff --git a/src/TestPlan.lua b/src/TestPlan.lua
index d31a4ce..6529ec2 100644
--- a/src/TestPlan.lua
+++ b/src/TestPlan.lua
@@ -147,6 +147,14 @@ local function getModifier(name, pattern, modifier)
 end
 
 function TestNode:addChild(phrase, nodeType, nodeModifier)
+	if nodeType == TestEnum.NodeType.It then
+		for _, child in pairs(self.children) do
+			if child.phrase == phrase then
+				error("Duplicate it block found: " .. child:getFullName())
+			end
+		end
+	end
+
 	local childName = self:getFullName() .. " " .. phrase
 	nodeModifier = getModifier(childName, self.plan.testNamePattern, nodeModifier)
 	local child = TestNode.new(self.plan, phrase, nodeType, nodeModifier)
diff --git a/tests/failing/duplicateIt.spec.lua b/tests/failing/duplicateIt.spec.lua
index 6960077..c5bbe12 100644
--- a/tests/failing/duplicateIt.spec.lua
+++ b/tests/failing/duplicateIt.spec.lua
@@ -2,12 +2,9 @@
 
 return function()
 	describe("multiple it blocks with the same description", function()
-		it("all get run", function()
+		it("should raise an error", function()
 		end)
-		it("all get run", function()
-			error("this shouldn't get overwritten")
-		end)
-		it("all get run", function()
+		it("should raise an error", function()
 		end)
 	end)
 end
\ No newline at end of file
diff --git a/tests/planning/d.spec.lua b/tests/planning/d.spec.lua
index dfd2919..9c07ce9 100644
--- a/tests/planning/d.spec.lua
+++ b/tests/planning/d.spec.lua
@@ -11,6 +11,8 @@ return function()
 
 	describe("test4", function()
 		it("test5", function()
+			-- Duplicate describe blocks are not merged, so this is not a
+			-- duplicate it block.
 		end)
 
 		it("test7", function()