Setup playwright tests for accessibility testing#8664
Setup playwright tests for accessibility testing#8664radmorecameron wants to merge 7 commits intoFreeTubeApp:developmentfrom
Conversation
|
Oh yeah for added context these are the failures: For mobile/tablet it doesn't like:
[
{
"description": "Ensure landmarks are unique",
"help": "Landmarks should have a unique role or role/label/title (i.e. accessible name) combination",
"helpUrl": "https://dequeuniversity.com/rules/axe/4.11/landmark-unique?application=playwright",
"id": "landmark-unique",
"impact": "moderate",
"nodes": [
{
"all": [],
"any": [
{
"data": { "accessibleText": null, "role": "navigation" },
"id": "landmark-is-unique",
"impact": "moderate",
"message": "The landmark must have a unique aria-label, aria-labelledby, or title to make landmarks distinguishable",
"relatedNodes": [
{
"html": "<div data-v-2ac94927=\"\" data-v-4afaba5a=\"\" data-v-2fa9d48e=\"\" class=\"ft-flex-box sideNav closed\" role=\"navigation\">",
"target": [".sideNav"]
}
]
}
],
"failureSummary": "Fix any of the following:The landmark must have a unique aria-label, aria-labelledby, or title to make landmarks distinguishable",
"html": "<nav data-v-50bc178e=\"\" data-v-2fa9d48e=\"\" class=\"topNav\">",
"impact": "moderate",
"none": [],
"target": ["nav"]
}
],
"tags": ["cat.semantics", "best-practice"]
},
{
"description": "Ensure that the page, or at least one of its frames contains a level-one heading",
"help": "Page should contain a level-one heading",
"helpUrl": "https://dequeuniversity.com/rules/axe/4.11/page-has-heading-one?application=playwright",
"id": "page-has-heading-one",
"impact": "moderate",
"nodes": [
{
"all": [
{
"data": null,
"id": "page-has-heading-one",
"impact": "moderate",
"message": "Page must have a level-one heading",
"relatedNodes": []
}
],
"any": [],
"failureSummary": "Fix all of the following:Page must have a level-one heading",
"html": "<html lang=\"en-US\">",
"impact": "moderate",
"none": [],
"target": ["html"]
}
],
"tags": ["cat.semantics", "best-practice"]
}
]For desktop, it's a lot of buttons without text/titles/labels and colour contrast issues as well |
There was a problem hiding this comment.
The biggest issue I see with this pull request at the moment is that instead of running inside Electron like FreeTube does, it uses the unsupported, mostly broken web build. Unless it is possible to do it inside Electron, I don't think it is worth merging this, as running tests in an environment that doesn't match how FreeTube is actually used and where most of the functionality is unsupported and broken, is not a proper test.
Additionally could you please rename the pull request and code references to say accessibility testing, so it actually matches what it does (calling accessibility tests, in the wrong environment on only 3 pages "end to end tests" is quite misleading)
P.S. We only have the web build in-place in CI because it serves as a smokescreen for downstream FreeTubeAndroid, to check that Electron only APIs are properly guarded by build flags. FeeeTubeAndroid has reimplemented a lot of the Electron functionality with Android APIs instead so it doesn't use the web build either.
Pull request was converted to draft
Thanks for looking at this @absidue 😄
|
absidue
left a comment
There was a problem hiding this comment.
In the future it would probably be good to add a test build to FreeTube, that uses in-memory nedb databases rather than a production build that shares the development data stores. That's not a major issue for this pull request as it doesn't modify any significant user data but it would definitely need to be done before any tests that do actual data changes are implemented.
Head branch was pushed to by a user without write access
e24b928 to
f831d0c
Compare
Head branch was pushed to by a user without write access
Head branch was pushed to by a user without write access
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Head branch was pushed to by a user without write access
42f9150 to
9b176a9
Compare
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Head branch was pushed to by a user without write access
9b176a9 to
8fc1724
Compare
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
…te test-results directory
todo: look into using it for playwright tests
Head branch was pushed to by a user without write access
8fc1724 to
ef3060e
Compare
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
Pull Request Type
Related issue
related to: #6661 (a static analyzer of some sort might be better for this use case though?)
Description
This PR sets up initial accessibility testing through playwright and axe.
The tests are currently failing due to accessibility issues, I fixed one of the issues but there's still 2 accessibility issues for mobile/tablet and 4 for desktop.
Testing
To run the tests, you can use
yarn run test:e2e, you may need to runnpx playwright installfirstFuture
This is just an initial implementation but in the future it'd probably be worth it to:
Desktop