Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rejectedWith false positives when passed undefined #123

Closed
mgartner opened this issue Aug 28, 2015 · 8 comments
Closed

rejectedWith false positives when passed undefined #123

mgartner opened this issue Aug 28, 2015 · 8 comments

Comments

@mgartner
Copy link

If I pass undefined to rejectedWith() I get a dangerous false-positive test.

For example:

var Errors = require('./errors');

expect(SomePromise('hello')).to.be.rejectedWith(Errors.SomeError);

If I fat-finger Errors.SomeError, to something like Errors.SomeErrro, it will be undefined, and the expectation passes, when it shouldn't. This is gives the potential for false-positives which I feel are very dangerous.

Are there any better ways of dealing with this?

@brokenbot
Copy link

Unfortunately I'm finding this issue is true with any undefined condition, all these test pass just fine

return Promise.reject('rejected').should.be.fullfilled
return Promise.reject('rejected').should.eventually.be.randomundefinedthing
return Promise.resolve('resolved').should.be.reject

@Merott
Copy link

Merott commented Oct 19, 2015

rejectedWith also passes with a false positive for integer values, e.g:

return Promise.reject(2).should.be.rejectedWith(1);
return Promise.resolve().should.be.rejectedWith(1);

EDIT

On further investigation, I've realised that the false-positive issue is a lot more widespread than initially thought. rejectedWith also passed with a false positive when dealing with booleans:

return Promise.reject(false).should.be.rejectedWith(true);  // passes

I'm trying to fix this in my own fork, hopefully to open a PR. However, I don't understand the logic behind some of the assertions/expectations. For example, the following is a PASS, and there is a test for it, to ensure it stays that way.

return Promise.reject('a long error string').should.be.rejectedWith('error');  // passes

I'd expect the above to fail. If I wanted to test partial strings, I'd use a regex, like so:

return Promise.reject('a long error string').should.be.rejectedWith(/error/);

Can anyone explain?

@domenic
Copy link
Collaborator

domenic commented Oct 19, 2015

I wonder if this is connected to #126.

For example, the following is a PASS, and there is a test for it, to ensure it stays that way.

Chai matches error strings based on substring matching, so we do the same in Chai as Promised.

@domenic
Copy link
Collaborator

domenic commented Oct 19, 2015

The best fix for this and many other issues would be that described in #47 (comment)...

@keithamus
Copy link
Member

Some early work for this has been done in chaijs/chai#470 but it needs some work to get it to a useful point. PRs welcome 😄

@evonsdesigns
Copy link

After stumbling upon this issue, it seems like the problem is just with rejectedWith. That is, if you write out its logical equivalent,

return expect(somePromise).to.be.rejected.and.to.eventually.equal(new Error('some error message'));

...it works just fine.

@mattS-H
Copy link

mattS-H commented Jul 22, 2016

it also fails to behave correctly with empty strings and single characters:
return expect(result).to.be.rejectedWith("a")

@domenic
Copy link
Collaborator

domenic commented Aug 5, 2016

I guess this is a dupe of #56.

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

No branches or pull requests

7 participants