-
Notifications
You must be signed in to change notification settings - Fork 344
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
WIP: add tests for toolbar/toolbar.html example #845
Conversation
fb4bcd5
to
babb7a2
Compare
babb7a2
to
0ae7157
Compare
@mcking65, currently 2 -- I only wrote two tests (left arrow and right arrow) for the issues I had questions with. I can write the remaining tests and comment them out as we discussed. |
The last two commits add tests (expect for the "aria-disabled") and comment them out, with a reference to #847 |
@jessebeach, @tatermelon, @sh0ji, even though travis is failing, this is ready for review. The failures are related to a race condition that is not specific to these tests. @spectranaut is debugging the failures separately. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@spectranaut This is still a work in progress. Let me know when it should be reviewed for accept.
|
||
const waitAndCheckFocus = async function (t, selector) { | ||
return t.context.session.wait( | ||
async function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be async? Everything in the function body is synchronous, unless executeScript
is missing an await
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just read Matt's comment about the commented out tests.
I have no major comments for the active code. Onward and upward.
Waiting on questions on toolbar review: #539 (comment)
Also blocking on acceptance of race condition PR: #837