Refs #33620 - Approach on how to structure accessibility tests #17074

tushar5526 · 2023-07-15T16:52:23Z

Refs ticket-33620
This is a draft PR to get some feedback on my approach for accessibility tests and is not meant to be merged.

This approach focuses on using the existing suite of selenium tests to be used for accessibility tests. Based on @carltongibson's comment #16372 (comment) I have added a function that takes the current webpage in the selenium instance, writes it to a html file named ClassName.TestName.Count and stores it in a folder named snapshot.

These snapshots can then be used by pa11y-ci tool using the pa11y-ci-reporter-html reporter to generate an HTML report for the accessibility tests as follows.

I have written a small helper script to generate the pa11y report. Also, we can use a flag to tell CI when to generate snapshots. These snapshots can then be downloaded as artificats in GitHub Actions.

Integration with selenium is quite straight forward - we will have to just call take_snapshot function in our test cases whenever we want that a UI change should be tested. For example, calling take_snapshot after we add 4 rows in django admin.

Following image is the generated report for each snapshot

Detailed reports look like this for each of the above rows.

pa11y also provides options to configure the level of accessibility testing. We can start with a lower compliance and keep increasing the levels, as we keep solving accessibility issues.

sarahboyce · 2023-07-16T11:43:27Z

Hi Tushar 👋 this looks really cool!

What we want to achieve:

be able to tell if a patch/PR has a negative effect on accessibility
be able to tell if a patch/PR has solved an accessibility issue
be able to identify existing issues to create tickets from

I think this has potential, I'm going to give some thoughts and hope they're useful.

Thoughts on a migration process

In order to achieve the goal "be able to tell if a patch/PR has a negative effect on accessibility" we need to establish a baseline.

pa11y also provides options to configure the level of accessibility testing. We can start with a lower compliance and keep increasing the levels, as we keep solving accessibility issues.

This is interesting ☝️
I can see from your screenshots that we are failing on some accessibility criteria. Hopefully there are some accessibility criteria we are passing on. If we can configure to only test against the criteria we are passing on, and raise tickets to resolve the criteria we fail against, then that should achieve a baseline. This can then act as a road map for fixing the issues and make it easy for people to tell if they have in fact "fixed" a particular issue (as they can then add the new criteria to our config and show the ci doesn't fail).
The advantage of this approach is you could add in some snapshot tests to gain some coverage without having to worry about solving the accessibility issues in the same patch/PR.
Another approach is we solve an area at a time, then add the snapshot test in 🤔 but that would take quite a bit of time.

But I'm not sure what you had in mind?

Side note: this has lots of similarities to #16963 trying to improve UI testing 🎉

I would take a look there and see if any of the discussions can help guide/inspire this.
Would also be interesting if we can combine the work somehow 🤔 as if we change the admin UI I imagine we would also want to check both the accessibility and that there is no visual regression.

Anyway I have fiddled around with the selenium test suite and the GitHub actions here quite a bit so shout if you need any support with it.

If you're struggling for input, you might want to advertise this PR in #contributor-discussions in the Django Discord or on the Django Internals topic of the forum. Just be aware that this might be a slow burn ticket as it needs a bit of thought 👍

tushar5526 · 2023-07-16T15:20:35Z

Hi @sarahboyce thanks for the feedback and for defining the expectations.

But I'm not sure what you had in mind?

Initially, I thought of this CI to be something that generates a report on the current status of accessibility, and something that is run only at specific points. End result of it being a report on accessibility.

But based on your comment.

What we want to achieve:

I went on to look for more alternatives for accessibility testing and found IBM's Equal Access which seems more apt for our use case. I have not deep-dived into it, but it seems to me we can use Baseline to set the "baseline" to the current failing accessibility issues and then new PRs can be tested for point 1.

point 2 and point 3 also seem possible using this.

sarahboyce · 2023-07-16T18:43:29Z

Just to throw more ideas out there 😅
In my head, we would have a method on SeleniumTestCase like def assertPageIsAccessible(self, ignore=None) where ignore is an optional list of error codes to "ignore". We achieve this with a tool like Axe (maybe download the min script from here) and use selenium's execute_script method to run the test. There is an example of what I have in mind here.

If we have something that asserts during the test we won't need to write the html anywhere and run a separate npm script against the generated files.

I think when playing around with the different tools suggested and it will become clear what the limitations are. So I would just pick your favourite and then see if it hits a blocker. 🤷

sabderemane · 2023-07-20T09:34:54Z

As I started something which is related to this, I will handle the Github Actions part and introduce the report in it

tushar5526 · 2023-08-02T20:15:43Z

I evaluated these tools and found https://github.com/IBMa/equal-access it to be actively maintained and have features (like baseline) to support our testing needs and is most configurable. I will try putting something together soon.

tushar5526 · 2023-08-05T15:38:25Z

PoC using IBM's Equal Access

IBM's equal access provides a achecker CLI that runs accessibility tests on either html files or URLs. It can also compare the test results/violations against a pre-defined baseline which we can specify in a folder.

The baselines folder in this demo is the results folder generated on the first run (which I renamed to baselines) so that it can serve as a "baseline" in subsequent runs for the tests.

We can run a simple python -m http.server or any other webserver on the baselines folder to see the current violations as shown in the video.

I had to hardcode the PORT number on which the selenium tests run so that the folder structure remains same for multiple runs.

Also, we have a choice to either run is_page_accessible once on every unique page (basically every unique URL) or append page URLs with #(id) to allow running is_page_accessible on multiple runs. (This might cause a problem later as the order of calling is_page_accessible has to be same or we update the baselines folder accordingly)

Any contributor can solve an accessibility issue and remove it from the baselines and this approach also tests that no new accessibility issues get added in a new PR.

The example in this demo was generated for this test - ./runtests.py --selenium=chrome --parallel=1 admin_changelist.tests.SeleniumTests.test_add_row_selection - as you can see the folder hierarchy in baselines folder.

Screen.Recording.2023-08-05.at.8.55.38.PM.mov

sarahboyce

I think this is getting closer! I still have some thoughts, but good job pulling this together ⭐

sarahboyce · 2023-08-06T16:51:28Z

tests/baselines/summary_2023-08-05T15:01:52.604Z.json

@@ -0,0 +1,52 @@
+{


These baseline summaries seem to be very repetitive, do we need them? I find the individual detailed report more useful

These summaries are auto-generated and persist over multiple runs, moreover, I was thinking of not to setting results folder to a path inside Django project, instead it can go to a location like /tmp or ~/.acheker/results.

I added the results folder here to showcase everything.

sarahboyce · 2023-08-06T16:55:55Z

tests/.achecker.yml

+# Default: json
+outputFormat:
+    - json
+    - html


I think I prefer the json output and would remove the html one, as it's then less noise in pr's and more readable as a changeset 🤔

Do you think we should provide a script to re-generate baselines ?

I kept the html format so that users could quickly go through the accessibility issues using the browser as I demonstrated in the video.

Also, we can skip adding a results folder as a part of this.

Do you think we should provide a script to re-generate baselines ?

I think that's probably a good idea, would make sense to document in a section within the contributing docs also.

I kept the html format so that users could quickly go through the accessibility issues using the browser as I demonstrated in the video.

Ok maybe we git ignore the html files, then we can still generate them easily but it keeps the change-set small. Similar to how we don't commit a coverage report for example 🤔

I’d say the best output for us would be a pass/fail and assertion error message like all other Selenium tests produce. We don’t need a full JSON or HTML report, can just run the test suite locally like we would for other types of testing.

sarahboyce · 2023-08-06T17:08:15Z

django/test/selenium.py

+        self.counter[self.selenium.current_url] = id + 1
+
+        command = ["npx", "achecker", self.selenium.current_url + "#" + str(id)]
+        output = subprocess.run(command, capture_output=True, text=True)


At the minute there isn't a strong link between the test and the output created from the test based off naming (if I wanted to update the baseline it would feel random). As tests can share urls then with the counter, the order you run tests also matter. Can we have the test name in the filepath?
I also don't love the naming convention with localhost etc - is this configurable at all? Or does it have to be generated from the url?

Can we have the test name in the filepath?

No, this is not possible (or if it is, I was not able to find any docs on it). Basically the baseline folder is autogenerated and the folder is structured according to the path of the URL on which achecker is run.

Another way to omit localhost is to add a hostname like django-selenium-test in the /etc/hosts file on the local system (on Linux, not sure about Windows or Mac) and point it to 127.0.0.1 similar to how localhost points to 127.0.0.1 - but this will make it difficult for a lot of other things.

There is a boilerplate folder in the tool's repo - https://github.com/IBMa/equal-access/tree/master/accessibility-checker/boilerplates - that have some examples on baselines and other features.

I found this https://github.com/IBMa/equal-access/tree/master/accessibility-checker/src#apis getCompliance which could be useful in naming baselines. I will take a look and come back.

Nice 👍 you might also be able to write the html from the page into a tmp file with a well named filename/path and run the achecker against it (which might dictate the name of the report). Not sure if that will work though

sarahboyce · 2023-08-06T17:10:39Z

tests/baselines/localhost_12345/admin/auth/user/#0.json

@@ -0,0 +1,367 @@
+{


We need the @django/accessibility team to have a look at one of these outputs and confirm that what it is flagging up is valid, understandable, useful and actionable.
If we want to use this tool, this report should be identifying real issues here we want to solve 👍

You can also run a webserver on the results folder and see the *.html files. They provide a good HTML report with intuitive UI which will help.

sarahboyce · 2023-08-06T17:15:59Z

django/test/selenium.py

+        id = self.counter.get(self.selenium.current_url, 0)
+        self.counter[self.selenium.current_url] = id + 1
+
+        command = ["npx", "achecker", self.selenium.current_url + "#" + str(id)]


The selenium test suite github actions are now broken unfortunately. I think you will need to have to setup nodejs npm and install this package. We would also need to update the instructions for running the selenium test suite
I think that's the main disadvantage of this approach as the python selenium test suite is now reliant on some javascript setup which people might find surprising 🤔

Yes, or we can make it optional by default - Otherwise, we can use an a11y checker that has a use script injection, generate results for all the test files, create baselines and then write some sort of logic to generate baselines with those previous results.

Three things here:

We should be using Axe rather than IBM’s Equal Access Accessibility Checker. Nothing wrong with Equal Access per se, but Axe is the gold standard everywhere else.

I’d have preferred we load Axe (or any other checker if we changed our mind) within the Selenium browser via script injection as proposed in Fixed #33620 -- Allowed running accessibility checks on selenium tests. #16372, rather than invoke a command line tool. Similarly to what https://github.com/mozilla-services/axe-selenium-python does. If that package isn’t good enough for us, I’d still rather we use that approach and re-implemented it ourselves.

If we did decide to go with a command-line tool integration – we shouldn’t be using npx for this. We should instead have a package.json where we define our dependencies, and use the tool’s CLI directly.

I prefer a browser-injected script over a command line tool because:

It’ll allow us to run tests before-after a specific browser interaction rather than being limited to "one test per URL at most"

Like @sarahboyce mentions, it’d be annoying to have our tests dependent on Node & npm at runtime. With a browser script, it’s less of a requirement.

It’ll likely be faster than having to invoke a command line tool, and the output will be easier for us to process

thibaudcolas

Exciting to see this happening 😄… but before we proceed here, I think we need to follow @felixxm’s prompt in #16372 (comment) and have more discussion about the possible solutions.

In particular, personally I’d have said #16372 was a better approach. I think the package we used there is good enough, but if that’s not the case we should consider re-implementing it as per our requirements rather than change approach completely.

On the desired output – I think we should strive to have the same as what we have for every other kind of test: a pass/fail, and a good assertion message in the test output on failure. Having to sift through JSON/HTML reports will only add extra steps to get to the results.

@sarahboyce discussed how we migrate towards having this in place despite the presence of existing issues. This should be really simple – most accessibility checkers support configuration to either skip specific checks, or skip checks on specific elements. As part of introducing this new type of testing, we’ll simply add enough of those exclusions so the tests pass. Over time, as we address issues, we remove the exclusions one after the other.

Edit: have followed up with the ticket at https://code.djangoproject.com/ticket/33620 with my suggestions for a path forward.

thibaudcolas · 2023-08-18T06:54:25Z

django/test/selenium.py

+        id = self.counter.get(self.selenium.current_url, 0)
+        self.counter[self.selenium.current_url] = id + 1
+
+        command = ["npx", "achecker", self.selenium.current_url + "#" + str(id)]


Three things here:

We should be using Axe rather than IBM’s Equal Access Accessibility Checker. Nothing wrong with Equal Access per se, but Axe is the gold standard everywhere else.

I’d have preferred we load Axe (or any other checker if we changed our mind) within the Selenium browser via script injection as proposed in Fixed #33620 -- Allowed running accessibility checks on selenium tests. #16372, rather than invoke a command line tool. Similarly to what https://github.com/mozilla-services/axe-selenium-python does. If that package isn’t good enough for us, I’d still rather we use that approach and re-implemented it ourselves.

If we did decide to go with a command-line tool integration – we shouldn’t be using npx for this. We should instead have a package.json where we define our dependencies, and use the tool’s CLI directly.

I prefer a browser-injected script over a command line tool because:

It’ll allow us to run tests before-after a specific browser interaction rather than being limited to "one test per URL at most"

Like @sarahboyce mentions, it’d be annoying to have our tests dependent on Node & npm at runtime. With a browser script, it’s less of a requirement.

It’ll likely be faster than having to invoke a command line tool, and the output will be easier for us to process

thibaudcolas · 2023-08-18T06:55:03Z

django/test/selenium.py

@@ -134,3 +139,17 @@ def disable_implicit_wait(self):
            yield
        finally:
            self.selenium.implicitly_wait(self.implicit_wait)
+
+    def is_page_accessible(self):


We need to be careful with naming here, we don’t want to conflate "automated accessibility checks pass" with "the page is accessible".

thibaudcolas · 2023-08-18T06:56:02Z

tests/.achecker.yml

+# Default: json
+outputFormat:
+    - json
+    - html


I’d say the best output for us would be a pass/fail and assertion error message like all other Selenium tests produce. We don’t need a full JSON or HTML report, can just run the test suite locally like we would for other types of testing.

thibaudcolas · 2023-11-16T19:12:20Z

Just noting this is still in progress behind the scenes – we’re currently discussing bringing https://github.com/mozilla-services/axe-selenium-python into the Django org to use a different approach.

I don’t think we’ll go ahead with this PR if this all works out, but let’s see what happens.

thibaudcolas · 2024-02-10T17:07:12Z

The DSF board has recommended against transfer of the package to the Django org, so we’re now looking at alternative options. The discussion is in Package maintainers? axe-selenium-python#192 if anyone wants to follow along.

Edit: September 2024 update: transfer is in progress

sabderemane requested a review from a team July 20, 2023 09:29

tushar5526 force-pushed the 33620 branch from f9d0c19 to bb4d911 Compare August 5, 2023 15:20

feat: demo accessibilty tests ci using IBM's equal access

8078f6c

tushar5526 force-pushed the 33620 branch from bb4d911 to 8078f6c Compare August 5, 2023 15:23

sarahboyce added the selenium Apply to have Selenium tests run on a PR label Aug 6, 2023

sarahboyce reviewed Aug 6, 2023

View reviewed changes

thibaudcolas suggested changes Aug 18, 2023

View reviewed changes

sarahboyce added the Djangonauts 🚀 label Feb 1, 2024

Uh oh!

Refs #33620 - Approach on how to structure accessibility tests #17074

Are you sure you want to change the base?

Refs #33620 - Approach on how to structure accessibility tests #17074

Uh oh!

Conversation

tushar5526 commented Jul 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sarahboyce commented Jul 16, 2023

Uh oh!

tushar5526 commented Jul 16, 2023

Uh oh!

sarahboyce commented Jul 16, 2023

Uh oh!

sabderemane commented Jul 20, 2023

Uh oh!

tushar5526 commented Aug 2, 2023

Uh oh!

tushar5526 commented Aug 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PoC using IBM's Equal Access

Uh oh!

sarahboyce left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tushar5526 Aug 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tushar5526 Aug 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thibaudcolas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thibaudcolas commented Nov 16, 2023

Uh oh!

thibaudcolas commented Feb 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

tushar5526 commented Jul 15, 2023 •

edited

Loading

tushar5526 commented Aug 5, 2023 •

edited

Loading

tushar5526 Aug 7, 2023 •

edited

Loading

tushar5526 Aug 7, 2023 •

edited

Loading

thibaudcolas left a comment •

edited

Loading

thibaudcolas commented Feb 10, 2024 •

edited

Loading