-
Notifications
You must be signed in to change notification settings - Fork 111
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
Consolidate identity verification accessibility tests to improve test speed #10359
Consolidate identity verification accessibility tests to improve test speed #10359
Conversation
… speed changelog: Internal, Testing, Consolidate identity verification accessibility tests to improve test speed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
big win!
|
||
expect_page_to_have_no_accessibility_violations(page) | ||
end | ||
|
||
scenario 'how to verify page' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consolidate this one in as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It gets kind of difficult since it's not in the default happy path. Enabling in-person-proofing adds a screen in the middle. We could probably modify the existing step helpers to accommodate it though (especially since it will be enabled more widely soon).
I'm going to merge this for now though.
fill_in t('idv.form.password'), with: Features::SessionHelper::VALID_PASSWORD | ||
click_continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like we're missing accessibility checks for a lot of these screens. I'd wonder if we could throw it in here for the password step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think it'd probably be useful to have some expectation of which pages have been checked to make sure we're covering everything we expect to, but I'd leave that to another PR?
🛠 Summary of changes
I did some testing to ensure that the same pages are tested for accessibility, this should only be a reduction in duplicative tests. This is intended to be a speed improvement on #4000 and the intervening changes since then.
I added a small patch to keep track of which pages the test file was running accessibility checks on to be more sure we aren't losing accessibility coverage. We'd expect to see some duplication since the mobile test shares many of the same pages. The results don't suggest that we've lost any coverage, so this change should be a reduction in time spent without any reduction in test coverage.
Before:
After:
Patch