chaijs / chai

BDD / TDD assertion framework for node.js and the browser that can be paired with any testing framework.
https://chaijs.github.io
MIT License
8.13k stars 698 forks source link

Is there a "supported" way to extend the set of assertions? #9

Closed domenic closed 12 years ago

domenic commented 12 years ago

I was hoping to make a sinon adapter for chai. This would allow code like

var spy = sinon.spy();

expect(spy).to.have.been.calledWith(1, 2, 3);
spy.should.not.have.been.called;

Unfortunately, the only way I could see to do this is to muck around with the undocumented parts of chai, by modifying require("chai").Assertion.prototype.

Is this the best way? If so, could you document Assertion.prototype and promise that it will be maintained as a supported extension point as long as the major version number stays the same?

logicalparadox commented 12 years ago

As of this moment there is no "official" way to extend the set of assertions, but I really like the idea!

As of right now, the only thing that you can really do is tack things on to Assertion.prototype, as you mentioned. I had no intent to make significant changes that would cause issues if you did. The issue would be that the interfaces require Assertion directly, so your mods wouldn't be picked up. You would also need to create a new interface. Just thinking out loud here.

As far as documenting Assertion.prototype, it already is. If you look at the expect interface you will see that all it does is return a new Assertion.

// https://github.com/logicalparadox/chai/blob/master/lib/interface/expect.js
module.exports = function (val, message) {
  return new Assertion(val, message);
};

I'm going to toy with the idea and see what I come up with.

tj commented 12 years ago

adding an api to extend a prototype makes no sense IMO, unless it's wrapping with some kind of added behaviour

logicalparadox commented 12 years ago

I think the "middleware-ish" approach seems the best.

var chai = require('chai')
  , custom = require('custom');

chai.use(custom.sinon(sinon));

var should = chai.interface('should')
  , expect = chai.interface('expect');

And then your custom component would look like this:

// custom.js

var custom = module.exports = {};

custom.sinon = function (sinon) {
  // passed in global sinon
  return function (assert) {
    assert.define('calledWith', function (arg1) {
      var spy = this.obj;
      // … or whatever it is you need to do
      return this; // for chaining
    });

    assert.get('called', function () {
      var spy = this.obj;
      // again, test
      return this; // for chaining
    });
  }
}

Thoughts on this approach?

domenic commented 12 years ago

That sounds pretty nice! Just to make sure I'm clear, the above could be done as follows too, yes? (Seems simpler.)

var chai = require("chai");
var custom = require("custom");

chai.use(custom.sinon);

// custom.js
var sinon = require("sinon");

exports.sinon = function (assert) {
  assert.define(...);
  assert.get(...);
};

I like the idea of helpers like define, get, maybe grammar (for words like "been").

Would this chai.interface("should") be a new thing replacing chai.should? Would it be necessary to call chai.use before chai.interface, or could the order be reversed?

domenic commented 12 years ago

Also, there would need to be an interface for defining negated versions, yes?

tj commented 12 years ago

if it's middleware-ish it would be simple enough to just do:

function sinon(chai){
  chai.whatever.prototype.foo = function(){}
}

chai.use(sinon);

chai.use = function(fn){
  fn(this);
  return this;
};

or just:

sinon(chai)

haha. I dont know, I think anything more than that is really over-engineering something people should already be used to (prototypes). I've done the first a couple times for libraries that needed to use events etc but .use() gives you some future-proofing and kinda helps make sure the api is unified

logicalparadox commented 12 years ago

@visionmedia I could see how this could get complicated quickly. I do rather like my approach, but without the value added behavior it seems a mute point at the moment. I think you have given a good starting point with the simplest .use.

@DomenicDenicola You should familiarize yourself with chai.Assertion.prototype.assert, as it handles negation for you. You can also check for negation on this.negate as seen here#L535.

Will post code in a bit.

logicalparadox commented 12 years ago

The 0.2.x version of chai will allow you to expand on the main chai export. Your code should be something like this:

module.exports = function (chai) {
  Object.defineProperty(chai.Assertion.prototype, 'called', {
    get: function () {
      // whatever
      return this;
    }
  });

  chai.Assertion.prototype.calledWith = function () {
    // whatever here
    return this;
  };
};

You can then add them into your tests...

var chai = require('chai')
  , mymods = require('mymods');

chai.use(mymods);

Also note, exposing of the interfaces is exactly the same as it was.

var should = chai.should()
  , expect = chai.expect
  , assert = chai.assert;

Developer Notes:

Your first point of reference for building well formed assertions is lib/assertion.js.

If you build something for sharing, let me know!

logicalparadox commented 12 years ago

Hey @DomenicDenicola, did you get a chance to work with this. If so, what did you come up with?

Going to close this issue as minimal viable feature has been implemented. But do let me know if you have further thoughts or ideas.

domenic commented 12 years ago

@logicalparadox Working on it! :) Seems to do the trick. I should have something up this week.

domenic commented 12 years ago

@logicalparadox Here is something I have run into: if two test scripts call chai.use(sinonChai), I get errors because both instances of use are trying to redefine the same properties of chai.Assertion.prototype.

There are a number of workarounds, including ensuring I only do this once, or defining the properties as configurable. But I think it would be most elegant if chai.use checks to see if the function passed has ever been seen before (storing all use'ed functions in an array would do the trick) and bails out early if it has.

jfirebaugh commented 12 years ago

@domenic Any news on the sinon-chai integration? I'd definitely be interested in it.

domenic commented 12 years ago

@jfirebaugh I was actually just planning on finishing it up today!! More motivation, haha.

logicalparadox commented 12 years ago

I have added a section to the readme and the website that lists plugins.

@domenic let me know when you publish a module :)

domenic commented 12 years ago

@logicalparadox Alright, done!! :D

https://github.com/domenic/sinon-chai http://search.npmjs.org/#/sinon-chai

logicalparadox commented 12 years ago

@domenic Your up on the readme and website.

Great work guys. jQuery and Sinon integrations in one day. This is awesome!