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

Cannot console.log assertions because of proxies #765

Closed
meeber opened this issue Aug 6, 2016 · 6 comments · Fixed by #774
Closed

Cannot console.log assertions because of proxies #765

meeber opened this issue Aug 6, 2016 · 6 comments · Fixed by #774

Comments

@meeber
Copy link
Contributor

meeber commented Aug 6, 2016

Consider:

const {expect} = require("chai");

it("meep", () => {
  const myAssertion = expect(42).to.equal(42);

  console.log(myAssertion);
});

In Chai v3.5, the output is:

Assertion {
__flags: { ssfi: [Function], object: 42, message: undefined } }
✓ meep

1 passing (25ms)

But in the current version of Chai off Github, the output is:

  1. meep

0 passing (13ms)
1 failing

  1. meep:
    Error: Invalid Chai property: inspect
    at Error (native)
    at Object.getProperty [as get] node_modules/chai/lib/chai/utils/proxify.js:30:15
    at formatValue (util.js:343:19)
    at inspect (util.js:198:10)
    at exports.format (util.js:75:24)
    at Console.log (console.js:43:37)
    at Context.it (test/index.js:6:11)

Possible solution:

Add inspect as a blacklisted property here. then already exists as a blacklisted property for promise support. inspect is a standard property in Node, as noted in #727 (comment).

Rather than hard-coding the blacklist, it might be more appropriate to refactor the blacklist to be a new Chai config value.

@keithamus
Copy link
Member

Just wanna say btw, aside from this ticket, thank you for being an awesome chai maintainer! It really shows that you care and thats super important. You're my hero ❤️

@meeber
Copy link
Contributor Author

meeber commented Aug 6, 2016

IF CARTOON LEMURS COULD BLUSH

@lucasfcosta
Copy link
Member

Hello everyone, I'm back!
Keith's words are the same as mine, you're really awesome buddy!

Also, blacklisting values on the Chai config really seems more appropriate than hard-coding values.

@meeber
Copy link
Contributor Author

meeber commented Aug 12, 2016

@lucasfcosta Welcome back sir. You've been missed.

NOW GET TO WORK :D

AND ABSOLUTELY NO MORE VACATIONS EVER

(By the way, @keithamus @lucasfcosta, I'll be on vacation with limited connectivity Aug 18th - Sept 4th)

@lucasfcosta
Copy link
Member

I missed you too, especially your funny code reviews.
Have a nice vacation, make sure to eat lots of pizza and have lots of fun 😄

@meeber
Copy link
Contributor Author

meeber commented Aug 15, 2016

Welcoming PRs on this. See #770 for inspiration on how to go about this type of change.

What we're looking for is:

  • New Chai config value named something like proxyExclude, proxyBlacklist, proxyExcludedKeys, or proxyExcludedProperties. Not sure which is best. Maybe something else not listed.
  • It should be an array of strings (property keys) with default values of "then" and "inspect".
  • Any property key in this array should be excluded from the proxy property check here, replacing the current hard-coded value of "then".
  • The in-file documentation should note that this setting only applies to users with useProxy enabled, and only in environments where proxies are supported.
  • A couple of tests should be added to show that assertions don't contain a then or inspect property and yet accessing myAssertion.then and myAssertion.inspect doesn't throw an error despite useProxy being enabled in an environment where proxies are supported.

lucasfcosta added a commit to lucasfcosta/chai that referenced this issue Aug 23, 2016
lucasfcosta added a commit to lucasfcosta/chai that referenced this issue Aug 31, 2016
lucasfcosta added a commit to lucasfcosta/chai that referenced this issue Aug 31, 2016
lucasfcosta added a commit to lucasfcosta/chai that referenced this issue Aug 31, 2016
lucasfcosta added a commit to lucasfcosta/chai that referenced this issue Aug 31, 2016
lucasfcosta added a commit to lucasfcosta/chai that referenced this issue Sep 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants