-
Notifications
You must be signed in to change notification settings - Fork 364
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
Add tests for disclosure/disclosure-faq.html #846
Conversation
@tatermelon, @jessebeach, @sh0ji, this is ready for review. |
c0b7319
to
5e28051
Compare
@spectranaut, looks like we are getting the session errors. I'm restarting the build another time to see if it happens again. |
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.
Just a few nits. Nothing major.
|
||
let buttons = await t.context.session.findElements(By.css(ex.buttonSelector)); | ||
for (let button of buttons) { | ||
await button.click(); |
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.
Huh, I thought having an await in a for..of loop would cause a problem, but the Iteration Protocol handles it automatically. Very readable, very nice.
let answers = await t.context.session.findElements(By.css(ex.buttonSelector)); | ||
for (let answer of answers) { | ||
t.true( | ||
await answer.isDisplayed(), |
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.
This might fail if true
isn't asynchronous. Maybe pull the await
to the line above, store the result and then call true
with the variable as the first argument. If this pattern is prevalent, it might be a source of your timing issues.
await button.sendKeys(Key.ENTER); | ||
|
||
t.true( | ||
await waitAndCheckExpandedTrue(t, button), |
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.
ditto to above and for all the rest below.
Tests written, only blocking on acceptance of race condition PR: #837