-
-
Notifications
You must be signed in to change notification settings - Fork 697
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
assertKeys cannot handle some valid ES6 keys #674
Comments
Resolution of chaijs#674
Hi, @meeber, thanks for this issue and for detailing it so well 😄 I think your solution for the first issue is valid. IMO we should still use About the second issue: what if we add support for symbols in inspect and use it into the |
Using Test var key1 = "purr"
, key2 = Symbol()
, key3 = "hiss"
, key4 = "meow"
, obj = {};
obj[key1] = 'val1';
obj[key2] = 'val2';
obj[key3] = 'val3';
obj[key4] = 'val4';
expect(obj).to.have.all.keys(key4, key3, key2, key1, "newp"); Result
(Disclaimer: Latest commit of Mocha was used so Symbol appears as "{}" in the above message) AssertionError's
AssertionError's
|
@meeber awesome job testing it! If you're willing to open a PR for that and need any help, please let me know. |
"Symbol(description)" is just the built-in toString conversion of a Symbol (it only failed in the Array.prototype.sort because Symbol has special behavior to forbid implicit conversion). I can't think off the top of my head of any alternatives regarding output; not much else can be derived from a Symbol because it's all magical with its internal unique identifier. But maybe someone has an idea! I should be able to cook up a PR, just need to add some tests. |
@meeber ah yes, I was just wondering if someone has another idea on how a Symbol should be represented, but I totally agree with you. I will be very happy to review and merge your PR after it's done 😄 |
Whoops, I misinterpreted the output of the test above: Chai's |
- Resolution of chaijs#674 - Add compareByInspect util for use with assertKeys sorts - Add Symbol support to the inspect utility - Add tests to utilities, should, expect, and assert
- Resolution of chaijs#674 - Add compareByInspect util for use with assertKeys sorts - Add Symbol support to the inspect utility - Add tests to utilities, should, expect, and assert
- Resolution of chaijs#674 - Add compareByInspect utility for use with assertKeys sorts - Add getOwnEnumerableProperties utility - Add getOwnEnumerablePropertySymbols utility - Add Symbol support to the inspect utility - Add tests to utilities, should, expect, and assert
- Resolution of chaijs#674 - Add compareByInspect utility for use with assertKeys sorts - Add getOwnEnumerableProperties utility - Add getOwnEnumerablePropertySymbols utility - Add Symbol support to the inspect utility - Add tests to utilities, should, expect, and assert
- Resolution of chaijs#674 - Add compareByInspect utility for use with assertKeys sorts - Add getOwnEnumerableProperties utility - Add getOwnEnumerablePropertySymbols utility - Add Symbol support to the inspect utility - Add tests to utilities, should, expect, and assert
- Resolution of chaijs/chai#674 - Add compareByInspect utility for use with assertKeys sorts - Add getOwnEnumerableProperties utility - Add getOwnEnumerablePropertySymbols utility - Add Symbol support to the inspect utility - Add tests to utilities, should, expect, and assert
There seems to be two ES6 issues with assertKeys:
Issue 1
If
expected
is a regular object with a key that is a Symbol, then assertKeys will not behave as expected. For example, the following assertion fails:val1 is missing from the AssertionError's
actual
because the behavior ofObject.keys
is to not include Symbols in its result set. (chai/lib/chai/core/assertions.js
Line 1173 in 6b640f7
val2 is showing up as the string "Symbol()" in the AssertionError's
expected
because it's explicitly converted to a string here: (chai/lib/chai/core/assertions.js
Line 1187 in 6b640f7
Issue 2
A
TypeError
is thrown if any of theexpected
oractual
keys in a keys assertion cannot be implicitly converted to a string byArray.prototype.sort
here:chai/lib/chai/core/assertions.js
Lines 1251 to 1252 in 6b640f7
Examples that fall victim to this issue but not the previous issue:
Possible Resolutions
The first issue could perhaps be resolved by checking if Symbol is implemented, and if so, first concatenating
Object.getOwnPropertyNames
(Edit: Or stillObject.keys
, so no unexpected behavior with enumerability) withObject.getOwnPropertySymbols
instead of usingObject.keys
, and then only converting the keys to strings if they aren't Symbols. (In the futureReflect.ownKeys
could be used for the first part).The second issue could perhaps be resolved by removing the two sorts linked above. The sorts were added a long time ago to make the diffs more readable (#264). Unfortunately, removing the sorts will break any user-created assertKeys test that catches the AssertionError and directly references its
.actual
and/or.expected
members and relies on at least one of them to be in sorted order when it isn't already in sorted order prior to the sort call. To preserve current behavior, but still resolve the issue, here are some alternative solutions:compareFunction
to feed into Array.prototype.sort that has special behavior to dump unsortable elements to the end of the arrayThe text was updated successfully, but these errors were encountered: