Enhance Tests: button-move #6300
Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
|
|
||
| }); | ||
|
|
||
| describe('properties', () => { |
There was a problem hiding this comment.
These could arguably be moved into the axe file
There was a problem hiding this comment.
Do you think these disabled tests are adequately covered by the event testing and visual-diff? I much prefer testing the public API rather than inspecting internals. For the aria-label and title, do the aXe tests effectively cover the requirement? All these tests that reference elements in the shadowDOM are subject to break when we change the internals. That's something we would typically aim to avoid - maybe CoPilot wouldn't suggest it because of context, but we definitely would not want consumers inspecting these elements for any reason.
There was a problem hiding this comment.
I think the vdiff and event tests don't specifically test the button having the disabled attribute directly applied, which is not something specifically detectable by those tests. I do agree we want to avoid digging into the shadowDOM unnecessarily.
For the title and aria-label tests, I just ran the axe tests without both of those and they do fail, and then they pass if one is present, so I'll remove that test.
Jira task
These seem generally useful to me, especially for regression prevention and just to double check assumptions.