From 40acffb2f0d3e9b652a36ec649844707cd334c85 Mon Sep 17 00:00:00 2001
From: Lucien Greathouse <me@lpghatguy.com>
Date: Fri, 14 Sep 2018 11:31:24 -0700
Subject: [PATCH] Improve robustness of Promise.all tests

---
 lib/init.lua      | 36 +++++++++++++++++++++++++++---------
 lib/init.spec.lua | 28 ++++++++++++++++++----------
 2 files changed, 45 insertions(+), 19 deletions(-)

diff --git a/lib/init.lua b/lib/init.lua
index 1054415..641e667 100644
--- a/lib/init.lua
+++ b/lib/init.lua
@@ -167,7 +167,7 @@ end
 ]]
 function Promise.all(promises)
 	if type(promises) ~= "table" then
-		error("Please pass an array of promises or values to Promise.all", 2)
+		error("Please pass a list of promises to Promise.all", 2)
 	end
 
 	-- If there are no values then return an already resolved promise.
@@ -208,15 +208,16 @@ function Promise.all(promises)
 
 		-- We can assume the values inside `promises` are all promises since we checked above.
 		for i = 1, #promises do
-			promises[i]:andThen(function(...)
-				resolveOne(i, ...)
-			end)
-			:catch(function(...)
-				-- Only reject if this promise is unrejected.
-				if not rejected then
-					reject(...)
+			promises[i]:andThen(
+				function(...)
+					resolveOne(i, ...)
+				end,
+				function(...)
+					if not rejected then
+						reject(...)
+					end
 				end
-			end)
+			)
 		end
 	end)
 end
@@ -314,6 +315,23 @@ function Promise:await()
 	end
 end
 
+--[[
+	Intended for use in tests.
+
+	Similar to await(), but instead of yielding if the promise is unresolved,
+	_unwrap will throw. This indicates an assumption that a promise has
+	resolved.
+]]
+function Promise:_unwrap()
+	if self._status == Promise.Status.Started then
+		error("Promise has not resolved or rejected.", 2)
+	end
+
+	local success = self._status == Promise.Status.Resolved
+
+	return success, unpack(self._values, 1, self._valuesLength)
+end
+
 function Promise:_resolve(...)
 	if self._status ~= Promise.Status.Started then
 		return
diff --git a/lib/init.spec.lua b/lib/init.spec.lua
index c153375..b1c1da0 100644
--- a/lib/init.spec.lua
+++ b/lib/init.spec.lua
@@ -272,14 +272,14 @@ return function()
 			end).to.throw()
 		end)
 
-		it("should resolve instantly with an empty table if given no values", function()
+		it("should resolve instantly with an empty table if given no promises", function()
 			local promise = Promise.all({})
-			local success, value = promise:await()
+			local success, value = promise:_unwrap()
 
 			expect(success).to.equal(true)
 			expect(promise:getStatus()).to.equal(Promise.Status.Resolved)
 			expect(value).to.be.a("table")
-			expect(#value).to.equal(0)
+			expect(next(value)).to.equal(nil)
 		end)
 
 		it("should error if given non-promise values", function()
@@ -289,23 +289,31 @@ return function()
 		end)
 
 		it("should wait for all promises to be resolved and return their values", function()
+			local resolveFunctions = {}
+
 			local promises = {
 				Promise.new(function(resolve)
-					resolve(1)
+					table.insert(resolveFunctions, {resolve, 1})
 				end),
 				Promise.new(function(resolve)
-					resolve("A string")
+					table.insert(resolveFunctions, {resolve, "A string"})
 				end),
 				Promise.new(function(resolve)
-					resolve(nil)
+					table.insert(resolveFunctions, {resolve, nil})
 				end),
 				Promise.new(function(resolve)
-					resolve(false)
-				end)
+					table.insert(resolveFunctions, {resolve, false})
+				end),
 			}
 
-			local promise = Promise.all(promises)
-			local success, resolved = promise:await()
+			local combinedPromise = Promise.all(promises)
+
+			for _, resolve in ipairs(resolveFunctions) do
+				expect(combinedPromise:getStatus()).to.equal(Promise.Status.Started)
+				resolve[1](resolve[2])
+			end
+
+			local success, resolved = combinedPromise:_unwrap()
 
 			expect(success).to.equal(true)
 			expect(resolved).to.be.a("table")