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

Added jest matcher support for .toHaveValue() #166

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

connormeredith
Copy link
Contributor

@connormeredith connormeredith commented Nov 19, 2019

What:
Added jest matcher support for .toHaveValue().

Why:
Addressing issue on #165 and for greater flexibility in tests.

How:
Similar to #93.

Checklist:

  • Documentation
  • Tests
  • Updated Type Definitions
  • Ready to be merged

@connormeredith
Copy link
Contributor Author

@gnapse I'd like to add the same support to toHaveFormValues where you can provide a matcher per property or a matcher for the whole form, however I noticed there's a test that expects an array of values to match regardless of the order and I'm wondering how useful that is: https://github.com/testing-library/jest-dom/blob/master/src/__tests__/to-have-form-values.js#L132-L140.

I guess this means that this PR would also remove that functionality from toHaveValue, since their implementation is very similar.

As it stands we wouldn't be able to switch out the implementation for this.equals because it's checking the order of arrays under the hood and therefore wouldn't be able to support jest matchers for .toHaveValue and .toHaveFormValue. I'd personally be in favour of using this.equals and enforce array order for matchers and allow us to use jest matchers. What are your thoughts?

@@ -23,7 +17,7 @@ export function toHaveValue(htmlElement, expectedValue) {
const expectsValue = expectedValue !== undefined
return {
pass: expectsValue
? isEqualWith(receivedValue, expectedValue, compareArraysAsSet)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change would mean we can no longer match arrays regardless of order, as per my comment. But we wouldn't be able to support jest matchers without it, e.g. expect.arrayContaining(['second', 'third'])

@gnapse
Copy link
Member

gnapse commented Jan 20, 2020

@connormeredith sorry that this has slipped off our plate for so long. But it still sounds like an interesting path to follow. Do you have the bandwidth to resume working on this and ship it? If not that's ok. I got the gist of it and I think I can help finishing it too. Let me know and I'll plan accordingly.

@tonivj5
Copy link
Contributor

tonivj5 commented Mar 26, 2020

Any news on this? I have noticed that I can not use regexes to assert values with the toHaveValue matcher

@gnapse
Copy link
Member

gnapse commented Mar 26, 2020

Oh boy this is embarrassing. @connormeredith I'm ready to merge this. Can you take care of the merge conflict? That file needs to be removed, since types are no longer maintained by us.

Base automatically changed from master to main January 20, 2021 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants