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

Regression Tests: Enable disclosure_navigation.js test to scroll to element and click in executeScript context #3059

Merged
merged 5 commits into from
Jul 27, 2024

Conversation

howard-e
Copy link
Contributor

@howard-e howard-e commented Jul 1, 2024

Fixes #2996. The previous fix, #2997 didn't fully resolve the issue described in #2996 (comment).

Update ../tests/disclosure_navigation.js test > "aria-current" attribute on links to scroll to link before click event and do click inside executeScript.


WAI Preview Link (Last built on Mon, 01 Jul 2024 20:50:58 GMT).


@howard-e howard-e added the regression-testing Related to AVA regression tests of example pages or AVA framework implementation within repo label Jul 1, 2024
@howard-e
Copy link
Contributor Author

howard-e commented Jul 1, 2024

For revieiwers, I re-ran this test locally 50 times using for i in {1..50}; do npm run regression -- -t test/tests/disclosure_navigation.js; done without any errors being thrown.

I also re-ran the regression tests for this PR in CI 10 times without any errors being thrown. Unless something changes, I feel confident that this should remove the flakiness with the test described in #2996.

@howard-e howard-e marked this pull request as ready for review July 1, 2024 21:05
@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed Regression test fix.

The full IRC log of that discussion <jugglinmike> Topic: Regression test fix
<jugglinmike> github: https://github.com//pull/3059
<jugglinmike> howard-e: This is a change related to the viewport with a specific test
<jugglinmike> howard-e: It aligns the test with others in the suite
<jugglinmike> howard-e: previously, there was some flakiness--perhaps because the click was happening while scrolling was in progress
<jugglinmike> howard-e: I could reproduce the flakiness locally with the previous version, but I couldn't reproduce it locally with this version
<jugglinmike> jongund: I'm looking at the code right now, and it looks good to me
<jugglinmike> jongund: I'll verify the fix locally and submit a review
<jugglinmike> Matt_King: Thank you!
<jugglinmike> s/you/you, jongund and howard-e/

@jongund jongund self-requested a review July 23, 2024 18:26
Copy link
Contributor

@jongund jongund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good to me.
I ran it locally on my mac power book and it ran fine.

@mcking65 mcking65 merged commit 1637413 into main Jul 27, 2024
17 checks passed
@mcking65 mcking65 deleted the update-disclosure-navigation-test branch July 27, 2024 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-testing Related to AVA regression tests of example pages or AVA framework implementation within repo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failing disclosure_navigation.js test
4 participants