Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

toHaveProperty does not match properties coming from class instance getters #5339

Closed
ruyasan opened this issue Jan 18, 2018 · 8 comments · Fixed by #5367
Closed

toHaveProperty does not match properties coming from class instance getters #5339

ruyasan opened this issue Jan 18, 2018 · 8 comments · Fixed by #5367

Comments

@ruyasan
Copy link

ruyasan commented Jan 18, 2018

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

The following code will fail on the last expect():

test('toHaveProperty should match getter values', () => {
  const obj = {
    normalProp: true,
    get getterProp() {
      return true;
    },
  };

  expect(obj).toHaveProperty('normalProp', true);
  expect(obj).toHaveProperty('getterProp', true);

  class Foo {
    get classGetterProp() {
      return true;
    }
  }
  expect(new Foo()).toHaveProperty('classGetterProp', true);
})

If the current behavior is a bug, please provide the steps to reproduce and
either a repl.it demo through https://repl.it/languages/jest or a minimal
repository on GitHub that we can yarn install and yarn test.

https://repl.it/repls/WoozyShortBanteng

What is the expected behavior?

It should pass.

Please provide your exact Jest configuration and mention your Jest, node,
yarn/npm version and operating system.

See repl.

@anilreddykatta
Copy link
Contributor

Hi @ruyasan, I verified the issue. It seems issue is in expect/utils.js.
https://github.com/facebook/jest/blob/f040725f81f2b4b32705a762c279cee2d001cf74/packages/expect/src/utils.js#L49-L55. For class based objects, prototype is returned as undefined, which makes utils -> getPath return Object without value property as we are deleting in this case.

@cpojer I am not able understand the purpose of the check. Please, possible to shed some light on it?

@anilreddykatta
Copy link
Contributor

anilreddykatta commented Jan 20, 2018

@SimenB I can raise a PR for this one.

// result.hasEndProp will give us undefined in the case of class.
if (result.hasEndProp === false) {
    delete result.value;
    result.traveresedPath.shift(); 
}

what you think about this one?

@SimenB
Copy link
Member

SimenB commented Jan 20, 2018

I'll defer to our test suite - if that change fixes this case without breaking other cases I'd say that looks like a really clean fix 🙂

@anilreddykatta
Copy link
Contributor

awesome.. 👍

@anilreddykatta
Copy link
Contributor

@SimenB That doesn't solve it! Either we should add logic to handle class probably like adding if clause where prototype is undefined, then we shouldn't even call hasOwnProperty?

@SimenB
Copy link
Member

SimenB commented Jan 22, 2018

@thymikee thoughts? ^

@thymikee
Copy link
Collaborator

Decided to give it a go, and filed a PR #5367

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants