1824841 - Improve assertions for tests in /mozilla/tests/webdriver/new_session/profile_root.py

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, task, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Lets improve the assertions for the given Mozilla specific tests so that this might give us an idea what's going on with the failure on bug 1785893.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Bug 1824841 - [wdspec] Improve assertions for tests in /mozilla/tests/webdriver/new_session/profile_root.py

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok some updates here. I've improved some assertions in the tests but this caused Windows jobs to always fail due to a bug in mozprocess which I tried to fix 5 years ago on bug 1433905, but the important part got backed out via bug 1493796. As such the current behavior is that poll() continues to report None even with the geckodriver process gone:

https://treeherder.mozilla.org/logviewer?job_id=410631876&repo=try&lineNumber=109643-109648

[task 2023-03-29T09:20:31.185Z] 09:20:31     INFO - STDOUT: Exiting with code: 64
[task 2023-03-29T09:20:31.187Z] 09:20:31     INFO - STDOUT: DBG::MOZPROC PID:9140 (Thread-9) | process id 9140 exited normally
[task 2023-03-29T09:20:31.187Z] 09:20:31     INFO - STDOUT: DBG::MOZPROC PID:9140 (Thread-9) | job object msg active processes zero
[task 2023-03-29T09:20:31.188Z] 09:20:31     INFO - STDOUT: DBG::MOZPROC ProcessReader | _read loop exited
[task 2023-03-29T09:20:31.188Z] 09:20:31     INFO - STDOUT: DBG::MOZPROC ProcessReader | _read exited
[task 2023-03-29T09:20:33.201Z] 09:20:33     INFO - STDOUT: DBG::MOZPROC PID:9140 (Thread-7) | No process handle
[task 2023-03-29T09:20:33.203Z] 09:20:33     INFO - STDOUT: ** returncode: None

When I re-apply the patch (which got backed out) on bug 1493796 the tests on Windows are working fine:
https://treeherder.mozilla.org/jobs?repo=try&revision=8abc3858992898ae0221b0f836804c31540fbb59&selectedTaskRun=OQK5tAUfTBiuxpJsQDkYhw.0

Now I'm not sure if it makes sense to try to fix the bug in mozprocess given that it was kinda problematic in the past and I don't want to spend too much time on that, or if our code just uses subprocess directly.

Geoff, do you have a vision for mozprocess? As I remember at some point there was the discussion to get rid of it.

Comment 3

[Geoff Brown [:gbrown]]

I suspect that mozprocess is not needed today and can be replaced with modern subprocess. I think that would be a good thing because - as you well know! - mozprocess is buggy and development of mozprocess is long dormant.

But converting clients from mozprocess to subprocess is not always easy and I have neither a plan nor the time to pursue that migration.

Your patch and try run in comment 2 looks good; if it stands up to more testing (wider range of tests and maybe a few reruns), maybe that's the way forward for now?

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Geoff Brown [:gbrown] from comment #3)

Your patch and try run in comment 2 looks good; if it stands up to more testing (wider range of tests and maybe a few reruns), maybe that's the way forward for now?

That's just for a simple case but I'm fairly sure it will cause quite a bit of trouble with process tracking when Marionette restarts Firefox. I've used the same try build to trigger some more tests for Marionette and wdspec specific. In the case it's passing I can do a full try build to see which other jobs are affected. Overall this change will be risky and due to the investigation on bug 1493796 it might still require other changes. But lets see.

If it fails I'll go ahead and just use subprocess for the Mozilla specific wdspec tests to handle the geckodriver process.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: WIP: Bug 1824841 - extra debug logging

Depends on D173767

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: WIP: Bug 1824841 - Fix mozprocess handler for poll

Depends on D175152

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #4)

If it fails I'll go ahead and just use subprocess for the Mozilla specific wdspec tests to handle the geckodriver process.

Turns out that using mozprocess directly for geckodriver is problematic and not easy to solve. While my upcoming patch for bug 1493796 will fix the restart issues with Marionette I cannot get this misbehavior fixed at the same time without regressing the Marionette (or other) parts again. As such I will use subprocess directly for geckodriver - also because mozprocess would be overhead and not really needed.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/109f85897efe
[wdspec] Improve assertions for tests in /mozilla/tests/webdriver/new_session/profile_root.py r=webdriver-reviewers,jgraham

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/109f85897efe