Refactor a11yAudit for auth-method list test #2520
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
||
| currentSession().set('data.theme', 'dark'); | ||
| await a11yAudit(); | ||
| }); | ||
| }); | ||
|
|
||
| test('users can navigate to auth methods with proper authorization', async function (assert) { |
There was a problem hiding this comment.
None blocking comment: the group of tests which do not belong to the a11Audit aren't group by a module statement. Is this in purpose?
Following the pattern we have in the acceptance test, i.e: users list-test, auth-methods delete-test, we wrap them in a module, such as 'Acceptance | auth-methods | list'.
There was a problem hiding this comment.
Thanks for pointing this out as looking at this is confusing. All the tests in this file are wrapped in a module named 'Acceptance | auth-methods | list'. What I did was add another module within that just for the a11yAudit tests. The main reason for this is we get access to all of the work done in the beforeEach. I wonder if maybe I should move the whole a11yAudit module to the bottom of the parent test module to make this less confusing?
There was a problem hiding this comment.
Ahh gotcha. Now I see it,
So the a11yAudit block is within the Acceptance | auth-methods | list block.
Yeah, not a strong opinion which can be the best formula for that. I am fine with it 😉 and you are leading this initiative, so whatever you think is best, let's keep with it and we can review later if its a problem.
Great work!!
| }); | ||
|
|
||
| module('a11yAudit', function () { | ||
| test.each('auth-methods', ['light', 'dark'], async function (assert, data) { |
There was a problem hiding this comment.
Big fan of using test.each for this, I didn't realize this actually existed and wanted to parametrize some tests before and would've loved to have done this instead!
Description
This PR introduces the new changes to our a11y auditing in our acceptance tests. I have also added some documentation for future reference. Please make any recommendations if the README entry isn't clear or complete.
I kept this PR small for easy review of this new change. In the future, refactoring the whole CRUDL of a specific resource should be fine.
Screenshots (if appropriate)
How to Test
Checklist