Skip to content

Implement toBeInvalid & toBeValid matchers #110

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 7 commits into from
May 24, 2019

Conversation

hiwelo
Copy link
Contributor

@hiwelo hiwelo commented May 23, 2019

What:

This pull-request is a proposed implementation for the custom matchers toBeInvalid and toBeValid.

Why:

This is following a discussion we quickly had with the issue #108.
These two custom matchers can avoid folks to check with different expect tests:

  • is having aria-invalid not equal to false
  • checkValidity is returning false

How:

  • toBeInvalid checks if one of these are correct:

    • aria-invalid exists and if the value is not equal to false (as each value different than false are considered true (ref))

    • the targetted element is a form element and checkValidity() returns false

  • toBeValid checks if all of these are correct:

    • aria-invalid is not part of the markup or the value is equal to false

    • if the targetted element is a form element, checkValidity() returns true

Checklist:

  • Documentation
  • Tests
  • Updated Type Definitions
  • Ready to be merged
  • Added myself to contributors table

I do not know if it's a good practice, please let me know your thoughts about that: I had to use jsdom to be able to test if these two custom matchers are correctly checking .checkValidity().
Without it, .checkValidity() was always returning true even when the input had an invalid value.

@hiwelo hiwelo marked this pull request as ready for review May 23, 2019 17:34
Copy link
Member

@gnapse gnapse left a comment

Choose a reason for hiding this comment

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

Looking great. Just a couple of comments requesting to add more documentation or links to documentation and issues.

@hiwelo
Copy link
Contributor Author

hiwelo commented May 23, 2019

I added linked resources to the README.md.
I will move the extract the jsdom object within an helper function and let's see after what we can do with this.

I will at least document why it's there.
Also, it should only impact the tests, not the usage of jest-dom itself. At the moment, it is a devDependency only called within this test file.
In the toBeInvalid file, any real DOM node should work, no need for this library when using jest-dom. 👍

@hiwelo
Copy link
Contributor Author

hiwelo commented May 23, 2019

Ok, so finally I was able to remove the specific testing I created earlier.
Thanks to the jsdom update to a 15+ version, I was able to checkValidaty() on a rendered HTMLElement using the existing helper. 🎉
Way cleaner this way 🙂

Every test passed, so I think the upgrade should not be an issue.

@hiwelo
Copy link
Contributor Author

hiwelo commented May 23, 2019

Hmm… The test is completed without any issue on my local machine… :/
Any idea why it's failing specifically on Travis?

@gnapse
Copy link
Member

gnapse commented May 24, 2019

Hmm… The test is completed without any issue on my local machine… :/
Any idea why it's failing specifically on Travis?

It seems the jsdom issue is still biting us

image

I don't mind if you need to revert this to how it was for the time being.

@hiwelo
Copy link
Contributor Author

hiwelo commented May 24, 2019

Cool! I will revert my last commit and push it again.

@hiwelo
Copy link
Contributor Author

hiwelo commented May 24, 2019

@gnapse I just rebased the branch to remove the last commit 👍
(gosh, I love git rebase so much haha)

@gnapse gnapse merged commit 09f7f04 into testing-library:master May 24, 2019
@gnapse
Copy link
Member

gnapse commented May 24, 2019

🎉 This PR is included in version 3.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@hiwelo hiwelo deleted the toBeInvalid branch May 24, 2019 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants