Skip to content

fix(objectContaining): Fixed a bug where ObjectContaining matched with non-object values. #15463

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

Merged
merged 3 commits into from
May 22, 2025

Conversation

mohammednumaan
Copy link
Contributor

Summary

The objectContaining method matches with non-object values, which is inconsistent. Thanks to @flovouin for finding this bug!

This PR fixes: #15420

This PR does the following:

  • Adds a conditional/check to ensure the argument passed to expect is an object.
  • Write new unit tests to ensure that new code does not break and works as intended.
  • Removes invalid test cases for the above bug.
  • Adds a few comments to clarify some bits of code.

Test plan

This is the new code I wrote for the objectContaining implementation:

image

This is the new test cases I wrote for verifying the above added code works:

image

To run the tests, I ran the following command:

yarn jest /packages/expect/src/__tests__/

The test results:

image

Copy link

netlify bot commented Jan 17, 2025

Deploy Preview for jestjs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit c751ae0
🔍 Latest deploy log https://app.netlify.com/sites/jestjs/deploys/6794bcc34f8eef00083e3272
😎 Deploy Preview https://deploy-preview-15463--jestjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mohammednumaan
Copy link
Contributor Author

mohammednumaan commented Jan 23, 2025

@SimenB I was wondering if the behavior in this issue is intentional? Imo, it doesn't make sense to treat them as the same. (i will open a PR, it if its a valid issue). Thanks!

@mohammednumaan
Copy link
Contributor Author

@phryneas @SimenB Hey, can you take a look at this PR? It would be greatly appreciated!

@phryneas
Copy link
Contributor

@mohammednumaan I'm not a maintainer of jest, just a contributor - sorry ^^

Copy link

This PR is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label May 13, 2025
@MillerSvt
Copy link
Contributor

not stale

Copy link
Member

@cpojer cpojer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Facebook is gonna have fun upgrading to Jest 30.

@cpojer cpojer merged commit 9753fdc into jestjs:main May 22, 2025
86 checks passed
@madcapnmckay
Copy link
Contributor

madcapnmckay commented Jun 10, 2025

When trying to upgrade my team to v30 I found this bug was being used in asymetric matching to assert on the array length. I couldn't see an alternative approach now that this has been fixed.

For example, this used to pass, but now fails when trying to assert on the length of list

expect(testFunc).toHaveBeenLastCalledWith(
    expect.objectContaining({
        someArray: expect.arrayContaining([
            expect.objectContaining({
                list: expect.objectContaining({ length: 2 }),
            }),
        ]),
    }),
);

My workaround has been to switch to an array filled with an expect.anything()

expect(testFunc).toHaveBeenLastCalledWith(
    expect.objectContaining({
        someArray: expect.arrayContaining([
            expect.objectContaining({
                list: new Array(2).fill(expect.anything())
            }),
        ]),
    }),
);

@MillerSvt
Copy link
Contributor

My workaround has been to switch to an array filled with an expect.anything()

I think it is valid solution.

Copy link

This pull request 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 Jul 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: objectContaining matches non-object values
5 participants