Proposal: rspec helpers for GOV.UK services

[frankieroberto]

Hi all,

This repo (and the initial code) is exploring an idea for a set of helpers that make it easier to write automated tests for services using rspec, whilst also helping to improve accessibility and usability.

Rationale

Generally, automated tests for GOV.UK services don't need any special helpers, as most components are just regular HTML features, so you can use all the standard rspec helpers like:

click_link "Back"
click_button "Continue"
fill_in "Name", with: "Jo Doe"
within "Country" { choose "United Kingdom" }

However, there are some GOV.UK components which are more custom, and so require a bit more work to test:

• Summary list (and summary cards)
• Error summaries
• Task list (soon)
• etc

These can end up with tests that are harder to read:

within(page.find("dt", text: "Name").ancestor(".govuk-summary-list__row").find("dd")) do
  expect(page).to have_text("Jo Bloggs")
end

Much nicer is to have a helper which lets the test focus on the content and not the markup:

expect(page).to summarise(key: "Name", value: "Jo Bloggs")

In addition to making tests easier to read and write, the tests could also help to ensure that some common accessibility and usability issues don’t get missed, such as making sure that "Change" links have visually-hidden (or visible) disambiguation words.

Rather than exhaustively testing for every possible implementation error, the tests would focus on the mistakes that are most common in the real world, from our own experience and things like external accessibility audits.

Initial sketch

I’ve started by sketching out 3 helpers:

summarise

This is intended for checking summary lists and is extracted from some code I originally wrote for Apply.

It supports keys and values, plus an optional single action per row (in theory you can have multiple actions per row, but I've never seen this done in practice):

expect(page).to summarise(key: "Date of birth", value: "20 September 1956"
expect(page).to summarise(key: "Name", value: "Jo Bloggs", action: { 
  text: "Change name", href: "/name" 
})

Currently you'd use a single assertion per row, and there’s no way to assert a particular order - but I’m not sure that order is something useful to test anyway?

summarise_errors

This checks the Error summary component, and also that it’s implemented correctly.

expect(html).to summarise_errors([
  "Enter your full name",
  "Select if you are British, Irish or a citizen of a different country"
])

This tests the follow:

• that the page has an Error: prefix in the title tag
• that the error summary is present and has a heading
• that the two errors listed are both present
• that the errors are in the order listed (because they should match the order of the fields on the page)
• that no other errors are present (to avoid accidentally having an extra error you weren’t expecting)
• that each error is an anchor link to a field on the page (to avoid accidentally having a link which doesn’t go anywhere)

have_descriptive_links

This is a more speculative one that I’m less sure about, as it’s so general and therefore less likely that you'd remember to test it for every page. But perhaps it's still useful?

expect(page).to have_descriptive_links

This tests that:

• No links match a list of short, ambiguous words (like "Change" or "Add")
• There aren’t 2 links on the page with the same link text

Possibly it could also test that there aren’t 2 links to the same page, which is ideally avoided, but I’m less sure about that as it's sometimes inevitable (eg a "Back" link going to the same page as a link in a navigation bar).

Open questions

• Is this approach useful?
• What other components or patterns could usefully have test helpers?
• What are the most common accessibility and usability issues that could be easily tested?

[frankieroberto]

@peteryates @paulrobertlloyd @nataliecarey @carlosmartinez any thoughts on this? ⬆️

[paulrobertlloyd]

A couple:

1. How tied to GOV.UK Components is this package? Am guessing its component library agnostic? In which case, how much do these tests check for correct markup? In my experience, I’ve seen manually marked-up summary lists missing the wrapping .govuk-summary-list__row <div> or .govuk-summary-list__row--no-actions on rows with no actions. Also change links missing govuk-link.

2. Ooh! Could we test for that!? The dreaded default blue-coloured links plague many a GOV.UK service 🫣 Gotta catch em’ all.

3. Talking about personal bugbares, a test for straight quotes, too!

4. Could the summary card test name be a little more descriptive? As much as I like summarise it's a bit vague and doesn’t say enough about what it’s actually testing. Perhaps something like summarise_list_value.

[paulrobertlloyd]

As to summary cards, would tests for the following recommendations be useful also?

Summary card titles:

Each title must be unique and help identify what the summary list describes. For example, this could be the name of a specific person, organisation or professional qualification.

Try to keep titles short and relevant. You can use one or two important values in the summary list — such as the first and last name of a person.

Summary card actions:

Keep it short and do not add more than 2 to 3 actions in a header.

[frankieroberto]

The tests aren’t tied to the GOV.UK Components or the GOV.UK Formbuilder, although so far I’ve assumed that most people who would use these rspec tests would also be using those, or something similar – so I’ve mainly focused on testing mistakes you could make when using those libraries, rather than the markup itself.

But if there are common errors you’ve actually seen, then we could definitely test those. I've seen missing govuk-summary-list__row--no-actions classes too.

I like the idea of testing for govuk-link classes missing from links across the whole page – but not sure how best to do that. I’ve created a placeholder expect(page).to have_descriptive_links expectation (which checks for link uniqueness and no ambiguous link text), but I'm not sure whether that’s the right approach – as you'd have to remember to add that expectation to every page...

summarise_list_values - or @peteryates’s suggestion of summaise_key().with_value() is nice. summarise made sense to me, until I realised that we'd need a separate expectation for summarise_errors... 🤦

[peteryates]

I love the idea of this. It's something I've considered a few times too.

How tied to GOV.UK Components is this package?

I don't know if it needs to be tied to govuk-components because some of these would probably apply to the form builder too (like the error summary) and I'm sure we'd want to make other form-based assertions like checking fields have the right label, hint, legend etc.

One huge strength of RSpec is that it allows for a slightly more conversational approach. Without too much additional work we could do something like this

expect(page).to summarise(key: "Name", value: "Jo Bloggs")

# could be expressed as the equivalent of:
expect(page).to summarise("Name").with("Jo Bloggs")

# or sticking to the class names in the design system:
expect(page).to summarise_key("Name").with_value("Jo Bloggs")

# and possibly, by wrapping `#within` from our matcher:
expect(page).to have_a_summary_card_titled("Lead tenant").with_items do
  expect(page).to summarise_key("Name").with_value("Jo Bloggs")
  expect(page).to summarise_key("Email").with_value("[email protected]")
end

[frankieroberto]

Agree that it doesn’t need to be tied to the components and formbuilder gems. That way, GDS could use it to test pages built with their govuk_publishing_components gem too.

This approach is interesting:

expect(page).to summarise_key("Name").with_value("Jo Bloggs")

...but is one downside that it doesn’t enforce always testing a value along with key? I can’t think of any use cases where that would be useful.

Another approach might be to reverse the order and do something like this:

expect(page).to have_value("Jo Bloggs", for: "Name")

(although have_value feels more like a form thing than a summary list!)

[frankieroberto]

Oh and by the way, on this:

I'm sure we'd want to make other form-based assertions like checking fields have the right label, hint, legend

I was assuming that these were unnecessary as the standard rspec/capybara expectations have these covered?

That said, I don't think they cover hints, as those aren’t part of standard HTML but require aria attributes, so maybe something to cover that would be good?

for_text_field "name", with_hint: "You do not need to enter middle names" do
  fill_in 'Jo Bloggs'
end

The tricky bit will be avoiding naming clashes with the default expectations... 🤔

[peteryates]

The capybara expectations work for labels and are the thing to reach for when filling and interacting, but when building form elements there are lots of levers to pull and I suspect people might want to ensure those things are rendered on the page. I admit testing for all of them might be a touch excessive, but I was thinking more along the lines of:

expect(page).to have_text_field("Account number")
  .with_prefix("ABC")
  .with_extra_spacing_between_letters
  .with_hint("It says it on your ID card")

There's also size, width, heading level, error messages that it might make sense to make available as a single assertion - because those things are linked to the input.

[frankieroberto]

@peteryates my assumption is that most of the time there’s no need to explicitly test the presence of text fields on a page with a given label etc, as these would usually be tested implicitly when filling in the fields for the next step?

ie something like this would be needlessly verbose?

expect(page).to have_text_field("Name")
fill_in("Name").with("Jo Bloggs")

It's also hard to know whether testing for things like hint text, prefixes, and extra-spacing would be overkill or not. Possibly? I'm unsure.

[peteryates]

It's also hard to know whether testing for things like hint text, prefixes, and extra-spacing would be overkill or not. Possibly? I'm unsure

Yeah, ordinarily no but occasionally it's useful

[carlosmartinez]

Great idea @frankieroberto. Will take a closer look at the initial implementation this afternoon.

[frankieroberto]

@carlosmartinez any thoughts?

[carlosmartinez]

only that i like it! let's start using it and get some feedback from usage in the wild. the summarise and summarise_errors matchers will add immediate value for a lot of teams.