[wdspec] Tests have to run with "MOZ_DISABLE_NONLOCAL_CONNECTIONS" set
Categories
(Testing :: web-platform-tests, defect, P1)
Tracking
(firefox-esr102 wontfix, firefox-esr115 fixed, firefox116 wontfix, firefox117 wontfix, firefox118 wontfix, firefox119 fixed)
People
(Reporter: whimboo, Assigned: whimboo)
References
(Blocks 1 open bug)
Details
(Keywords: perf-alert, regression, Whiteboard: [webdriver:m8], [wptsync upstream])
Attachments
(5 files, 1 obsolete file)
By surprise I noticed today that for WebDriver tests we currently remove the already set MOZ_DISABLE_NONLOCAL_CONNECTIONS
environment variable:
The reason is: in those cases we get the default geckodriver profile which doesn't guarantee zero network access
.
That is actually not true. We worked hard on getting Marionette tests running with this environment variable set and this also applies to geckodriver which shares the same preferences to disable certain features.
If there is a crash happening with geckodriver in WebDriver tests we should certainly figure out why that is happening and disabling the feature.
Also removing this environment variable actually makes the Wdspec tests not a Tier-1 test suite, but we are running as Tier-1 for quite some time.
Note that removing this environment variable actually has an influence to the Cu.isInAutomation
check as well and triggers crashes when we are trying to send widget level events (see bug 1848957)
As such lets make sure that we can keep the environment variable set for wdspec tests. Checking the history the lines got added on bug 1678044 for Firefox 90.
Comment 1•9 months ago
|
||
Set release status flags based on info from the regressing bug 1678044
:jgraham, since you are the author of the regressor, bug 1678044, could you take a look? Also, could you set the severity field?
For more information, please visit BugBot documentation.
Comment 2•9 months ago
|
||
I don't think this is a regression. Before that change we didn't set any custom environment variables at all for wdspec tests i.e. we've never set this variable.
Assuming you're correct and with a default profile geckodriver no longer does any network access it should simply be safe to remove that block of code.
Assignee | ||
Comment 3•9 months ago
|
||
As it looks like there is only a single pref that is not yet set via recommended preferences in Remote Agent. Also some tests use example.com
which is not a valid wpt host. Changing the latter to use example.org
should probably do it.
Assignee | ||
Updated•9 months ago
|
Assignee | ||
Updated•9 months ago
|
Assignee | ||
Comment 4•9 months ago
|
||
Assignee | ||
Comment 5•9 months ago
|
||
Depends on D186823
Assignee | ||
Comment 6•9 months ago
|
||
Depends on D186824
Assignee | ||
Comment 7•9 months ago
|
||
Depends on D186825
Assignee | ||
Comment 8•9 months ago
|
||
We did the same for Marionette over on bug 1371576. As such setting the same prefs so that we can share that between geckodriver and WebDriver BiDi would be a good idea.
Updated•9 months ago
|
Updated•9 months ago
|
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/24b824e226c5 [wdspec] Only use local network in WebDriver BiDi's reload.py. r=webdriver-reviewers,jdescottes https://hg.mozilla.org/integration/autoland/rev/57929c2e184a [remote] Sync preferences from Marionette to disable services trying to connect to remote addresses. r=webdriver-reviewers,jdescottes https://hg.mozilla.org/integration/autoland/rev/d8ac28803dd6 [wptrunner] Do not remove "MOZ_DISABLE_NONLOCAL_CONNECTIONS" for wdspec tests. r=jdescottes
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/41661 for changes under testing/web-platform/tests
Comment 11•9 months ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/24b824e226c5
https://hg.mozilla.org/mozilla-central/rev/57929c2e184a
https://hg.mozilla.org/mozilla-central/rev/d8ac28803dd6
Updated•9 months ago
|
Upstream PR was closed without merging
Comment 13•9 months ago
|
||
The patch landed in nightly and beta is affected.
:whimboo, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval.
- If no, please set
status-firefox118
towontfix
.
For more information, please visit BugBot documentation.
Assignee | ||
Comment 14•9 months ago
|
||
The upstream PR has finally been merged!
Lets wait a couple more days for possible regressions of this change. If it's all still fine by next week I'll request an uplift so that wdspec is a Tier-1 for sure.
Upstream PR merged by sideshowbarker
Assignee | ||
Comment 16•8 months ago
|
||
Given the performance regressions this landing has been caused and the findings (bug 1850950 comment 16) what's still missing I'm asking for a backout of this patch. For a next landing we have to get two things fixed and make sure to not cause perf regressions again.
Sheriffs can you please backout all the changesets? Thanks!
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/41862 for changes under testing/web-platform/tests
Comment 18•8 months ago
|
||
Backed out 57929c2e184a & d8ac28803dd6 as requested by whimboo
Backout link: https://hg.mozilla.org/integration/autoland/rev/24c5f0ac0033f113e83081775c276aa128156462
Upstream PR merged by moz-wptsync-bot
Assignee | ||
Comment 20•8 months ago
|
||
I've pushed a try build with the preference change for safebrowsing in the default profile for Firefox (see bug 1850950) included:
https://treeherder.mozilla.org/jobs?repo=try&revision=45010ad3bf58e5c9575f59b66d0fb4c438fd95e9
The wdspec test webdriver/tests/classic/new_session/default_values.py | test_no_capabilities
should still fail in that try build due to an expected crash. As such I'll disable the test for now and we can try to get it running again via bug 1852252.
Assignee | ||
Comment 21•8 months ago
|
||
Depends on D186825
Assignee | ||
Comment 22•8 months ago
|
||
Depends on D187780
Assignee | ||
Updated•8 months ago
|
Assignee | ||
Comment 23•8 months ago
|
||
With the recent patches there is no performance regression visible. So nothing should block us from landing those patches now!
Updated•8 months ago
|
Comment 24•8 months ago
|
||
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d418c3caa7de [remote] Sync preferences from Marionette to disable services trying to connect to remote addresses. r=webdriver-reviewers,jdescottes https://hg.mozilla.org/integration/autoland/rev/43dd7fd52252 [wdspec] Improve stability for desired and no capabilities new session tests. r=webdriver-reviewers,jgraham https://hg.mozilla.org/integration/autoland/rev/7949e9eb4564 [wdspec] mozilla/tests/webdriver/classic/new_session/binary.py has to correctly pass capabilities. r=webdriver-reviewers,jdescottes https://hg.mozilla.org/integration/autoland/rev/6b6020b8d61f [wptrunner] Do not remove "MOZ_DISABLE_NONLOCAL_CONNECTIONS" for wdspec tests. r=jdescottes
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/41919 for changes under testing/web-platform/tests
Comment 26•8 months ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d418c3caa7de
https://hg.mozilla.org/mozilla-central/rev/43dd7fd52252
https://hg.mozilla.org/mozilla-central/rev/7949e9eb4564
https://hg.mozilla.org/mozilla-central/rev/6b6020b8d61f
Upstream PR merged by moz-wptsync-bot
Comment 28•8 months ago
|
||
(In reply to Norisz Fay [:noriszfay] from comment #18)
Backed out 57929c2e184a & d8ac28803dd6 as requested by whimboo
Backout link: https://hg.mozilla.org/integration/autoland/rev/24c5f0ac0033f113e83081775c276aa128156462
== Change summary for alert #39482 (as of Tue, 12 Sep 2023 07:07:38 GMT) ==
Regressions:
Ratio | Test | Platform | Options | Absolute values (old vs new) | Performance Profiles |
---|---|---|---|---|---|
10% | fandom loadtime | macosx1015-64-shippable-qr | cold fission webrender | 886.09 -> 977.43 | Before/After |
7% | fandom loadtime | linux1804-64-shippable-qr | bytecode-cached fission warm webrender | 201.70 -> 215.23 | Before/After |
6% | fandom SpeedIndex | linux1804-64-shippable-qr | bytecode-cached fission warm webrender | 204.22 -> 215.48 | Before/After |
5% | fandom PerceptualSpeedIndex | linux1804-64-shippable-qr | fission warm webrender | 232.22 -> 244.89 | Before/After |
Improvements:
Ratio | Test | Platform | Options | Absolute values (old vs new) | Performance Profiles |
---|---|---|---|---|---|
5% | fandom ContentfulSpeedIndex | linux1804-64-shippable-qr | bytecode-cached cold fission webrender | 711.90 -> 676.97 | Before/After |
4% | fandom LastVisualChange | linux1804-64-shippable-qr | cold fission webrender | 928.75 -> 892.22 | Before/After |
4% | fandom ContentfulSpeedIndex | linux1804-64-shippable-qr | cold fission webrender | 708.12 -> 681.49 | Before/After |
4% | fandom LastVisualChange | linux1804-64-shippable-qr | cold fission webrender | 928.67 -> 894.38 | Before/After |
2% | wikia ContentfulSpeedIndex | windows10-64-shippable-qr | fission warm webrender | 796.85 -> 780.28 | |
2% | imgur fcp | windows10-64-shippable-qr | fission warm webrender | 424.37 -> 415.83 | Before/After |
For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=39482
Updated•8 months ago
|
Comment 29•8 months ago
|
||
Pushed by wptsync@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/be02ed103cf0 [wpt PR 41661] - [Gecko Bug 1849972] [wdspec] Only use local network in WebDriver BiDi's reload.py., a=testonly
Assignee | ||
Comment 30•8 months ago
|
||
As discussed with perf sheriffs there is no action to take.
Comment 31•8 months ago
|
||
bugherder |
Updated•8 months ago
|
Assignee | ||
Comment 33•7 months ago
|
||
Comment on attachment 9350185 [details]
Bug 1849972 - [wptrunner] Do not remove "MOZ_DISABLE_NONLOCAL_CONNECTIONS" for wdspec tests.
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration: Affects tests and makes sure that we fulfill the Tier1 requirements for wdspec tests by causing crashes of Firefox when trying to access a remote address.
Beside the patches on this bug we also need bug 1821981 and bug 1853147.
- User impact if declined: No user impact.
- Fix Landed on Version: 119
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Only changes some default values of preferences to not cause Firefox to reach external network addresses.
Assignee | ||
Updated•7 months ago
|
Comment 34•7 months ago
|
||
Comment on attachment 9350185 [details]
Bug 1849972 - [wptrunner] Do not remove "MOZ_DISABLE_NONLOCAL_CONNECTIONS" for wdspec tests.
test-only change, doesn't need approval
Updated•7 months ago
|
Updated•7 months ago
|
Updated•7 months ago
|
Updated•7 months ago
|
Comment 35•7 months ago
|
||
uplift |
https://hg.mozilla.org/releases/mozilla-esr115/rev/fd600df7f05f https://hg.mozilla.org/releases/mozilla-esr115/rev/4a3e1107ae2d https://hg.mozilla.org/releases/mozilla-esr115/rev/1fd932656221 https://hg.mozilla.org/releases/mozilla-esr115/rev/e8660da6a427
Updated•7 months ago
|
Description
•