Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
roseb89
requested review from
Toxiapo,
clarissarichard,
milliesavalia and
russellcullen
and removed request for
clarissarichard
| await page.keyboard.press("Tab"); | ||
| await expect(accountPage.availableUsernameButton).toBeFocused(); | ||
| await accountPage.availableUsernameButton.press("Enter"); | ||
| await expect(accountPage.availableUsernameMessage).toBeVisible(); |
There was a problem hiding this comment.
Where is the focus expected to be after the "username is available" message is displayed?
There was a problem hiding this comment.
The focus should jump back to the input field. We should also add this behavior to the unavailable test.
| await page.keyboard.press("Tab"); | ||
| await expect(accountPage.availableUsernameButton).toBeFocused(); | ||
| await accountPage.availableUsernameButton.press("Enter"); | ||
| await expect(accountPage.unavailableUsernameMessage).toBeVisible(); |
There was a problem hiding this comment.
Would it make sense to mock the username availability like we have in account.spec.ts and just confirm the focus is correct in this test?
| /* Tests existing username rejection. | ||
| Account is temporary; username availability reset expected in 30 days. */ | ||
| export const USED_USER_NAME = "tester"; | ||
| export const AVAILABLE_USER_NAME = "testuser"; |
There was a problem hiding this comment.
How do we know this username is available?
There was a problem hiding this comment.
I'd setup this username yesterday. I’m considering deleting it and re-registering via the NYC flow to test its longevity beyond 30 days, though I'm unsure of Sierra’s database flush frequency. The AVAILABLE_USER_NAME is available because this is not a full end-to-end with application submission. It's only the account's page.
There was a problem hiding this comment.
I think mocking the username would eliminate the need for this process. Thoughts on trying that?
There was a problem hiding this comment.
I think mocking the username would eliminate the need for this process. Thoughts on trying that?
The first approach of having the real check username endpoint enabled is preferred but I will defer to the mock approach since the user name changes will cause this test to be brittle.
There was a problem hiding this comment.
The first approach of having the real check username endpoint enabled is preferred
I'm curious, why is that approach preferred in this context?
There was a problem hiding this comment.
For this particular context, do we feel like using the real username availability API will change what we're testing from an accessibility point of view?
I'm not sure if the mock is going to catch this defect. Since, the tests I'm writing is in response to this ticket. https://newyorkpubliclibrary.atlassian.net/browse/SWIS-366
There was a problem hiding this comment.
I guess to dig a little further -- what about mocking would make this defect/test in particular behave different? If that's the only concern, maybe it makes sense to try it out to validate or disprove our assumptions? I am trying to understand our concerns and testing methodologies here is all
There was a problem hiding this comment.
\
I guess to dig a little further -- what about mocking would make this defect/test in particular behave different? If that's the only concern, maybe it makes sense to try it out to validate or disprove our assumptions? I am trying to understand our concerns and testing methodologies here is all
My existing Accessibility UI tests do not use mocks, but the live environment. @clarissarichard propose a mock solution twice in this PR. Since, that wasn't my original approach, I have to "dig a littler further" to explain my reasonings on using the real live approach to validate or disapprove of a suggested mock strategy that may or may not be the best solution.
There was a problem hiding this comment.
That makes total sense. I definitely didn't mean to add extra cycles to your plate -- I’m just interested in the tradeoffs since we have two different patterns being discussed. Understanding your reasoning for sticking with the live environment and concerns with mocking helps us all align on the best methodology for these a11y tests moving forward
There was a problem hiding this comment.
I appreciate the deep dive. It was a valuable exercise to align on these A11y best practices, and I’m glad we could clarify why the live environment is necessary to catch these specific focus-loss defects. It gives us a much stronger methodology for these types of tests moving forward.
|
@russellcullen @Toxiapo @clarissarichard I spoke with Antonio, and he confirmed that our test accounts in DEV/QA are persistent and last indefinitely. They are only flushed during major upgrades or the annual production data backfill/merge or when we delete them manually or through our automated Sierra e2e tests. Given this stability, the Live Environment remains the most reliable way to test the SWIS-366 focus-loss defect without masking the issue through an instant mock. Since the maintenance overhead is minimal (occurring roughly once a year), I’d like to move forward with the live-integration approach to ensure maximum A11y coverage |
|
@russellcullen @Toxiapo @clarissarichard To clarify the mock vs. live-environment choice: focus-management is highly sensitive to network timing and dynamic rendering. Using a mock provides an 'instant' response that can create an artificial success state, effectively masking the focus-loss issues that occur during the loading window in the real world. Testing in the live environment ensures we are capturing the full page context—verifying that focus doesn't unexpectedly jump to other components while the username check is "in-flight. |
4 participants
Description
Tickets:
Motivation and Context
How Has This Been Tested?
Checklist: