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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

expect.not.objectContaining() does not match documentation #10186

Closed
ninevra opened this issue Jun 22, 2020 · 4 comments 路 Fixed by #10708
Closed

expect.not.objectContaining() does not match documentation #10186

ninevra opened this issue Jun 22, 2020 · 4 comments 路 Fixed by #10708

Comments

@ninevra
Copy link
Contributor

ninevra commented Jun 22, 2020

馃悰 Bug Report

The documentation for expect.not.objectContaining() says that it matches any object that is not a superset of the expected object. In fact, it matches any object that contains no entry equal to any entry of the expected object.

The documentation also says that expect.not.objectContaining() is the inverse of expect.objectContaining(). This is not true; for some (expected, received) pairs, neither assertion will match.

To Reproduce

Steps to reproduce the behavior:

import expect from 'expect';

expect({a: 1, b: 3}).toEqual(expect.not.objectContaining({a: 1, b: 2}); // throws

expect({a: 1, b: 3}).toEqual(expect.objectContaining({a: 1, b: 2}); // _also_ throws

In this example, the recieved object ({a: 1, c: 3}) is not a superset of the expected object ({a: 1, b: 2}), so according to the documentation, it should match; however, it does not.

Expected behavior

The behavior should be changed to reflect what the documentation says.

Link to repl or repo (highly encouraged)

repl.it demo.

envinfo

  System:
    OS: Linux 4.15 Ubuntu 18.04.4 LTS (Bionic Beaver)
    CPU: (4) x64 Intel(R) Core(TM) i7-4500U CPU @ 1.80GHz
  Binaries:
    Node: 14.4.0 - ~/.nvm/versions/node/v14.4.0/bin/node
    npm: 6.14.5 - ~/.nvm/versions/node/v14.4.0/bin/npm
@jeysal
Copy link
Contributor

jeysal commented Jun 22, 2020

The .not behavior seems potentially broken to me here. Regarding usefulness - do you have an example use case for .not.objectContaining (maybe one that lead you to discover this)? Asking because really I can barely imagine a use case for .not.objectContaining either way.

@ninevra
Copy link
Contributor Author

ninevra commented Jun 22, 2020

Not really, honestly. It can be used to assert that a property of an object does not have a given value, e.g. expect(received).toEqual(expect.not.objectContaining({foo: 'bar'})) would assert that received.foo does not exist or does not deep-equal 'bar', but that assertion is better written expect(received.foo).not.toEqual('bar'), and it's usually going to be better practice to positively assert that received.foo does equal some precise expected value.

On the other hand, I can't even begin to imagine a use case for asserting that at least one of a number of given properties does not match (to rephrase the documented behavior). Such a test would seem very likely to hide bugs, since any single missing or inequal property would cause a pass.

My guess as to the intended behavior was mostly based on the fact that someone went to the trouble of writing a more complex behavior than just negating expect.objectContaining(). I should have said "does not match documentation" rather than "documentation is incorrect"; I'll update the original issue unless you mind.

@ninevra ninevra changed the title expect.not.objectContaining() documentation is incorrect expect.not.objectContaining() does not match documentation Jun 22, 2020
@jeysal
Copy link
Contributor

jeysal commented Oct 25, 2020

Hmm, I think given that .not.objectContaining is such an obscure feature that almost no one will ever use, I think I would apply the principle of least surprise, not what is most (= marginally more) useful.
Least surprise here would IMO mean 'an object not containing (all of) the properties', so the opposite of .objectContaining without .not, so what the documentation says. So this is a bug fix up for grabs 馃憤 it should probably be marked breaking when it is fixed and landed in a major, to be safe.

@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 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants