wptrunner should not immediately kill the WebDriverServer process
Categories
(Testing :: web-platform-tests, defect)
Tracking
(Not tracked)
People
(Reporter: whimboo, Unassigned)
Details
I tried to run the Gecko profiler with the webdriver tests, but no profile gets created:
MOZ_PROFILER_STARTUP=1 MOZ_PROFILER_SHUTDOWN=/data/profile.json mach wpt --webdriver-arg=-vv --webdriver-binary=target/debug/geckodriver testing/web-platform/tests/webdriver/tests/new_window/user_prompts.py
After checking wptrunner, and then geckodriver it's clear now that geckodriver doesn't pass the system environment variables into the browser process. The only variables I can see are:
{"MOZ_NO_REMOTE": "1", "MOZ_CRASHREPORTER": "1", "MOZ_CRASHREPORTER_SHUTDOWN": "1", "MOZ_CRASHREPORTER_NO_REPORT": "1"}
We should get this into the 0.27.0 release. I will have a look.
| Reporter | ||
Comment 1•5 years ago
|
||
Actually we want the current geckodriver process environment variables, and not the system ones.
Comment 2•5 years ago
|
||
I'm surprised; per [1] we should inherit environment variables from the parent by default, and I think we just add environment variables, not remove.
[1] https://doc.rust-lang.org/std/process/struct.Command.html
| Reporter | ||
Comment 3•5 years ago
|
||
So this is not a geckodriver issue but a wptrunner one. As seen below we always hard-kill the WebDriverServer process but don't try to quit the session first:
Based on that the gecko profiler is not writing the profile to disk. Also other shutdown logic will not happen so that re-using the same profile for another test (in the future) might not be possible.
James, where could we actually call the DeleteSession command for the different drivers?
Comment 4•5 years ago
|
||
So the issue is that the WebDriver session is controlled by pytest, which can create many sessions per driver instance. Therefore this code doesn't know anything about sessions; it just knows that a WebDriver binary is running.
I think there are two options. Option 1 is to have the pytest fixtures clean up the session at the end of the tests. Option 2 is to have the fixtures communicate the session ID in some hacky way like setting an environment variable, and use that at the end to clean up. That option seems more problematic in the face of running multiple sessions in parallel etc. so maybe we should try the first one. In any case some fallback to killing the binary is going to be needed since it may not quit correctly.
| Reporter | ||
Comment 5•5 years ago
|
||
I could have a look next week if I could get it into the pytest runner or fixtures.
Comment 6•5 years ago
|
||
The severity field is not set for this bug.
:jgraham, could you have a look please?
For more information, please visit auto_nag documentation.
Updated•5 years ago
|
| Reporter | ||
Comment 7•5 years ago
|
||
I think the case where it is working is when we restart the browser in-between tests if the requested capabilities are different from the current ones:
Hereby we use the webdriver client to end the session:
Where it doesn't seem to work is after finishing the very last test. Not exactly sure where exactly the session should be ended, before the webdriver process gets killed.
Raphael, is there a hook or something else in pytest that allows you to run code after the very last test?
Comment 8•5 years ago
|
||
Hi Henrik, you could try the pytest_sessionfinish hook. If that doesn't work for you, you could try using a fixture:
@pytest.fixture(name="end_webdriver_session", scope="session", autouse=True)
def fixture_end_webdriver_session():
yield
# End webdriver session here
| Reporter | ||
Comment 9•2 years ago
|
||
Looks like I totally forgot about this bug. Thanks Raphael for your comment.
I had another check now and as it looks like the problem that I had is gone. I can now perfectly run Wdspec tests with the profiler creating a gecko profile. Here is an example: https://share.firefox.dev/3NZLVSp
That means we do no longer force kill the process but the session is ended normally.
Description
•