Chai: embracing proxies

Created on 7 Sep 2016  路  8Comments  路  Source: chaijs/chai

Hi,

We could start enjoying the power of proxies to create even better DSL.
Examples:

let element = { visible: true }
element.should.be.visible
let element = { isVisible: true }
element.should.be.visible
let response = { status: 200 }
response.should.have.status(200)

WDYT?

Most helpful comment

My first impression is that it sounds cool in theory but in practice it opens the door too wide for mistakes and unexpected behavior without adding enough benefit over .property(name, value). As discussed in

726, one of our central focuses is making it difficult for developers to accidentally write tests that look and act as if they're correct but actually silently aren't.

I think enabling cool shortcuts like .be.visible is better served via domain-specific plugins as opposed to enabled via a proxy-based catch-all.

All 8 comments

My first impression is that it sounds cool in theory but in practice it opens the door too wide for mistakes and unexpected behavior without adding enough benefit over .property(name, value). As discussed in

726, one of our central focuses is making it difficult for developers to accidentally write tests that look and act as if they're correct but actually silently aren't.

I think enabling cool shortcuts like .be.visible is better served via domain-specific plugins as opposed to enabled via a proxy-based catch-all.

I've seen that in v4 proxies are used to protect from such problems to occur,
wouldn't it be enough to change it to something like
verifyChaiPropertyExists() || verifyTargetPropertyExists() ?

  • If we wanna be extra safe, we could flag be/have so we could restrict which assertion to use when calling visible/status/etc...

The use of proxy in 4.x is for defensive purposes only.

Acting as a catch-all property-assertion in the proposed way, I think there's too much room for accidents and misinterpretation.

let gasTank = {
  empty: true,
};

gasTank.should.be.empty;

Did the developer mean to check the domain-specific concept of a tank of gas being empty, or did they mean to use Chai's empty assertion? Maybe the developer knows and got it right, maybe they didn't. Maybe the developer's coworker reviewing the code knows and understands what's being tested, maybe they don't.

This becomes even more problematic when you consider how the behavior changes between environments that support proxies and those that don't.

let element = { visible: true }
element.should.be.visible

In proxy-supported environments, this would perform an assertion to check if element.visible was set to true or not. In non-proxy supported environments, it would just silently pass. To be encouraging a construct that has such dangerously varying behavior between JS engines is a non-starter. The whole point of the proxy stuff in v4 is to at least give developers a chance to detect a meaningless assertion, to reduce risk and error.

Due to the risk, I think this type of change is better suited to a plugin as opposed to Chai's default behavior. I would recommend that such a plugin be configured to only work in environments that support proxies.

Restricting the behavior to .be and .have doesn't alleviate my concerns.

Could such plugin (which monkey patches Chai) be exposed at chaijs.com/plugins ?

@yairper I suspect it's possible to write a normal plugin (http://chaijs.com/guide/plugins/) that implements the desired behavior by replacing be and have with custom logic. Adding chai-plugin keyword to the plugin's package.json would cause it to be added to the website list.

If it turns out there's really no way to implement this behavior through the plugin system and forking Chai is the only option, then I'm not sure what the best way to advertise it is. @keithamus @lucasfcosta?

I checked it up, and it could be done using the conventional way, Thanks !

@meeber Hmm, I'm not sure this behavior cannot be added to Chai through the plugin system.
Since chai.use basically gives the plugin developer the whole object to do whatever they please with I think it would be possible to implement this. It would be a lot of work probably, but I think it is possible.

Regarding the addition of this to the Core I totally agree with @meeber. His great explanation covers every thought I've had about it. I'd rather keep this in a separate plugin and avoid adding the possibility of writing contradictory assertions.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

zzzgit picture zzzgit  路  3Comments

sverrirs picture sverrirs  路  3Comments

ghost picture ghost  路  4Comments

meeber picture meeber  路  4Comments

JuHwon picture JuHwon  路  5Comments