[testing] Add option to keep browser open after running a test. #25374
Conversation
Handy option to inspect the browser after the test has run.
| def browser_terminate(cls): | ||
| if EMTEST_KEEP_OPEN: | ||
| assert len(cls.browser_procs) == 1, '--keep-open only supports one browser process' | ||
| cls.browser_procs[0].wait() |
There was a problem hiding this comment.
But will this not also keep the test running open ? Why not just exit?
There was a problem hiding this comment.
At least for chrome the browser will be killed when python exits.
There was a problem hiding this comment.
Oh I see, maybe add a comment there, something like "# Keep the python process alive until the browser is closed. This also means that http server will keep runnning"
Alternatively we could exit the python process after orphaning the browser, but then I guess that server would also go down.
There was a problem hiding this comment.
At least for chrome the browser will be killed when python exits.
Firefox browser windows stay open after python exits, both on Windows and Linux.
| @classmethod | ||
| def browser_terminate(cls): | ||
| if EMTEST_KEEP_OPEN: | ||
| assert len(cls.browser_procs) == 1, '--keep-open only supports one browser process' |
There was a problem hiding this comment.
In the Windows on Firefox case, len(cls.browser_procs) > 1 even if there is only one test running.
Also, this failure seems like a configuration/setup failure, rather than an after-the fact failure?
I.e. if I run EMTEST_CORES=128 test/runner browser --keep-open, then this will still launch all the 128 browsers, and only when they are all being shut down at the very end of the test (or if one test fails), then that Worker will assert fail about this --keep-open not being compatible with multithreaded runs?
So maybe a better check might be to move this assert earlier when the test suite is starting, e.g. after the parallel_testsuite.NUM_CORES variable has been set, and verify that NUM_CORES==1?
There was a problem hiding this comment.
This is kind of a half baked feature. I really just use it for running one test and debugging. I can move the asserts around to be earlier and only support single tester mode.
| def browser_terminate(cls): | ||
| if EMTEST_KEEP_OPEN: | ||
| assert len(cls.browser_procs) == 1, '--keep-open only supports one browser process' | ||
| cls.browser_procs[0].wait() |
There was a problem hiding this comment.
how about
for p in cls.browser_procs:
p.wait()?
| parser.add_argument('--browser-auto-config', type=bool, default=True, | ||
| help='Use the default CI browser configuration.') | ||
| parser.add_argument('--keep-open', action='store_true', | ||
| help='Keep the browser open after tests have run.', default=None) |
There was a problem hiding this comment.
In the current form, this might not be accurate?
Is the idea to be able to run only a single fixed test with this?
I.e. test/runner browser.test_my_failing_test --keep-open ?
Or to be able to run multiple tests, and to be able to fail on first failing one?
Currently browser.terminate() will be called if a test times out in the suite? Is that intended to interact with this --keep-open?
Handy option to inspect the browser after the test has run.