Increase jest timeout for PWDEBUG option #596
Conversation
Pull Request Test Coverage Report for Build 605362498
💛 - Coveralls |
Co-authored-by: Max Schmitt <[email protected]>
yes) |
| if (configTimeout) { | ||
| return configTimeout | ||
| } | ||
| return process.env.PWDEBUG ? DEBUG_TIMEOUT : DEFAULT_TEST_PLAYWRIGHT_TIMEOUT |
There was a problem hiding this comment.
@mmarkelov I think you should document that you set the default timeout to 15s. I was surprised when running tests after a recent upgrade that it increased from 5 -> 15 s for each jest test. It is worth mentioning in your docs that this is the case.
Also, https://playwright.dev/docs/next/debug/#run-in-debug-mode mentions that it both runs in headful, as well as sets the playwright timeout to 0 (= no timeout). Is this the same you do? I.e. set both Jest's and Playwrights timeout to no timeout? I think it is worth mentioning in the docs what you do when a user sets PWDEBUG=1. I.e. do you, in addition to what Playwright does, also change Jest's default?
There was a problem hiding this comment.
@jhildenbiddle i believe that changes from 5 -> 15 s was made in the first versions of jest-playwright, and it was made because some interactions with page can be slow and 5 second was not enough, lots of tests are failing because of it.
I suppose no timeout for PWDEBUG mode is about playwrights timers. And we set timeout to big value, because otherwise the usage of debugging features, like page.pause() will relate to jest failure
There was a problem hiding this comment.
@mmarkelov Did you mean to tag me really? :)
Could you document this? Two things I think of.
- Document somewhere that you set the default timeout for all tests to 15 sec.
- Document in the Debug section that when
PWDEBUGis set, that you following defaults are set: https://playwright.dev/docs/next/debug/#defaults, plus that you set Jest's own timeout to a huge value. Maybe be explicit
I think that would be great :)
There was a problem hiding this comment.
@thernstig create new issue about doc improvements #602
| PWDEBUG=1 jest | ||
| ``` | ||
|
|
||
| - Jest Playwright exposes a method `jestPlaywright.debug()` that suspends test execution and gives you opportunity to see what's going on in the browser. |
There was a problem hiding this comment.
How does jestPlaywright.debug() interact with page.pause()? https://playwright.dev/docs/api/class-page?_highlight=paus#pagepause
It would be great if jestPlaywright.debug() also pauses (and allows the user to resume) but I think this should be explicitly explained here. It is good to say here what debugging does both for the Playwright settings, as well as the Jest settings.
There was a problem hiding this comment.
maybe we can rework that jestPlaywright will use playwright.pause under the hood?
There was a problem hiding this comment.
@jhildenbiddle page.pause() was introduced only in last version. And jestPlaywright.debug() does not related to this function. Personally, I don't like jestPlaywright.debug() because it's weird and hacky solution to simplify debugging, that came from jest-puppeteer
There was a problem hiding this comment.
@mxschmitt yes we can. But it will be breaking change. I think usage of this feature for debugging is much better that jestPlaywright.debug()
There was a problem hiding this comment.
That would be sweet, do you want to create an issue? I can do it if you want but you can probably better explain it. Release it under a 2.0 release then, I think this awesome project has graduated enough to warrant a new major version 👍
There was a problem hiding this comment.
@thernstig yes. It will be nice if you create an issue)
4 participants
close #594