Skip to content
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

refactor: switch e2e tests to async/await #4705

Merged
merged 2 commits into from
Jun 1, 2017

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented May 21, 2017

Switches to using async/await in the e2e tests. This should make it a bit easier to deal with Protractor's async APIs.

Note: The CI failures are unrelated to the changes in this PR and are the same as the ones from #4702 and #4703.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 21, 2017
Switches to using async/await in the e2e tests. This should make it a bit easier to deal with Protractor's async APIs.


describe('scroll blocking', () => {
beforeEach(() => browser.get('/block-scroll-strategy'));
afterEach(() => clickOn('disable'));

it('should not be able to scroll programmatically along the x axis', async (done) => {
it('should not be able to scroll programmatically along the x axis', asyncSpec(async () => {
Copy link
Member

Choose a reason for hiding this comment

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

According to @vikerman this asyncSpec wrapper should be unnecessary w/ jasminewd2

Copy link
Member Author

@crisbeto crisbeto May 23, 2017

Choose a reason for hiding this comment

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

Hmm looks like it. Initially, I was playing around with passing in the done function and calling it at the end, which ended up timing out for failures, because I wasn't wrapping it in a try/catch and calling done.fail. I guess that was overriding the custom handling from jasminewd. I've removed the asyncSpec now and it behaves as expected.

@jelbourn
Copy link
Member

LGTM for the code changes, but what's up w/ the screenshot diffs here?
cc @tinayuangao

@jelbourn
Copy link
Member

Poke @tinayuangao on the screenshot stuff; not clear to me what's happening on the results page

@tinayuangao
Copy link
Contributor

LGTM.

The screenshot tests failed because we changed the test name, so the test images are unavailable to old test names.
After we merge this PR & update goldens, the images & tests should be fine.

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels May 31, 2017
@mmalerba mmalerba merged commit 50f1c26 into angular:master Jun 1, 2017
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants