Acceptance Tests: Use the most specific selector #661

seanpdoyle · 2022-05-09T15:37:37Z

Capybara provides a wide range of semantically meaningful selectors,
and also provides affordances for applications to declare their own
through the Capybara.add_selector interface.

In the sample Acceptance test, replace the Exercise phase's within
call with a call to within_fieldset.

The within call's "#sign_up" argument scopes the subsequent calls to
fill_in and click_button to operate on elements nested inside an
element with [id="sign_up"]. The "Sign up"
argument passed to the within_fieldset call (its "locator") scopes the calls to fill_in and
click_button to operate on elements nested inside a <fieldset>
element with a descendant <legend> element that
contains the text "Sign up".

Similarly, replace the Verify phase's have_content call with one to
have_button. A call to have_content("Sign out") will succeed if
the phrase Sign out exists as visible text anywhere on the page. A
call to have_button("Sign out") will only match a <button> element
with Sign out as its text or an <input type="button" value="Sign out"> element.

Capybara provides a wide range of semantically meaningful [selectors][], and also provides affordances for applications to declare their own through the [Capybara.add_selector][] interface. In the sample Acceptance test, replace the Exercise phase's [within][] call with a call to [within_fieldset][]. The `within` call's `"#sign_up"` argument scopes the subsequent calls to `fill_in` and `click_button` to operate on elements nested inside an element with `[id="sign_up"]`. The `within_fieldset` call's "Sign up"` argument (its "locator") scopes the calls to `fill_in` and `click_button` to operate on elements nested inside a [`<fieldset>` element][fieldset] with a descendant [`<legend>` element][legend] that contains the text "Sign up". Similarly, replace the Verify phase's [have_content][] call with one to [have_button][]. A call to `have_content("Sign out")` will succeed if the phrase `Sign out` exists as visible text anywhere on the page. A call to `have_button("Sign out")` will only match a `<button>` element with `Sign out` as its text or an `<input type="button" value="Sign out">` element. [selectors]: https://rubydoc.info/github/teamcapybara/capybara/master/Capybara/Selector#built-in-selectors [add_selector]: https://rubydoc.info/github/teamcapybara/capybara/master/Capybara.add_selector [within]: https://rubydoc.info/github/teamcapybara/capybara/master/Capybara/Session:within [within_fieldset]: https://rubydoc.info/github/teamcapybara/capybara/master/Capybara/Session:within_fieldset [fieldset]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/fieldset [legend]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/legend [have_content]: https://rubydoc.info/github/teamcapybara/capybara/master/Capybara/RSpecMatchers:have_content [have_button]: https://rubydoc.info/github/teamcapybara/capybara/master/Capybara/RSpecMatchers:have_button

seanpdoyle · 2022-05-09T16:23:24Z

testing-rspec/acceptance_test_spec.rb

      fill_in "Email", with: "[email protected]"
      fill_in "Password", with: "Examp1ePa$$"
      click_button "Sign up"
    end

-    expect(page).to have_content("Sign out")
+    expect(page).to have_button("Sign out")


@cpjmcquillan this commit replaces the have_content matcher with a have_button matcher.

Since the sample code block is already making assumptions about the hypothetical content of the response, making the matcher more specific didn't feel like a risk.

Did you have something else in mind when you introduced this code sample? Does this feel like an improvement?

I didn't originally introduce the code sample, but I changed the formatting and some of the language used to describe the tests in #654.

I think the change you've made is helpful, though. I can't imagine a real scenario where I would expect this test to render a page with "Sign out" content that wasn't a button.

seanpdoyle · 2022-05-09T16:24:17Z

testing-rspec/acceptance_test_spec.rb

@@ -6,12 +6,12 @@
  it "signs up the user with valid details" do
    visit sign_up_path

-    within "#sign_up" do
+    within_fieldset "Sign up" do


@cpjmcquillan similar to https://github.com/thoughtbot/guides/pull/661/files#r868194849, I'm curious how you feel about changing the hypothetical example page to nest these form fields within a <fieldset> + <legend> pairing instead of a <div id="sign_up"> element.

To share more context, this change draws inspiration from Section 5.3.2.5 Naming Fieldsets with the Legend Element:

Using the legend element to name a fieldset element satisfies Rule 2: Prefer Visible Text and Rule 3: Prefer Native Techniques.

I think it's a worthwhile change.

Is it worth linking to the w3 guidance somewhere too?

I miss being able to lean on the GOV.UK Design System for things like this. I wonder if an accessibility-focused, style-agnostic design system (that we could build component libraries and form builders for) would help us standardise the way we structure most of our web pages?

cpjmcquillan

LGTM

cpjmcquillan · 2022-05-10T17:15:44Z

testing-rspec/acceptance_test_spec.rb

      fill_in "Email", with: "[email protected]"
      fill_in "Password", with: "Examp1ePa$$"
      click_button "Sign up"
    end

-    expect(page).to have_content("Sign out")
+    expect(page).to have_button("Sign out")


I didn't originally introduce the code sample, but I changed the formatting and some of the language used to describe the tests in #654.

I think the change you've made is helpful, though. I can't imagine a real scenario where I would expect this test to render a page with "Sign out" content that wasn't a button.

cpjmcquillan · 2022-05-10T17:25:18Z

testing-rspec/acceptance_test_spec.rb

@@ -6,12 +6,12 @@
  it "signs up the user with valid details" do
    visit sign_up_path

-    within "#sign_up" do
+    within_fieldset "Sign up" do


I think it's a worthwhile change.

Is it worth linking to the w3 guidance somewhere too?

I miss being able to lean on the GOV.UK Design System for things like this. I wonder if an accessibility-focused, style-agnostic design system (that we could build component libraries and form builders for) would help us standardise the way we structure most of our web pages?

thiagoa · 2022-05-13T16:45:54Z

testing-rspec/README.md

@@ -45,6 +45,7 @@

 [Sample](acceptance_test_spec.rb)

+- Use the most specific [selectors][] available.


Does "most specific selector" mean favoring button selectors when using buttons? I can definitely see a case where some flexibility would be helpful - for example, with a selector like link_or_button, which would allow changing from a button to a link and vice-versa without a test failing.

From a development perspective, being flexible enough to support finding or clicking on an <a> or <button> has appeal.

My goal with these guidelines is to encourage design and development exercises to center accessibility. With that in mind, the semantics of a <a> and <button> vary. The expectation for an <a> element are navigation focused, whereas <button> elements are action focused. To that end, assistive technologies like Screen Readers (e.g. VoiceOver) present Button and Links in different lists.

Being specific in your testing expectations forces you to decide which element to use ahead of time. A failing test is valuable feedback that the element in question was changed, and provides you with an inflection point to re-consider or re-affirm that decision.

That makes sense, thank you for explaining. Indeed there's a role difference between links and buttons and it really matters for accessibility.

mike-burns

Things that will make me read your PR more carefully: the eq selector ... I feel like have_content is the eq selector of the Capybara world.

Thanks for teaching me about within_fieldset!

JoelQ · 2022-05-31T16:24:39Z

Thanks for adding this! Hopefully this nudges us towards better form design while also making our tests less brittle 👍

seanpdoyle requested review from cpjmcquillan and JoelQ May 9, 2022 16:21

seanpdoyle commented May 9, 2022

View reviewed changes

cpjmcquillan approved these changes May 10, 2022

View reviewed changes

thiagoa reviewed May 13, 2022

View reviewed changes

thiagoa approved these changes May 13, 2022

View reviewed changes

mike-burns approved these changes May 13, 2022

View reviewed changes

seanpdoyle merged commit 70e1faa into main May 31, 2022

seanpdoyle deleted the acceptance-testing-specificity branch May 31, 2022 15:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Acceptance Tests: Use the most specific selector #661

Acceptance Tests: Use the most specific selector #661

seanpdoyle commented May 9, 2022 •

edited

Loading

seanpdoyle May 9, 2022

cpjmcquillan May 10, 2022

seanpdoyle May 9, 2022

seanpdoyle May 9, 2022

cpjmcquillan May 10, 2022

cpjmcquillan left a comment

cpjmcquillan May 10, 2022

cpjmcquillan May 10, 2022

thiagoa May 13, 2022

seanpdoyle May 13, 2022

thiagoa May 13, 2022

mike-burns left a comment

JoelQ commented May 31, 2022

		@@ -45,6 +45,7 @@

		[Sample](acceptance_test_spec.rb)

		- Use the most specific [selectors][] available.

Acceptance Tests: Use the most specific selector #661

Acceptance Tests: Use the most specific selector #661

Conversation

seanpdoyle commented May 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpjmcquillan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mike-burns left a comment

Choose a reason for hiding this comment

JoelQ commented May 31, 2022

seanpdoyle commented May 9, 2022 •

edited

Loading