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

Misleading .property assertion behaviour with value #452

Closed
onefifth opened this issue May 29, 2015 · 3 comments
Closed

Misleading .property assertion behaviour with value #452

onefifth opened this issue May 29, 2015 · 3 comments

Comments

@onefifth
Copy link

This test passes despite key not having a value of undefined.

var someObject = {key: 'value'};
expect(someObject).to.have.property('key', undefined);

I realize that the 'correct' way to test this would be to use the .undefined assertion, but there are a couple times where this has caught me off guard.
Say you've done something like .to.have.property('key', setupObject.myProprty), but you've misspelt .myProperty.
Now you have a really weird test that's passing without doing the value comparison it looks like it should be doing because you've mistakenly passed undefined.

"Offending" code: https://github.com/chaijs/chai/blob/master/lib/chai/core/assertions.js#L850

It would be nice to potentially use some internal CHAI_UNDEFINED const as the default value for val in these cases to internally differentiate between an unused optional parameter and one explicitly set to undefined. I have only encountered this using this particular assertion, but this behaviour quite possibly exists elsewhere there are optional arguments.

@onefifth
Copy link
Author

For clarification, this const could be set up by checking the length property of the arguments object to determine if the value undefined was passed in or not, assigning default values to unspecified args as needed.
The introduction of this const is just a suggestion and directly testing if (arguments.length > 1) { would suffice as a substitute for the if (undefined !== val) { line linked above.

@onefifth
Copy link
Author

The opened PR addresses this issue by checking the length of the arguments array. I attempted to add a few additional tests to cover these previously untested cases where undefined could be passed as an argument but my tests are failing with a fatal : Cannot read property 'sha' of undefined message.

If I could get help sorting that out I can update the PR with these additional tests.

Additional tests: onefifth@91f57e9
Failing TravisCI: https://travis-ci.org/onefifth/chai/builds/64526847

@onefifth
Copy link
Author

Apparently it's a weird environment thing. Pull request tests passed successfully.
If the additional tests look reasonable, consider #454 over #453.

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

2 participants