Overwrite method default _super does not throw

[wbyoung]

It appears that this is intentional, but it seems that the following plugin:

Assertion.addProperty('callCenter', function() {
  utils.flag(this, 'callCenter', true);
  this.assert(this._obj instanceof CallCenter);
});

Assertion.overwriteMethod('callCount', function(_super) {
  return function assertCallCount(n) {
    if (utils.flag(this, 'callCenter')) {
      /* custom assertion logic */
    }
    else {
      _super.apply(this, arguments);
    }
  };
});

Should work as so:

// this results in the assertion running & should pass/fail based on the call center plugin logic
var callcenter = new CallCenter();
callcenter.should.be.callCenter.and.have.callCount(0);

// using another plugin
var spy = sinon.spy();
spy.should.have.callCount(0);

// this results in a calling through to super every time. it doesn't make sense & should probably always fail
var array = [];
array.should.have.callCount(5);

The goal here would be to make a plugin that respects other plugins that may define a callCount method. I just wanted to check to see if this was the intended behavior of Chai. If it is, is there another way to achieve this type of result?

[keithamus]

Hey @wbyoung thanks for the issue.

_super just defaults to returning this, but if the method exists already, it is set (see L44,45).

If this is not working for you, I'd guess that maybe you're loading your plugin before other ones? Your plugin should be loaded last if you're doing this kind of logic - that way overwriteMethod actually has something to overwrite.

[wbyoung]

@keithamus thanks for the response. I may not have been completely clear. I understand the logic that's going on in the overwriteMethod implementation. I think, however, that the behavior could be enhanced & would result in better possibilities for the plugin ecosystem.

Imagine you're creating a plugin that you wanted to be compatible with another commonly used plugin (for instance, sinon-chai) and you wanted to add a method by the same name as theirs. You only want your plugin to apply for a custom type (or when some flag is set).

I reworked my example from above to use callCount instead to demonstrate this.

We'll have to delve into the theoretical a little to get to the point. If sinon-chai also used overwriteMethod to add callCount (which it doesn't) and only performed assertions when the object was a spy (again, it doesn't), then both of these plugins would be completely compatible with one another.

You could install either one before the other and they would work perfectly together. Both would call through to _super when the assertion wasn't applicable.

The main problem, though, is that this would pass:

var array = [];
array.should.have.callCount(5);

Put another way, in may other programming languages, this._super() would throw an exception if there was no super method to call. In this case, it just silently allows you to do so.

[wbyoung]

One last thought: I believe plugins could implement something to handle this scenario themselves using (semi-private) understanding of Chai's Assertion.prototype:

var overwriteOrAddMethod = function(name, fn) {
  if (typeof Assertion.prototype[name] === 'function') {
    Assertion.overwriteMethod(name, fn);
  }
  else {
    var _superError = function() { throw new Error('Method \'' +  name + '\' cannot be used in this context'); };
    Assertion.addMethod(name, fn(_superError));
  }
};

I guess the real questions are:

• Should this functionality be part of overwriteMethod by default?
• Should this even something plugins consider?
• If one really wanted to accomplish this anyway, does the above code make sense?

[keithamus]

Ah, I see now. You make a good point. The default behaviour should probably throw, because as you say there is a chance of silently passing on methods that don't exist.

PR welcome, but please make the PR against the 4.x.x branch - as it'll be a semver breaking change.

[berkerpeksag]

This is fixed in the 4.x.x branch by 2f4cd16.

[keithamus]

👍 thanks @berkerpeksag