Allow Timeouts of 0 #1964
Allow Timeouts of 0 #1964
Conversation
|
Seems like AppVeyor is not playing nice with the I've increased the test limit, but the problem remains, and it's not related to this PR: #1954 also fails. |
lib/FrameManager.js
Outdated
| */ | ||
| waitForFunction(pageFunction, options = {}, ...args) { | ||
| const timeout = options.timeout || 30000; | ||
| const timeout = (options.timeout != null) ? options.timeout : 30000; |
There was a problem hiding this comment.
nit: Let's use helper.isNumber instead.
const timeout = helper.isNumber(options.timeout) ? options.timeout : 30000;There was a problem hiding this comment.
@aslushnikov Great, do you want me to rollback the eslint config to == null too?
There was a problem hiding this comment.
Great, do you want me to rollback the eslint config to == null too?
@alixaxel yes, I like keeping it as strict as possible. Thanks!
test/test.js
Outdated
| }; | ||
|
|
||
| const timeout = slowMo ? 0 : 10 * 1000; | ||
| const timeout = slowMo ? 0 : 15 * 1000; |
There was a problem hiding this comment.
unfortunately this doesn't help; for some reason appveyour/travis sometimes are very short on resources and take forever to run tests.
Let's split timeout changse out of the PR and discuss separately.
There was a problem hiding this comment.
Yeah, on my local it was also timing out, after increasing it to 15secs it started working.
Must be the Chromium revision.
Closes #1960 .
It also addresses the timeout in
Puppeteer.launch(), as per the docs: