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 never asserts when given wrong type (boolean) #186

Open
kittaakos opened this issue Mar 21, 2017 · 6 comments
Open

rejectedWith never asserts when given wrong type (boolean) #186

kittaakos opened this issue Mar 21, 2017 · 6 comments
Labels

Comments

@kittaakos
Copy link

kittaakos commented Mar 21, 2017

Hey there,

I am experiencing an odd behavior when checking promise rejection. It seems, that the rejected boolean value is simply ignored. I would expect three failing test cases including the rejects as promised (boolean) one but I get only two. (Please see console output below). Maybe I am doing something wrong. Probably, one can easily spot the problem. I look forward to any kind of feedbacks. Cheers.

Here is my setup:

/src/foo.spec.js:

const chai = require("chai");
const expect = chai.expect;

before(function() {
    chai.config.showDiff = true;
    chai.config.includeStack = true;
    chai.use(require("chai-as-promised"));
});

describe('bar', function() {

    it('resolves as promised', function () {
        // That is fine.
        return expect(Promise.resolve('woof')).to.eventually.equal('woof');
    });

    it('rejects as promised (string)', function () {
        // That is reported as an error. Work fine.
        return expect(Promise.reject('caw')).to.eventually.be.rejectedWith('caw2');
    });

    it('rejects as promised (boolean)', function () {
        // !!! That is also fine but should *not* be.
        return expect(Promise.reject(false)).to.eventually.be.rejectedWith(true);
    });

    it('rejects as promised (Error)', function () {
        // That fails as well and reported as an error which seems to be reasonable.
        return expect(Promise.reject(false)).to.eventually.be.rejectedWith(Error);
    });

});

test/mocha.opts:

--reporter spec
--watch-extensions js
src/**/*.spec.js

package.json:

{
  "name": "tmp",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "./node_modules/.bin/mocha --watch"
  },
  "author": "",
  "license": "ISC",
  "devDependencies": {
    "chai": "^3.5.0",
    "chai-as-promised": "^6.0.0",
    "mocha": "^3.2.0"
  }
}

Console output when running the tests:

bar
  ✓ resolves as promised
  1) rejects as promised (string)
  ✓ rejects as promised (boolean)
  2) rejects as promised (Error)


2 passing (56ms)
2 failing

1) bar rejects as promised (string):
   AssertionError: expected promise to be rejected with an error including 'caw2' but got 'caw'
    at node_modules/chai-as-promised/lib/chai-as-promised.js:212:30
    at runMicrotasksCallback (internal/process/next_tick.js:58:5)
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)

2) bar rejects as promised (Error):
   AssertionError: expected promise to be rejected with 'Error' but it was rejected with false
    at node_modules/chai-as-promised/lib/chai-as-promised.js:203:30
    at runMicrotasksCallback (internal/process/next_tick.js:58:5)
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)
@domenic
Copy link
Collaborator

domenic commented Mar 21, 2017

Can you edit your example to be in JavaScript? I'm not interested in debugging other languages.

@kittaakos
Copy link
Author

@domenic I have updated my initial comment. Please let me know if I need to change anything else. Thanks in advance!

@keithamus
Copy link
Member

Firstly, I'm not sure of the intent of your code, but you should absolutely be rejecting with only Error instances, or subclass instances such as TypeError. rejecting Promises with strings or booleans is an anti-pattern.

Secondly, the reason this is not working correctly is because rejectedWith has a few type signatures, but none that support boolean properties. It's a bug that it is passing with a boolean, but it is also a misuse of the rejectedWith API to pass a boolean. The signatures are rejectedWith(ErrorConstructor), rejectedWith(ErrorConstructor, 'stringMessage'), rejectedWith('stringMessage'), rejectedWith(/regExpMessage/). Coincidentally, with a boolean as the first argument, the function manages to fall through all of the control flow logic we have.

For a fix to this, I suggest we ensure that errorLike in the assertion (here) is checked to ensure it is one of the expected types (function, string, regexp), and if it isn't then we should throw some kind of error describing how to use the rejectedWith function correctly.

@keithamus keithamus added the bug label Mar 21, 2017
@keithamus keithamus changed the title rejectedWith with boolean values rejectedWith never asserts when given wrong type (boolean) Mar 21, 2017
@kittaakos
Copy link
Author

rejecting Promises with strings or booleans is an anti-pattern.

I was not aware of this. Thanks for the explanation on this ticket!

@domenic
Copy link
Collaborator

domenic commented Apr 4, 2017

@keithamus Thanks so much for your help here and for digging into things.

Does Chai throw such an error if you pass it a boolean to .throws()?

@keithamus
Copy link
Member

@domenic Short answer: no. Longer answer: right now there is virtually no type checking inside chai. We are slowly doing this, and hopefully this will hopefully change to be enforced in future (see chaijs/chai#585). We can put this as a ticket in Chai to get it in for Chai@4 if you'd like us to fall in line with this plugin.

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

No branches or pull requests

3 participants