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

Add fill and submit form feature test #517

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

SamJamCul
Copy link
Contributor

@SamJamCul SamJamCul commented Nov 30, 2023

What problem does this pull request solve?

Trello card: https://trello.com/c/BP0bsQOf/1171-add-beginnings-of-missing-feature-tests-to-the-runner

This adds a basic feature test that covers filling in and submitting a form.
There's also a change to our Axe check helper, which skips checking radio buttons for the aria-allowed-attr rule. This is a rule that probably shouldn't be catching our use of aria-expanded="false" as explored in this thread.

I've opted to turn the email confirmation feature on in this test, as it's currently off by default. I kind of got most of the way through writing the tests before realising I'd turned it on locally, and it felt a bit pointless removing the opt-out-of-email-confirmation step in the test just because we haven't removed the feature flag yet.

The ARIA_ALLOWED_ATTR constant was added as sonarcloud thought it smelled funny otherwise.

@SamJamCul SamJamCul force-pushed the 1171-add-missing-feature-tests-to-the-runner branch 3 times, most recently from 8906477 to 586c757 Compare December 1, 2023 09:19
@chao-xian
Copy link
Contributor

@SamJamCul nice work! Please can you expand the description on the commit message. Even repeating the PR description is fine. See: https://gds-way.cloudapps.digital/standards/source-code/working-with-git.html#commit-messages and in particular https://cbea.ms/git-commit/

@chao-xian chao-xian self-assigned this Dec 1, 2023
@SamJamCul SamJamCul force-pushed the 1171-add-missing-feature-tests-to-the-runner branch from 586c757 to 4cd3ff1 Compare December 1, 2023 11:15
@SamJamCul
Copy link
Contributor Author

@SamJamCul nice work! Please can you expand the description on the commit message. Even repeating the PR description is fine. See: https://gds-way.cloudapps.digital/standards/source-code/working-with-git.html#commit-messages and in particular https://cbea.ms/git-commit/

Done, I've summarised the changes without as much hand-wringing as my pull request

@chao-xian
Copy link
Contributor

@SamJamCul the commit message itself can have a link to the issue in govuk-frontend too - it's useful background

Adds a new feature spec which covers the basic form filler journey. It only contains one question, which is answered without triggering any validation, and the form is submitted without opting for an email confirmation.

There's also a tweak to our use of the Axe clean accessibility checks, which was flagging up our use of `aria-expanded="false"`. It looks like this should be passing, and the Axe rule hasn't been changed yet to reflect this. As a result, we've ignored any triggering of this rule on radio buttons. See alphagov/govuk-frontend#979 for more details
@SamJamCul SamJamCul force-pushed the 1171-add-missing-feature-tests-to-the-runner branch from 4cd3ff1 to ac2c982 Compare December 4, 2023 10:19
Copy link

sonarcloud bot commented Dec 4, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@SamJamCul
Copy link
Contributor Author

@SamJamCul the commit message itself can have a link to the issue in govuk-frontend too - it's useful background

Good shout! I was a bit wary of including links in commit messages, but as it's a github issue it seems silly to avoid it!

@SamJamCul SamJamCul added this pull request to the merge queue Dec 5, 2023
Merged via the queue into main with commit 19578ac Dec 5, 2023
4 checks passed
@SamJamCul SamJamCul deleted the 1171-add-missing-feature-tests-to-the-runner branch December 5, 2023 10:15
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.

3 participants