-
Notifications
You must be signed in to change notification settings - Fork 270
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
Upgrade webdriver.io to v7 and begin migration of tests from sync to asyn #4846
Conversation
✅ Deploy Preview for cbioportalfrontend ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
260b956
to
5f26d9d
Compare
c59ff19
to
9083661
Compare
2061fc4
to
da75381
Compare
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.
LGTM! Just added a few questions/comments about minor things.
- store_artifacts: | ||
path: /tmp/repo/end-to-end-test/remote/junit/errors | ||
path: /tmp/repo/end-to-end-test/local/junit/errors |
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.
were we storing the local artifacts in incorrect locations for all this time?
@@ -633,7 +646,7 @@ describe('Group Comparison Tour', () => { | |||
step++; | |||
}); | |||
|
|||
it('Step 8: Intro to the Survival tab, on the group comparison page.', () => { | |||
it.skip('Step 8: Intro to the Survival tab, on the group comparison page.', () => { |
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.
are these skips intentional? do they stop working after upgrading to v7?
checkBox.waitForExist({ timeout: 10000 }); | ||
$('[data-test="StudySelect"] input').click(); | ||
var input = await $('div[data-test=study-search] input[type=text]'); | ||
await input.waitForExist({ timeout: 10000 }); |
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.
do we still need to call waitForExist
? I was under the impression that await
was already doing that.
|
||
console.log('reportData', reportData); | ||
|
||
var tests = _(reportData) |
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.
we don't seem to use this tests
variable
|
||
const filteredReportData = reportData.tests.filter(test => { | ||
const de = deDupTests(reportData); |
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.
should this be tests
instead?
@@ -0,0 +1,830 @@ | |||
const clipboardy = require('clipboardy'); |
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 is not entirely new, right? mostly copy of the existing specUtils.js
?
We need to eventually upgrade to Webdriver.io 8, which will require we migrate our test style from sync to async per this documentation:
https://webdriver.io/docs/async-migration