Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lots of test flake when upgrading from 4.7.0 to 4.8.0. #349

Closed
mockdeep opened this issue Oct 28, 2023 · 6 comments
Closed

Lots of test flake when upgrading from 4.7.0 to 4.8.0. #349

mockdeep opened this issue Oct 28, 2023 · 6 comments
Assignees
Labels
in sprint Ticket is in current sprint PRIORITY: high High priority item; should be scheduled in this or next sprint QA SIGNOFF: passed This ticket has passed QA

Comments

@mockdeep
Copy link

mockdeep commented Oct 28, 2023


- `axe-core-rspec` version: 4.8.0
- Browser: Firefox

I'm having trouble figuring out why we're seeing this issue. Even when logging in our users, it will occasionally report accessibility violations like this:

Found 2 accessibility violations:

1) landmark-one-main: Document should have one main landmark (moderate)
    https://dequeuniversity.com/rules/axe/4.8/landmark-one-main?application=axeAPI
    The following 1 node violate this rule:
    
        Selector: html
        HTML: <html><head></head><body></body></html>
        Fix all of the following:
        - Document does not have a main landmark
        
2) page-has-heading-one: Page should contain a level-one heading (moderate)
    https://dequeuniversity.com/rules/axe/4.8/page-has-heading-one?application=axeAPI
    The following 1 node violate this rule:
    
        Selector: html
        HTML: <html><head></head><body></body></html>
        Fix all of the following:
        - Page must have a level-one heading
        
Invocation: axe.run({:exclude=>[]}, {:rules=>{"document-title"=>{:enabled=>false}, "html-has-lang"=>{:enabled=>false}}}, callback);

Prior to running a11y checks, we assert that we're fully on the following page. E.g.:

fill_in('session_email', with: user.email) 
fill_in('session_password', with: user.password)
click_button('Log in')
expect(page).to have_content('Manage')
expect(page).to be_axe_clean

However, it looks like axe-core is checking against a blank page, even though we've asserted that the page is loaded. These are server-rendered Rails pages without any client-rendering. There's a bunch of other JS loading on the page, but it doesn't do much on the home screen.

There was a little flake on 4.7.0, but we found that we could retry the a11y checks after a couple of seconds and they would work. Unfortunately, the retry logic doesn't resolve the issue on 4.8.0. It retries several times and still fails.

@mockdeep
Copy link
Author

Extra baffling, we added an extra check to make sure content is rendered before checking a11y, and it still fails as if the page is blank:

within_window(current_window) do
  expect(page).to have_css('div')
  expect(page).to be_axe_clean
end

@straker
Copy link
Contributor

straker commented Nov 20, 2023

First, some relevant comments from the axe-core issue

This seems to happen most often early in the test while logging in the user, and less so later in the spec. One thing I'm wondering is whether calling be_axe_clean in rapid succession might cause something like this. We call it on every page visit and button click. Is there any caching or asynchronous behavior in axe that might have a lingering affect? For more context, our login method looks like this:

visit(signin_path)
expect(page).to have_button('CONTINUE')
expect(page).to be_axe_clean
fill_in('session_email', with: user.email)

click_button('CONTINUE')
expect(page).to have_button('LOG IN')
expect(page).to be_axe_clean
fill_in('session_password', with: user.password)

click_button('LOG IN')
expect(page).to have_content('MANAGE')
expect(page).to be_axe_clean

What's odd is that we see the failure most frequently on the 3rd be_axe_clean call. Is it possible there is some lingering logic or caching from the previous calls?

@straker
Copy link
Contributor

straker commented Nov 20, 2023

Continuing the discussion from the axe-core issue:

We did recently encounter a Java bug where how we were opening a blank page for the axe-core audit caused issues, so we'd like to run some experiments in Ruby to see if the same problem occurs outside of Java.

We're also wondering if this might be a browser driver issue. Did you per-chance upgrade your browser driver version at the same time you updated axe-core? If so could you try downgrading the browser driver package to what it was before to see if that makes the tests less flakey?

Also, when the log in button is clicked, what is the flow that happens? A lot of log in flows will perform redirects and it could be that with axe-core being faster as tests were turned off that it's trying to run axe-core during a redirect flow where the page it's currently on is currently blank. It would depend on when the "Manage" content is there and what happens after that.

Lastly, would it be possible to create a reproducible test case that we could test on our end? That would better help us debug the issue and make sure it's not an issue with our ruby library.

@mockdeep
Copy link
Author

@straker thanks for the followup.

Did you per-chance upgrade your browser driver version at the same time you updated axe-core?

No, we've been on selenium-webdriver version 4.10.0 since June.

Also, when the log in button is clicked, what is the flow that happens?

Each step is a form post that redirects to the next page, but the assertions ensure the redirect is complete before running the accessibility checks. The "Manage" content shows up after the final redirect is completed.

Lastly, would it be possible to create a reproducible test case that we could test on our end?

Unfortunately, I don't know how to accomplish this. I'd be happy to do a screen share with you and walk you through things on our end. I did set it up on an open source project and put it on a repeat on CI, but so far haven't seen any flake. I'm a little surprised, actually, because on that project I haven't even added any assertions before running accessibility checks.

@dequejenn dequejenn added PRIORITY: high High priority item; should be scheduled in this or next sprint in sprint Ticket is in current sprint labels Jan 29, 2024
@michael-siek
Copy link
Member

Hey @mockdeep just following up to see if this is still an issue.

@mockdeep
Copy link
Author

Hey @michael-siek we've added a requirement to ensure that the next page is fully loaded after navigation and the amount of flake seems to have gone down significantly. In previous versions we were able to rescue from the various errors that popped up from axe-core and retry after a delay, but with the most recent version it almost seems like running the a11y checks causes navigation to halt, so retrying no longer works. Adding a required matcher makes sure the page is fully loaded, so I don't think there will be any more need for the retry.

I'll close this for now and I can report back in if anything else comes up.

@padmavemulapati padmavemulapati added the QA SIGNOFF: passed This ticket has passed QA label Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in sprint Ticket is in current sprint PRIORITY: high High priority item; should be scheduled in this or next sprint QA SIGNOFF: passed This ticket has passed QA
Projects
None yet
Development

No branches or pull requests

5 participants