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

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

Closed
domenic opened this issue Jan 26, 2012 · 16 comments
Closed

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

domenic opened this issue Jan 26, 2012 · 16 comments

Comments

@domenic
Copy link
Contributor

domenic commented Jan 26, 2012

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
Copy link
Member

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
Copy link

tj commented Jan 26, 2012

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

@logicalparadox
Copy link
Member

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
Copy link
Contributor Author

domenic commented Jan 26, 2012

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
Copy link
Contributor Author

domenic commented Jan 26, 2012

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

@tj
Copy link

tj commented Jan 26, 2012

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
Copy link
Member

@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 added a commit that referenced this issue Jan 27, 2012
@logicalparadox
Copy link
Member

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:

  • Always make sure you return this for chaining.
  • Use this.assert whenever possible so as that the errors are instances of chai.AssertionError.
  • You can also use chai.inspect to get a string-ified version of whatever object your looking at for error messages.
  • Language added on to the chai.Assertion.prototype will be available for both the should and expect interfaces.

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

If you build something for sharing, let me know!

@logicalparadox
Copy link
Member

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
Copy link
Contributor Author

domenic commented Jan 30, 2012

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

@domenic
Copy link
Contributor Author

domenic commented Feb 6, 2012

@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
Copy link
Member

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

@domenic
Copy link
Contributor Author

domenic commented Feb 13, 2012

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

@logicalparadox
Copy link
Member

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

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

@domenic
Copy link
Contributor Author

domenic commented Feb 13, 2012

@logicalparadox
Copy link
Member

@domenic Your up on the readme and website.

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

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

4 participants