Closed Bug 1648204 Opened 5 years ago Closed 2 years ago

wptrunner should not immediately kill the WebDriverServer process

Categories

(Testing :: web-platform-tests, defect)

defect

Tracking

(Not tracked)

RESOLVED WORKSFORME

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.

Actually we want the current geckodriver process environment variables, and not the system ones.

Summary: System environment variables aren't pushed to the browser process → Process environment variables aren't pushed to the browser process

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

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:

https://searchfox.org/mozilla-central/rev/a87a1c3b543475276e6d57a7a80cb02f3e42b6ed/testing/web-platform/tests/tools/wptrunner/wptrunner/webdriver_server.py#85-92

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?

Assignee: hskupin → nobody
No longer blocks: 1584911
Status: ASSIGNED → NEW
Component: geckodriver → web-platform-tests
Flags: needinfo?(james)
Priority: P1 → --
Summary: Process environment variables aren't pushed to the browser process → wptrunner should not immediately kill the WebDriverServer process

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.

Flags: needinfo?(james)

I could have a look next week if I could get it into the pytest runner or fixtures.

The severity field is not set for this bug.
:jgraham, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(james)
Severity: -- → S3
Flags: needinfo?(james)

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:

https://searchfox.org/mozilla-central/rev/85ae3b911d5fcabd38ef315725df32e25edef83b/testing/web-platform/tests/webdriver/tests/support/fixtures.py#142-145

Hereby we use the webdriver client to end the session:

https://searchfox.org/mozilla-central/rev/85ae3b911d5fcabd38ef315725df32e25edef83b/testing/web-platform/tests/tools/webdriver/webdriver/client.py#441

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?

Flags: needinfo?(rpierzina)

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
Flags: needinfo?(rpierzina)

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.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.