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

test: refactor utils.waitForEvents into utils.waitEvent #2336

Merged
merged 2 commits into from Apr 9, 2018

Conversation

aslushnikov
Copy link
Contributor

This simplifies waiting for events in tests.

This simplifies waiting for events in tests.
@@ -380,7 +377,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p
describe('Page.Events.DOMContentLoaded', function() {
it('should fire when expected', async({page, server}) => {
page.goto('about:blank');
await waitForEvents(page, 'domcontentloaded', 1);
await waitEvent(page, 'domcontentloaded', 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

extra 1 here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

console.log(Promise.resolve('should not wait until resolved!'));
}),
// Wait for 5 events to hit - console.time is not reported
waitForEvents(page, 'console', 5)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This works because console logs wait for the ack in JavaScript?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's because protocol is serial: console.log's will be posted before the evaluation response.

@aslushnikov aslushnikov merged commit 06d6191 into puppeteer:master Apr 9, 2018
@aslushnikov aslushnikov deleted the test-waitevent branch May 9, 2018 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants