chaijs/chai-things

Make Chai-Things work together with Chai-as-Promised

mancvso opened this issue · 9 comments

Hi there, I know there was an effort to make both extensions work together.
Currently I cannot use expressions such as

( promise ).should.eventually.all.have.property('property', 'value');

I get an AssertionError:

AssertionError: expected { Object (_bitField, _fulfillmentHandler0, ...) } to have a property 'length'
      at allMethod (/home/mancvso/dev/nfc/node-mongo-bare/node_modules/chai-things/lib/chai-things.js:171:37)
      at ctx.(anonymous function) [as property] (/home/mancvso/dev/nfc/node-mongo-bare/node_modules/chai/lib/chai/utils/overwriteMethod.js:48:33)
      at Context.<anonymous> (/home/mancvso/dev/nfc/node-mongo-bare/test/asana_test.js:31:73)
      at callFn (/home/mancvso/dev/nfc/node-mongo-bare/node_modules/grunt-mocha-cli/node_modules/mocha/lib/runnable.js:249:21)
      at Test.Runnable.run (/home/mancvso/dev/nfc/node-mongo-bare/node_modules/grunt-mocha-cli/node_modules/mocha/lib/runnable.js:242:7)
      at Runner.runTest (/home/mancvso/dev/nfc/node-mongo-bare/node_modules/grunt-mocha-cli/node_modules/mocha/lib/runner.js:373:10)
      at /home/mancvso/dev/nfc/node-mongo-bare/node_modules/grunt-mocha-cli/node_modules/mocha/lib/runner.js:451:12
      at next (/home/mancvso/dev/nfc/node-mongo-bare/node_modules/grunt-mocha-cli/node_modules/mocha/lib/runner.js:298:14)
      at /home/mancvso/dev/nfc/node-mongo-bare/node_modules/grunt-mocha-cli/node_modules/mocha/lib/runner.js:308:7
      at next (/home/mancvso/dev/nfc/node-mongo-bare/node_modules/grunt-mocha-cli/node_modules/mocha/lib/runner.js:246:23)
      at Object._onImmediate (/home/mancvso/dev/nfc/node-mongo-bare/node_modules/grunt-mocha-cli/node_modules/mocha/lib/runner.js:275:5)
      at processImmediate [as _immediateCallback] (timers.js:345:15)

But I can use

( promise ).should.eventually.be.an("array");

Working with bluebird promises.

If I am missing something obvious, maybe It could be a good idea to add the respective documentation.

If it is useful, the promise object actually has an array

{ _bitField: 805306368,
  _fulfillmentHandler0: undefined,
  _rejectionHandler0: undefined,
  _promise0: undefined,
  _receiver0: undefined,
  _settledValue: 
   [ { id: 15981206245217,
       name: '-test2-',
       completed: false,
       assignee_status: 'upcoming',
       due_on: '2014-09-04' },
     { id: 15301661079681,
       name: 'Asana',
       completed: false,
       assignee_status: 'today',
       due_on: null },
     { id: 15981206245213,
       name: '-test-',
       completed: false,
       assignee_status: 'today',
       due_on: '2014-09-04' } ],
  _boundTo: undefined }

I don't know what @domenic thinks, but this is inherently difficult (/ impossible) because of the way Chai work internally. On the outside, it seems as if you are accessing properties (and thus retrieving new objects), whereas on the inside, you are modifying the same object. The current architecture would thus mean that Chai Things has to know about Chai as Promised and vice-versa, and this goes for any plugin combination. I don't think it scales—a different architecture for Chai would be needed.

i noticed this issue and got a little curious - in theory they should all happily work together. So I put it to the test.

I installed latest of all versions:

chai@2.2.0
chai-as-promised@4.3.0
chai-things@0.2.0
mocha@2.2.1

And put together a test to ensure that it all works as its supposed to:

var chai = require('chai');
chai
    .use(require('chai-things'))
    .use(require('chai-as-promised'))
var should = chai.should();
var expect = chai.expect;
var promise = new Promise(function (resolve) {
    setTimeout(resolve, 200, [2,3,4]);
});

it('without promise deep equal', function () {
    expect([2,3,4]).to.all.be.above(1);
    return expect(promise).to.eventually.deep.equal([2,3,4]);
});


it('promise deep equal', function () {
    return expect(promise).to.eventually.deep.equal([2,3,4]);
});

it('without promise all', function () {
    return expect(promise).to.eventually.all.be.above(1);
});

it('fail', function () {
    return expect(promise).to.eventually.all.be.below(3);
});

it('like @mancvso\'s', function () {
    var promise = new Promise(function (resolve) {
        setTimeout(resolve, 200, [
            {property: 'value'},
            {property: 'value'}
        ]);
    });

    return ( promise ).should.eventually.all.have.property('property', 'value');
});

And voila! It just works!

   without promise deep equal (196ms)
   promise deep equal
   without promise all
  1) fail
   like @mancvso's (203ms)

  4 passing (411ms)
  1 failing

  1)  fail:
     AssertionError: expected all elements of [ 2, 3, 4 ] to be below 3

@mancvso the real trick is to ensure chai-as-promised is the last use() in chai. In other words:

// This works :)
chai
    .use(require('chai-things'))
    .use(require('chai-as-promised'));

// This doesn't :(
chai
    .use(require('chai-as-promised'))
    .use(require('chai-things'));

@mancvso if you could try this and let me know if it worked for you. If you do I'll add a PR in the readme to explictly mention it.

@mancvso have you given my example a try? I'd be interested to know if this works for you or not.

Hi!
I've tried your test case and it works as expected. Who could have imagined the order would matter?

✓ without promise deep equal (202ms)
  ✓ promise deep equal 
  ✓ without promise all 
  1) fail
  ✓ like @mancvso's (202ms)

  4 passing (415ms)
  1 failing

  1)  fail:
     AssertionError: expected all elements of [ 2, 3, 4 ] to be below 3

Sadly, at work we've switched to a different testing framework (cucumber-js).
Thanks for your investigation and the solution. I'll use this in personal projects.

@keithamus Any progress on this?

Sorry @RubenVerborgh this kind of slipped my mind with so many other things. Thanks for reminding me about this, I can PR what I have ready now, maybe you could take a look and take it further?

This should work as of Chai as Promised 5.0.0.

Oh yeah, I was thinking about the wrong issue. As @domenic mentions this should work.

I was thinking about #7, which I do have a PR for. This one can be closed @RubenVerborgh