Make assertions (such as .exist) callable

[fizker]

I am currently writing some tests using the expect-version of chai. And it is going fine, except there is a readability issue.

Take the following code:

expect(someVariable).to.exist;

This is perfectly valid, and will test as expected, since the assertion is done inside the getter .exist.

But to my eye, there is no action taken here. I would much prefer the line to be:

expect(someVariable).to.exist();

This can be achieved by simply changing the .exist definition from:

Object.defineProperty(Assertion.prototype, 'exist',
  { get: function () {
      this.assert(
          null != flag(this, 'object')
        , 'expected #{this} to exist'
        , 'expected #{this} to not exist'
      );

      return this;
    }
  , configurable: true
});

to:

Object.defineProperty(Assertion.prototype, 'exist',
  { get: function () {
      this.assert(
          null != flag(this, 'object')
        , 'expected #{this} to exist'
        , 'expected #{this} to not exist'
      );

      var self = this;
      return function() { return self; };
    }
  , configurable: true
});

The only drawback is that you cannot chain directly on .exist (like expect(a).to.exist.ok), but since there is no .and and the tests does not break after doing the change above, I don't see that as an issue.

So the question is, is this only an issue in my mind? And should I spend time on making a proper pull request?

[logicalparadox]

This behavior is by design and will not be changed, for a few reasons. I think what might help you is to explain our guidelines for how we construct each assertion. I am hoping that you will find this informative enough that helps you to adapt.

Please let us know if you have any questions after this. Hope this helps:

Guidelines

1. An assertion will be a method when parameter(s) are needed, a property when a paramater(s) are not needed.

Though some properties don't do anything (such as to), some properties do an action (such as not, ok, etc). As apposed to making it a method when an assertion does something, it is easier to look at it as whether an assertion needs something. Overall, this makes it easier to learn the chain-able api as you are not required to understand the exact inner working of each assertion.

Few caveats:

• throw() method can currently be called without any parameters. It remains this way for backwards compatibility, but a good tester will supply parameters to check against.
• In the master branch chai version 1.0.0-alpha1 and above, a / an and include / contain can be called as both properties, expect({ a: 1, b: 2}).to.contain.keys('a') or as methods, expect([1,2]).to.contain(1). Mainly, this is because the behavior of these 4 are different depending on how you use them (method vs property). Furthermore, they provide backwards compatibility and make it easier to convert tests from various other chain-able assertion libraries. We do however, want to keep this to a minimum as it makes it incredibly difficult for plugin developers to overwrite assertions for only certain use cases.

2. All assertions must permit the construction of an infinite chain.

In your scenario you mentioned that you would not be able to chain unless you called the function. Coincidentally, there is an and chain so you can do multiple assertions and still have it readable by us humans. Such as..

// 1 exists and is a number
var n = 1;
expect(n).to.exist.and.be.a('number'); 

// fn throws an instanceof ReferenceError with an error message that does not match regex /good function/
var fn = function () { throw ReferenceError('bad function'); };
expect(fn).to.throw(ReferenceError).and.not.throw(/good function/); 

[fizker]

Oh, so there is an .and. I only tested that against the current npm-version (0.5.2), and that does not contain .and yet. My proposal obviously won't fly then.

I guess I just have to suck it up until I get used to it ;). Writing tests in a proper red-green-refactor way should help with that as well.

Thanks for your time :).

[mgol]

One problem with this approach is that many editors/linters warns against using expressions as statements. It's quite useful to not have to disable such settings but sometimes they can only be set per project so one can't just disable them for tests.

[keithamus]

@mzgol and others, there is a plugin (dirty-chai) which overrides this behaviour, making all property-assertions method-assertions. If you desire this behaviour (to appease linting gods) then I suggest you use this.

[mgol]

@keithamus Thanks for the link, I'll try it! I think remarks in https://www.npmjs.com/package/dirty-chai#function-form-for-terminating-assertion-properties are valid, especially the part that mistyping one thing makes the test pass whereas the only correct approach when writing tests is that making a mistake means failing the test. One can achieve it either by using the function form (you'll get undefined is not a function) or by specifying the assertions count. Chai has neither of them so using the default interface will mean that some apps will break because their tests had mistakes in them. That's unfortunate.

Because of that, I think it's not just about personal taste and that it's something that should be supported by core. But if that's out of the picture, I'll try the plugin.

[keithamus]

@mzgol I agree, trust me we've been here before. We tried to support both but it didn't work well, so we reverted. At this juncture we'd need to make a breaking change for method-style-assertions - and it is something that polarises the community. Not something we're willing to do lightly, especially as a plugin like dirty-chai does the job. For context/history, feel free to read #41, #297, #306.

[mgol]

OK, I understand the problems with maintaining back compat. This is unfortunate but the world isn't ideal. I'll just try to use dirty-chai.

BTW, perhaps when ES6 Proxies become implemented throwing on accessing non-existing assertions could be implemented? In this way at least in Chrome/Firefox we'd get a failure on mismatch which will suffice in most cases.

[keithamus]

As an aside, Chai has ~650k downloads a month, while dirty-chai has ~2k, chai-missing-assertions has ~5k. Combined they make up just over 1% of all chai downloads. While I don't think those numbers are truly reflective of users who want to have method assertions, I think it can be used as a rough barometer. If the majority of chai users felt property assertions were that bad I'd expect to see at the very least double digit numbers, if not closer to, say, 50% of all chai downloads.

ES6 Proxies are a suitable idea. But we should discuss this in a new issue, rather than continuing discussion here.

[mgol]

ES6 Proxies are a suitable idea. But we should discuss this in a new issue, rather than continuing discussion here.

Sure. I've just created #407 for that.