-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat(config): Support setting SELENIUM_PROMISE_MANAGER flag via the…
#4023
Conversation
|
CircleCI: ✅ |
|
Travis: ✅ |
lib/config.ts
Outdated
| * | ||
| * At the moment, by default, the WebDriver control flow is still enabled. You can disable it by | ||
| * setting the environment variable `SELENIUM_PROMISE_MANAGER` to `0`. In about one year, the | ||
| * control flow will be disabled by default, and you will be able to re-enable it by setting |
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.
Instead of 'in about one year', maybe "in a webdriver release in Q4 2018"
lib/config.ts
Outdated
| * At the moment, by default, the WebDriver control flow is still enabled. You can disable it by | ||
| * setting the environment variable `SELENIUM_PROMISE_MANAGER` to `0`. In about one year, the | ||
| * control flow will be disabled by default, and you will be able to re-enable it by setting | ||
| * `SELENIUM_PROMISE_MANAGER` to `1`. In about two years, the control flow will be removed for |
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.
Instead of "in about two year", "at a later point"
lib/config.ts
Outdated
| * | ||
| * If you don't like managing environment variables, you can set this option in your config file, | ||
| * and Protractor will handle enabling/disabling the control flow for you. Setting this option | ||
| * will override the `SELENIUM_PROMISE_MANAGER` environment variable. |
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.
Instead say "Setting this option is higher priority than the SELENIUM_PROMISE_MANAGER environment variable".
spec/ts/noCFSmokeConf.ts
Outdated
| const env = require('../environment.js'); | ||
|
|
||
|
|
||
| // The main suite of Protractor tests. |
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.
Please fix this comment.
spec/ts/noCF/smoke_spec.ts
Outdated
|
|
||
| it('should allow handling errors', async function() { | ||
| await browser.get('index.html#/form'); | ||
| await $('.nopenopenope').getText().then(async function(/* string */) { |
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.
Would it make sense for this test to instead use try/catch (that works with async, yeah?)
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.
OMG YOU'RE RIGHT
|
Whoo! A few comments, mostly on wording. |
|
@juliemr all comments addressed |
… config
Closes #3691