Skip to content
This repository was archived by the owner on Jul 29, 2024. It is now read-only.

chore(test): move element_spec.js off of the control flow #4997

Merged
merged 1 commit into from
Nov 7, 2018

Conversation

cnishina
Copy link
Contributor

@cnishina cnishina commented Nov 6, 2018

For the basicConf test suite:

  • Only run the element_spec test in the Protractor config, we will add
    back other specs as we migrate the basicConf off of the control
    flow.
  • In the Protractor configuration file, set SELENIUM_PROMISE_MANAGER to false.
  • Refactor to use async / await.
  • Refactor var to use either const or let.

@cnishina
Copy link
Contributor Author

cnishina commented Nov 6, 2018

Reference #4995

@cnishina cnishina changed the title chore(test): move test suite off of the control flow chore(test): move element_spec.js off of the control flow Nov 6, 2018
@cnishina cnishina force-pushed the you_made_me_promises_promises branch from cb4a7d7 to 99d8192 Compare November 6, 2018 21:13
@cnishina cnishina requested review from juliemr and heathkit November 6, 2018 21:14
@cnishina cnishina force-pushed the you_made_me_promises_promises branch 3 times, most recently from ff2ad33 to 789bcde Compare November 6, 2018 23:02
Copy link
Contributor

@heathkit heathkit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's port these to TS to make them a bit easier to work with.

You can use the no-floating-promises TSLint rule (https://palantir.github.io/tslint/rules/no-floating-promises/) to make sure everything is awaited.


usernameInput.clear().sendKeys('Jane');
expect(name.getText()).toEqual('Jane');
await usernameInput.clear().sendKeys('Jane');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does usernameInput.clear() return? I'm suprised it's not a promise you need to await

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm... For some reason I thought I could still chain these together. I guess not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split these up but they should stay together. You should be able to chain these together.

@cnishina cnishina force-pushed the you_made_me_promises_promises branch 2 times, most recently from 3a469b5 to 55e01ae Compare November 7, 2018 00:34
@cnishina cnishina removed the request for review from juliemr November 7, 2018 00:43
@cnishina cnishina force-pushed the you_made_me_promises_promises branch 4 times, most recently from 95f5f5a to 6a542ab Compare November 7, 2018 00:57
@cnishina
Copy link
Contributor Author

cnishina commented Nov 7, 2018

So far it looks like starting up webdriver-manager + circleci is flaky. I am also removing all the tests to try to pass only my test.

@cnishina cnishina force-pushed the you_made_me_promises_promises branch 3 times, most recently from db651f6 to 7df618f Compare November 7, 2018 01:18
@CrispusDH
Copy link
Contributor

CrispusDH commented Nov 7, 2018

@cnishina could you take a look at my PR - #4998 . I did the same. It was easier to make own PR with it, than make a bunch of comments here. Thanks for your attention. You can just copy paste from element_spec.js and close my PR if you want. I just want to help to reach protractor with selenium 4

@cnishina cnishina force-pushed the you_made_me_promises_promises branch 2 times, most recently from 1b9c24e to e51ad92 Compare November 7, 2018 18:28
Update circleci to support async await.

For the basicConf test suite:

- Only run the element_spec test in the Protractor config, we will add
back other specs as we migrate the basicConf off of the control
flow.
- In the Protractor configuration file, set `SELENIUM_PROMISE_MANAGER` to false.
- Refactor to use async / await.
- Refactor `var` to use either `const` or `let`.
@cnishina cnishina force-pushed the you_made_me_promises_promises branch from e51ad92 to ef7f3e1 Compare November 7, 2018 18:36
@heathkit heathkit merged commit 8af3be7 into angular:selenium4 Nov 7, 2018
@heathkit
Copy link
Contributor

heathkit commented Nov 7, 2018

First step on the journey to Selenium 4!

@cnishina
Copy link
Contributor Author

cnishina commented Nov 7, 2018

So why were these tests failing? I forgot maybe 4-ish await statements. I made all the changes to remove all .then calls. Thanks @CrispusDH I found a diff that helped me out.

Why did Travis fail? Node 6 does not support async / await.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants