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

Use page.find instead of within in component tests #4712

Merged
merged 4 commits into from
Oct 18, 2021
Merged

Conversation

javierm
Copy link
Member

@javierm javierm commented Oct 17, 2021

References

Objectives

  • Avoid false negatives in component tests

@javierm javierm added the Specs label Oct 17, 2021
@javierm javierm self-assigned this Oct 17, 2021
@javierm javierm added this to Reviewing in Consul Democracy via automation Oct 17, 2021
@javierm javierm force-pushed the within_components branch 2 times, most recently from 2ca9384 to 58ffaec Compare October 18, 2021 10:19
@taitus taitus self-assigned this Oct 18, 2021
In component tests, the `within` method is actually an alias to RSpec's
`be_within` matcher, which is used to test numeric ranges. That meant
the tests always passed, even when there were bugs on the page.

In order to use `within` in component tests, we have to use
`page.within`. However, that also fails, since there's no such method
for `Capybara::Node::Simple'` objects, which are used in component
tests.

So we're using `page.find` instead.

See also pull request 945 in https://github.com/github/view_component
The test was passing because we were using `within`, but actually the
`have_css` method doesn't support the `href:` argument.
These tests don't work without JavaScript. They were passing because the
`within` method always passes in component tests.

This reverts most of commit 822140a.
This way we avoid writing useless tests which always pass.
Consul Democracy automation moved this from Reviewing to Testing Oct 18, 2021
@javierm javierm merged commit f4b338b into master Oct 18, 2021
@javierm javierm deleted the within_components branch October 18, 2021 14:28
Consul Democracy automation moved this from Testing to Release 1.4.0 Oct 18, 2021
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.

None yet

2 participants