-
-
Notifications
You must be signed in to change notification settings - Fork 701
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
Inspect doesn't work properly for Maps and Sets - It displays an empty object #662
Comments
Good work @lucasfcosta - this is definitely a valid issue. As per the roadmap issue (#457) we need to do some work to get the inspect functions from chai core into chaijs/loupe. I'd like to do this before we hit 4.0.0, and as soon as I've completed the work I'm doing with chaijs/deep-eql I'll be picking up Loupe as there's a few things I'd like to tackle with it. |
@keithamus Thanks for the info! |
I've got some other bits I want to work on for chaijs/loupe so hold off on this one just for now. We'll get this fixed for 4.0 though 😄 |
Marking this as pull-request-wanted as my refactor of loupe is quite far away. If you're reading this, feel free to pick this up and make this change for loupe! |
Important NotePlease enable the tests added on #668 which depend on this before closing this issue. |
This bit me today, and I see that 4.x has been released without loupe. So the question is, should this be done here in the meantime or is loupe ready for inclusion? In any case, I think this should be kept open until it has been fixed, yes? Here's node utils inspect for reference. |
@maxnordlund we're trying to track issues like this on our board: https://github.com/chaijs/chai/projects/2 as we get many duplicates or similar issues. You can see https://github.com/chaijs/chai/projects/2 for progress on this. Loupe will be released with chai 5 😄 |
Aha, I didn't see that.
Sweet, I'm looking forward to it. Though for native WeakMap and WeakSet, only the native inspect can look inside them, which may not be a problem but still, worth mentioning. |
Loupe will be at-least as good as Node's |
This got accidentally closed because #668's description contained the text |
This bug had me wondering for a while why my set was empty until I found it wasn't: expec(new Set([1,2,3])).to.contain(7)
// => AssertionError: expected {} to include 7 I wonder whether it would make sense to leverage |
Here's the todo list for releasing loupe v2: chaijs/loupe#15. Once released we can bring it into Chai and probably release a 4.4 version to get this in before Chai v5. Any one who wants to help out with the todo list, we'd super appreciate it! |
Just ran into this issue - it was fixed a while back, and can be closed. Loupe is already at v2.3 in Chai, and the commented out tests described above are now uncommented and passing with useful output for Maps & Sets: Lines 564 to 605 in a8359d3
|
Awesome thanks for the update @pimterry |
As I was implementing the
.deep
flag support on thekeys
assertion I noticed thatutils.inspect
isn't working properly when it comes to Maps and Sets. Instead of returning the values inside the Map or Set it returns the following string:{}
.You can reproduce this behavior by doing, for example:
What do you guys think should be the desired output when inspecting maps or sets?
IMO the ideal output would be:
I think this wouldn't be a difficult fix. Maybe adding a check alongside these other checks and then calling a function to iterate through every entry and format the Map/Set members should be enough. This will be very similiar to what we already do in
formatArray
.I will make sure to submit a PR for this before implementing the
deep
flag for Maps/Sets because this will be very useful to create good test cases.Please let me know if you've got any idea or opinion about this 😉
The text was updated successfully, but these errors were encountered: