SuffolkLITLab / ALKiln

Integrated automated end-to-end testing with docassemble, puppeteer, and cucumber.
https://assemblyline.suffolklitlab.org/docs/alkiln/intro
MIT License
14 stars 4 forks source link

a11y tests only sometimes fail with color of `terms` links #824

Open plocket opened 9 months ago

plocket commented 9 months ago

A test first failed with these files:

Then it passed with this scenario report:

Not sure how to troubleshoot this. We might want to start creating accessibility reports regardless of success or failure and they might give us clues.

plocket commented 9 months ago

This one too: https://github.com/SuffolkLITLab/ALKiln/actions/runs/6843450231

plocket commented 9 months ago

Maybe the screen sometimes hasn't finished loading in time for the axe tests. Maybe we should always take a screenshot before running the axe test. It'll give the page a bit more time to load (which isn't a guarantee, but at this point I really don't know how to guarantee that), and if it passes, we'll be able to see if it's because the screen is blank.

BryceStevenWilley commented 8 months ago

TBH, that should be failing. It's using the docassemble color for daterms instead of ours (#408E30 is the upstream value that the json is reporting, and it's known to fail constrast checks, it's why we override it in Assembly Line).

I think the question is why it's not consistently failing. Seems like an aXe issue.

plocket commented 8 months ago

Yes, the problem is indeed that the tests are only sometimes failing even though they should always failing. Sorry that the title wasn't clear. You don't think that slow page loading is the cause of this inconsistency?

This failure started with the GitHub server tests. I think the GitHub server runs faster than our usual servers and that the page actually loads fast enough to get caught by the a11y test. I figure that if the page isn't loaded when the a11y test runs, then it looks "accessible".

That's why my suggestion is to, at least temporarily, take screenshots before checking for a11y so we can see if the page is really loaded.

[That said, an aXe issue is an intriguing idea. I didn't realize they'd have race conditions of their own.]

BryceStevenWilley commented 8 months ago

Taking screenshots before isn't likely to catch the issue though; like you said, it gives the page a bit more time to load. Which IMO just hides the problem instead of fixing it.

I again looked at the best ways to wait in puppeteer, and saw that we're mostly using domcontentloaded. MDN says that:

DOMContentLoaded does not wait for stylesheets to load, however deferred scripts do wait for stylesheets, and the DOMContentLoaded event is queued after deferred scripts... A different event, load, should be used only to detect a fully-loaded page.

Docassemble does have one defer script, fontawesome, so I'm not sure why DOMContentLoaded wouldn't behave as described, but it's worth another shot, since we almost always want images and stylesheets to have loaded on a page.

Is it worth re-opening https://github.com/SuffolkLITLab/ALKiln/issues/189? I can't tell the context of what that issue was trying to fix, but it is certainly relevant here.

I thought that aXe would have been parsing it's own stylesheets instead of using the browser (since it got it the exact color value of the font, and not an approximate value from an image or anything), but on second thought, that doesn't make any sense. Puppeteer and our own code likely have many more issues.

plocket commented 8 months ago

We can try 'load'. I believe we ran into more race conditions where 'load' thought the page was loaded, but it wasn't yet. Trying to access elements with selectors got error messages about the context not existing anymore (which means the page changed in the middle). I could be misremembering, so it's worth a shot, but with some thorough testing. Maybe we can throttle our tests to see if we can catch these kinds of race conditions, but I'm not sure they'll recreate the right issues.

Throttling (I think). Not sure if there are different kinds of throttling.

plocket commented 8 months ago

We might be able to listen for the daPageLoad event. It waits for the jquery validator to exist, whenever in the flow that happens. Our custom datatypes depend on that event. I'm not sure if they would run before or after our event listener.