Fix race conditions and clean up code in regression tests #837
Conversation
spectranaut
force-pushed
the
bocoup-dialog-fix
branch
2 times, most recently
from
264494f to
f03f500
Compare
spectranaut
changed the title
spectranaut
changed the title
|
@tatermelon, @jessebeach, @sh0ji, are one of you available fore review? |
|
@spectranaut the code changes just look like cleanup. Are there any behavior changes to review? |
|
@jessebeach yup just clean up! No behavior changes. |
| const waitAndCheckFocus = async function (t, selector, index) { | ||
| return t.context.session.wait(async function () { | ||
| return t.context.session.executeScript(function () { | ||
| const [selector, index] = arguments; |
There was a problem hiding this comment.
Are selector and index being changed by the inner functions? They're already declared in the upper scope (waitAndCheckFocus).
There was a problem hiding this comment.
hey @sh0ji this is actually a function that will be converted to a string then sent to the browser -- you can read about it here (ctrl-f executeScript): http://seleniumhq.github.io/selenium/docs/api/javascript/module/selenium-webdriver/index_exports_WebDriver.html
It's not executed in the context of this script and arguments can only be provided via the argument object.
| selector = arguments[0]; | ||
| return document.activeElement === document.querySelector(selector); | ||
| }, selector); | ||
| }, 200); |
There was a problem hiding this comment.
Is there a reason this timeout is 200ms while the others are 500ms?
There was a problem hiding this comment.
I put the 200ms waits before I stress tests but found 500 was better -- I'll switch them all to 500 for consistency, thanks for th catch!
|
hey @jessebeach the current failures are expected -- they surface errors in the tests themselves (see comment #837 (comment)) |
|
Hmmm now I have new failures I really don't understand -- something with the test set up: https://travis-ci.org/w3c/aria-practices/builds/417355896?utm_source=github_status&utm_medium=notification |
Maybe something to do with async/await functions? |
spectranaut
force-pushed
the
bocoup-dialog-fix
branch
from
42da138 to
549303b
Compare
spectranaut
force-pushed
the
bocoup-dialog-fix
branch
from
549303b to
efabcd3
Compare
spectranaut
changed the title
|
@jessebeach -- this PR has some code and logic clean up, a couple race condition fixes (in my test code), and some lint error fixing. I'd like it to be accepted into the bocoup branch so I can work on top of these fixes. The errors in the CI are inconsistent and something I'd like to debug separately -- they are errors in the test infrastructure/coming from geckodriver. I'll have to enable logging and I'd like to do this on another PR. |
Test the CI