Skip to content
This repository has been archived by the owner on Sep 14, 2024. It is now read-only.

Expectations should always fail tests #72

Open
LPGhatguy opened this issue Mar 24, 2020 · 4 comments
Open

Expectations should always fail tests #72

LPGhatguy opened this issue Mar 24, 2020 · 4 comments

Comments

@LPGhatguy
Copy link
Contributor

If an expectation is caught by pcall or xpcall it should still fail the containing test. This can help guarantee that if code runs, it is able to fail a test!

There may be some compatibility concerns for code using expect in... unexpected ways 😎

@amoss-roblox
Copy link

Do you have an example of where this would be useful?

@LPGhatguy
Copy link
Contributor Author

When testing code that involves passing a closure to another function, it'd be useful to ensure an expectation failure isn't caught by pcall.

Imagine we're writing a test like this:

it("should fail if yadda yadda yadda", function()
	local result = runAThing(function(x)
		expect(x).to.equal(1)
	end)

	expect(result).to.equal("success")
end)

It's possible that a change to runAThing would start using pcall on the passed function, perhaps incorrectly, swallowing a failing test assertion in the process.

One case where this comes up in practice is in promises, which explicitly turn errors into promise rejections, which are easy to accidentally ignore. You can see the large number of promise rejection warnings in the lua-apps test suite as an example of this. 😢

@amoss-roblox
Copy link

I'm a bit averse to making expect more magical, but I can see this being useful, and not any downsides at present.

It would be nice to have unhandled rejections show up in the reporter somehow, perhaps as a failed afterAll hook. In jest/node I have had this working by waiting for all promises to resolve before exiting the process (with a timeout). Might be more tricky for us though.

@LPGhatguy
Copy link
Contributor Author

I think that'd be doable. We'd need to track which test each promise came from to be useful, which would require some integration into the promise library, but would also add a lot of value.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants