Closed Bug 1751492 Opened 3 years ago Closed 3 years ago

run xpcshell in the correct variant (no-fission || fission || 1proc)

Categories

(Testing :: XPCShell Harness, task)

Default
task

Tracking

(firefox101 fixed)

RESOLVED FIXED
101 Branch
Tracking Status
firefox101 --- fixed

People

(Reporter: jmaher, Assigned: jmaher, NeedInfo)

References

(Depends on 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

add a no-fission variant to firefox ci taskcluster configs:
https://searchfox.org/mozilla-central/source/taskcluster/ci/test/variants.yml

This bug will add this variant, and then set xpcshell to be xpcshell-no-fission by default.

When this bug is done further bugs can edit e10s specific tests as no-fission.

I am not sure what xpcshell should be run in. :nika, do you have any thoughts on this?

Flags: needinfo?(nika)
Summary: create a no-fission variant and run xpcshell by default in this variant → run xpcshell in the correct variant (no-fission || fission || 1proc)

I believe that we currently don't modify fission.autostart when running xpcshell, so the default configuration will technically run with fission enabled (though most xpcshell tests are not run against the browser prefs.js, so it has no impact as e10s is disabled). If this is the case, we should continue running xpcshell tests with fission enabled.

Flags: needinfo?(nika)

ok, for now I will move these to -fis so it is clear. for android I need to confirm, but probably -nofis.

I took a look at this and it was a mess on try. We run xpcshell and xpcshell_socketprocess_networking (spi-nw) just about everywhere. What I find is that running the tests as fission or no-fission, normal xpcshell had a few issues but spi-nw was a mess. After many try runs I have a relatively green run (some intermittents):
https://treeherder.mozilla.org/jobs?repo=try&revision=cfa00001b205301e474ff0ee587093ccfe12c2e3

I am concerned that explicitly enabling or disabling fission is causing so many failures- specifically in -spi-nw, but also in regular xpcshell.

I have added :kershaw (primary contact of spi-nw) and :mccr8 (helped look into fission specific failures). I have

there are 104 comments I have, and 90 of them are socketprocess_networking specific (14 would be needed without socketprocess). Of these 14, 1 skips a directory toolkit/mozapps/update/tests/unit_service_updater/xpcshell.ini for win/asan, which isn't scheduled normally, but is showing up on try.

Some of the socketprocess skip-ifs are for entire manifests, specifically in the extensions directory:
https://searchfox.org/mozilla-central/source/toolkit/components/extensions/test/xpcshell/xpcshell-remote.ini

:nika, do you have suggestions of how to proceed here?

Flags: needinfo?(nika)

I'm a bit curious about how you're force-enabling Fission for these tests, because in many of these cases I would expect Fission to not successfully enable, as IIRC by-default xpcshell is run with both e10s and fission disabled unless the firefox-appdir = browser annotation is added to the xpcshell.ini file (e.g. https://searchfox.org/mozilla-central/search?q=firefox-appdir&path=&case=true&regexp=false). If you're force-enabling e10s as well, I expect that some of these tests will be failing because they were previously run with e10s disabled.

I'm not super certain how the spi-nw enabling works as well, but there's also a chance for an interaction there.

Other than that, it seems there are some failures in toolkit/components/extensions. I know webextensions are one of the heaviest users of xpcshell tests for browser-like things, so I'll ni? :zombie on this bug to potentially look into what's happening there (although many of the lines you added in https://hg.mozilla.org/try/rev/81ecb8bc305ad472b971c22d362e2abb21840a7b appear to be commented out?)

Flags: needinfo?(tomica)
Flags: needinfo?(nika)
Flags: needinfo?(jmaher)

I was double checking many of my assertions and those commented out items were the remaining bits- my local patch has those cleaned up and that is where I harvested the numbers regarding 104 skip-if conditions added.

fission enabled and no-fission is just adding a variant which is done via a pref:

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

no-fission:
    description: "{description} without fission enabled"
    contact: jmaher
    suffix: "nofis"
    merge:
        mozharness:
            extra-options:
                - "--setpref=fission.autostart=false"

the spi-nw variant also sets prefs:

        mozharness:
            extra-options:
                - "--setpref=network.process.enabled=true"
                - "--setpref=network.http.network_access_on_socket_process.enabled=true"
                - "--setpref=network.ssl_tokens_cache_enabled=true"

Maybe :kershaw has more context on socketprocess_networking.

My ultimate goal is to figure out how to run xpcshell properly so we can reduce confusion in Treeherder about what we are running. I want to do this before removing -fis from treeherder as it is default, but xpcshell is sort of in limbo.

Flags: needinfo?(jmaher) → needinfo?(kershaw)

Maybe :kershaw has more context on socketprocess_networking.

My ultimate goal is to figure out how to run xpcshell properly so we can reduce confusion in Treeherder about what we are running. I want to do this before removing -fis from treeherder as it is default, but xpcshell is sort of in limbo.

Sorry that I currently have no idea why there so many test failures when fission and socket process are both enabled.
Do you have a list of failed tests? I could try to debug it.

Flags: needinfo?(kershaw)
Attached file WIP: Bug 1751492 - xpcshell [no]fission (obsolete) (deleted) —

:kershaw, in the attached patch look for socketprocess_networking # TODO and you will see many instances. That patch can be applied and pushed to try.

heavily concentrated in these manifests:
toolkit/components/extensions/test/xpcshell/xpcshell-content.ini
toolkit/components/antitracking/test/xpcshell/xpcshell.ini
toolkit/components/extensions/test/xpcshell/xpcshell-e10s.ini
toolkit/components/extensions/test/xpcshell/xpcshell-remote.ini
toolkit/mozapps/update/tests/unit_aus_update/xpcshell.ini
toolkit/components/search/tests/xpcshell/xpcshell.ini

Flags: needinfo?(kershaw)

checking in here- any updates? need any more information?

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

checking in here- any updates? need any more information?

Sorry that I don't have time to work on this for now.
I think it's fine to disable the failed tests for socketprocess_networking with fission.

Depends on: 1759035
Assignee: nobody → jmaher
Status: NEW → ASSIGNED
Attachment #9265785 - Attachment is obsolete: true

For anybody investigating these failures locally, as Nika said, Fission is on by default when you run XPCShell via mach or whatever. If you want to run it without Fission for comparison purposes you can do something like this: mach xpcshell-test --setpref fission.autostart=false

Depends on: 1762638
Pushed by jmaher@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/25760f0584a1 run xpcshell tests in fission mode. r=mccr8,necko-reviewers,application-update-reviewers,bytesized,dragana,mixedpuppy
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 101 Branch
Component: General → XPCShell Harness
Regressions: 1763314
No longer regressions: 1763314
Regressions: 1763515

@Joel, I fixed a typo in xpcshell-content.ini, causing bug 1763198 and I saw that test [test_ext_contentscript_permissions_fetch.js] is passing and maybe it shouldn't be skipped.

Flags: needinfo?(jmaher)
Regressions: 1763842

thanks for fixing the typo, the test still fails on linux/debug-spi-nw-nofis:
https://treeherder.mozilla.org/jobs?repo=try&tier=1%2C2%2C3&revision=13b6d2c0aed01e914c43739ceb9c379fc12bf766

Flags: needinfo?(jmaher)

Since bug 1751492, xpcshell sets run-without-variant to false, so we
were filtering out all xpcshell tasks.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: