1751259 - migrate marionette jobs to be running on fission everywhere.

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, task)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

now that fission is default everywhere, we need to run tests in fission mode- ideally only in fission mode.

This bug is to run marionette in both e10s (i.e. !fission) and fission everywhere.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: Bug 1751259 - Run Marionette on fission everywhere. r=#releng-reviewers!

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45a5c95d85db
Run Marionette on fission everywhere. r=releng-reviewers,gbrown

Comment 3

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/45a5c95d85db

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

it appears I didn't do this properly, I did taskcluster configs, but the harness is not setting the pref.
here is a try push with some failures on windows debug (test only run on windows debug):
https://treeherder.mozilla.org/jobs?repo=try&revision=339df08d8dadaddbd951a6c55771a6840473263e

I would like to get this fixed this week if possible, I am not sure how to fix that test though.

:whimboo, do you have ideas on how to fix the test, I see you are the only person to touch the file:
https://hg.mozilla.org/mozilla-central/log/tip/browser/components/sessionstore/test/marionette/test_restore_windows_after_windows_shutdown.py

it was a "move" test files, and finding the old files in tree is not trivial.

Comment 5

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

Oh, that's not good. So why is the harness not setting the pref? Do you have more details for me?

Regarding the test, as you can see by the error message none of the formerly open windows are getting restored after the restart of Firefox. It's just about:blank which stays open after constant checks with a timeout of 5s:

[task 2022-04-19T22:44:12.771Z] 22:44:12 ERROR - TEST-UNEXPECTED-ERROR | browser/components/sessionstore/test/marionette/test_restore_windows_after_windows_shutdown.py TestWindowsShutdown.test_with_variety | marionette_driver.errors.TimeoutException: Timed out after 5.1 seconds with message: Non private browsing windows should have been restored. Expected {('data:text/html;charset=utf-8,%3Cdiv%22%3Eipsum%3C/div%3E', 'data:text/html;charset=utf-8,%3Cdiv%22%3Edolor%3C/div%3E'), ('data:text/html;charset=utf-8,%3Cdiv%22%3Esit%3C/div%3E', 'data:text/html;charset=utf-8,%3Cdiv%22%3Eamet%3C/div%3E'), ('data:text/html;charset=utf-8,%3Cdiv%22%3ELorem%3C/div%3E',)}, got {('about:blank',)}.

So you might want to try it locally. But note that we had a similar issue with failing tests recently when we noticed that the Mn jobs were running on hardware but not virtualized machines (bug 1759705). Especially these failing tests were affected by that. Basically they are disabled (see bug 1727691). Why are those still running?

Regarding the former location of tests it's visible by checking the diff of my patch:
https://hg.mozilla.org/mozilla-central/rev/b7efd81174a5e11b13bf6dd84627f01ae102757e

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

if you look at my patch (https://hg.mozilla.org/try/rev/30c9f129342b75c7772abe70ba86b6f220cef2d4 ) you can see why it isn't working- basically it is looking for --enable-fission and not looking at the pref fission.autostart=(true|false)

this was disabled but not on win/debug. We are running on the t-win-10-2004 platform, these are virtualized machines located at AWS, not physical machines in our datacenter.

I don't know where to find file author history of the previous filename testing/firefox-ui/tests/functional/sessionstore/test_restore_windows_after_windows_shutdown.py, that is the problem I wanted to solve to ask the right person a question.

I ran this locally (it would be on hardware) and the test passed in both opt and debug. I do run windows 11, so my configuration is different than automation.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

I guess my thoughts are that we skip this test, enable on fission and keep a bug open to fix the test. There is also a high frequency intermittent in the linux asan tests, looks similar to bug 1727207.

Comment 8

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

(In reply to Joel Maher ( :jmaher ) (UTC -0800) from comment #6)

if you look at my patch (https://hg.mozilla.org/try/rev/30c9f129342b75c7772abe70ba86b6f220cef2d4 ) you can see why it isn't working- basically it is looking for --enable-fission and not looking at the pref fission.autostart=(true|false)

Can you please explain why the Marionette harness has to look for a fission.autostart value? We offer the --enable-fission option to enable Fission, but none of the Marionette unit tests get a pre-configured profile. So it's not clear to me in which constellation you are seeing this particular issue.

I don't know where to find file author history of the previous filename testing/firefox-ui/tests/functional/sessionstore/test_restore_windows_after_windows_shutdown.py, that is the problem I wanted to solve to ask the right person a question.

I assume that you are looking for the following information?

https://hg.mozilla.org/mozilla-central/log/b7efd81174a5e11b13bf6dd84627f01ae102757e/testing/firefox-ui/tests/functional/sessionstore/test_restore_windows_after_windows_shutdown.py

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

we were never sending --enable-fission to the harness, instead we are sending a pref as seen by the variants.yml file:
https://searchfox.org/mozilla-central/source/taskcluster/ci/test/variants.yml#77

fission:
    description: "{description} with fission enabled"
    component: "Testing::General"
    suffix: "fis"
    merge:
        mozharness:
            extra-options:
                - "--setpref=fission.autostart=true"

we can plumb this down from mozharness, or we can just check for the pref. I am adjusting things to be default fission.autostart=true and use --disable-fission as a cli option; in testing this out I found that marionette was not running properly.

Thanks for the history of the file- I can look through the bugs, but the original author is no longer part of the project.

Comment 10

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

(In reply to Joel Maher ( :jmaher ) (UTC -0800) from comment #9)

we were never sending --enable-fission to the harness, instead we are sending a pref as seen by the variants.yml file:
https://searchfox.org/mozilla-central/source/taskcluster/ci/test/variants.yml#77

we can plumb this down from mozharness, or we can just check for the pref. I am adjusting things to be default fission.autostart=true and use --disable-fission as a cli option; in testing this out I found that marionette was not running properly.

Ah I see. I totally missed that we do not use the argument. Question aside and probably better for the other bug but why are we going forward with --disable-fission and not using --setpref="fission.autostart=false" instead?

Thanks for the history of the file- I can look through the bugs, but the original author is no longer part of the project.

What exactly did you wanted to ask? Maybe I can help or forward the question.

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

I don't mind whatever path we take here. I thought we should sort this out (fission working) before making all the other changes.

I mostly wanted to ask the test case owner or inherited owner to take a look at this so they are aware- maybe there is a quick fix, or maybe we just disable and fix later.

:whimboo- do you prefer I make a patch to disable failing tests and use cli arguments for --disable-fission while setting fission.autostart=true by default. We will only use --disable-fission on linux/android where we run no-fission tests.

Comment 12

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

(In reply to Joel Maher ( :jmaher ) (UTC -0800) from comment #11)

I don't mind whatever path we take here. I thought we should sort this out (fission working) before making all the other changes.

That sounds fine. So here some questions / comments regarding your patch:

1. Do we need --enable-fission at all for the Marionette harness? Given that we seem to use the preference everywhere else and basically no other important harness makes use of it anymore, why not removing it?

2. In case we have to keep it... Should we give a higher priority for --enable-fission or the preference? Both are arguments for the Marionette harness. Whereby --enable-fission is way more specific here and as such IMHO should override any value as given by the preference. This should stay that way when the change for --disable-fission will happen.

3. prefs is always a dict so that we do not need the extra check.

I mostly wanted to ask the test case owner or inherited owner to take a look at this so they are aware- maybe there is a quick fix, or maybe we just disable and fix later.

See the bug that I have referenced before. There is a comment from Dao:
https://bugzilla.mozilla.org/show_bug.cgi?id=1727691#c39

:whimboo- do you prefer I make a patch to disable failing tests and use cli arguments for --disable-fission while setting fission.autostart=true by default. We will only use --disable-fission on linux/android where we run no-fission tests.

Disabling the failing session store test sounds fine. Regarding test_reftest.py I feel it would be fine to disable on ASAN as well. Otherwise lets check the above comments first.

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

1. I will remove --enable-fission; thanks!
2. switching harness to use the pref, otherwise set to fission
3. for the unittests I need to check, it is a mock.sentinel when running the unittests, so this was my workaround- open to other ideas.

testing a patch, once it looks good I will upload for review.

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: Bug 1751259 - make marionette really use fission. r=whimboo!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4de64fdf7064
make marionette really use fission. r=whimboo,webdriver-reviewers

Comment 16

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/4de64fdf7064