Do not automatically offer translations in automation
Categories
(Firefox :: Translations, task, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox-esr102 | --- | unaffected |
| firefox-esr115 | --- | unaffected |
| firefox116 | --- | unaffected |
| firefox117 | --- | fixed |
| firefox118 | --- | fixed |
People
(Reporter: gregtatum, Assigned: gregtatum)
References
(Blocks 1 open bug, Regressed 1 open bug)
Details
(Keywords: perf-alert)
Attachments
(1 file, 2 obsolete files)
|
48 bytes,
text/x-phabricator-request
|
diannaS
:
approval-mozilla-beta+
|
Details | Review |
In a previous bug we added a check for Cu.isInAutomation the automatic offering of translations so that they wouldn't run in automation. This doesn't seem to be respected in certain test suites, so we should flip the pref for these tests.
|
Assignee | |
Comment 1•1 year ago
|
||
|
|
Assignee | |
Comment 2•1 year ago
|
||
I am seeing if this clears up the intermittent failures here:
https://treeherder.mozilla.org/jobs?repo=try&revision=940f918fab0ce6e99ac49f7126f51969a7aea423
|
||
Comment 3•1 year ago
|
||
(In reply to Greg Tatum [:gregtatum] from comment #2)
I am seeing if this clears up the intermittent failures here:
https://treeherder.mozilla.org/jobs?repo=try&revision=940f918fab0ce6e99ac49f7126f51969a7aea423
There seem to be still quite a lot of hangs during shutdown so setting this pref doesn't seem to help.
|
|
Assignee | |
Comment 4•1 year ago
|
||
The issue is that Cu.isInAutomation is not being respected in our code for wdspec. I have a clean try run with adding a pref to disable the language identification code. I'm going to try one more run with still offering the popup, but not automatically identifying the language.
This disconnect is basically a user browsing the web would want to have pages identified, while in automation it's frequent that pages don't specify the language of the page.
|
|
||
Comment 5•1 year ago
|
||
Ah, I didn't check that you make use of Cu.isInAutomation. So there is bug 1834306 where we want to include Marionette and Remote Agent. Until this has been done you could do it like the following code:
|
|
Assignee | |
Comment 6•1 year ago
|
||
This is being triggered by a later patch that is adding new logging. I
believe this is all just because of initialization order.
|
||
Comment 7•1 year ago
|
||
Comment on attachment 9347027 [details]
Bug 1845518 - Drive by fix an issue with the console not being ready; r?nordzilla
Revision D185210 was moved to bug 1846828. Setting attachment 9347027 [details] to obsolete.
|
|
Assignee | |
Comment 8•1 year ago
|
||
This patch removes the automation check from the parent to the child,
since we don't want to load more scripts into the child process.
|
|
||
Updated•1 year ago
|
|
|
Assignee | |
Comment 9•1 year ago
|
||
The "drive by fix" patch was mistakenly attached here from another patch stack.
I got a clean run with the approach suggested by :whimboo.
|
||
Comment 10•1 year ago
|
||
|
|
||
Updated•1 year ago
|
|
||
Comment 11•1 year ago
|
||
| bugherder | ||
|
|
||
Comment 12•1 year ago
|
||
Greg, mozilla-beta is still affected by the shutdown hangs. Can you please request an uplift of the patch? Thanks.
|
|
||
Comment 13•1 year ago
|
||
Comment on attachment 9347027 [details]
Bug 1845518 - Drive by fix an issue with the console not being ready; r?nordzilla
Revision D185210 was moved to bug 1846828. Setting attachment 9347027 [details] to obsolete.
|
|
Assignee | |
Comment 14•1 year ago
|
||
Comment on attachment 9347165 [details]
Bug 1845518 - Make the automation check work for Marionette and Remote Agent; r?nordzilla!
Beta/Release Uplift Approval Request
- User impact if declined: Tests using Marionette and Remote Agent will flaky in automation. Users using these tools will be presented with a translations popup they do not expect, which can break their automations.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): This adds a few additional checks to some if checks, and is part of the decision process for offering translations. This should only affect Firefox under automation, which is a much smaller audience.
- String changes made/needed:
- Is Android affected?: No
|
|
||
Comment 15•1 year ago
|
||
Greg, to ask again because I haven't seen answer to my question on Matrix (maybe I missed)... While this solves the hang during shutdown for automation jobs the problem should still be there for real users. So I assume we need a follow-up bug to handle that scenario?
|
|
Assignee | |
Comment 16•1 year ago
|
||
I mentioned it in a thread on Matrix, but I believe it's caused by Bug 1826222, since our workers don't have shut down awareness.
|
||
Comment 17•1 year ago
|
||
Comment on attachment 9347165 [details]
Bug 1845518 - Make the automation check work for Marionette and Remote Agent; r?nordzilla!
Approved for 117.0b4
|
|
||
Comment 18•1 year ago
|
||
| uplift | ||
|
|
||
Updated•1 year ago
|
|
||
Comment 19•1 year ago
|
||
(In reply to Pulsebot from comment #10)
Pushed by gtatum@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/58d067f89f59
Make the automation check work for Marionette and Remote Agent; r=nordzilla
== Change summary for alert #39216 (as of Sat, 05 Aug 2023 10:29:14 GMT) ==
Improvements:
| Ratio | Test | Platform | Options | Absolute values (old vs new) |
|---|---|---|---|---|
| 5% | Images | linux1804-64-shippable-qr | fission tp6 | 5,168,113.83 -> 4,916,665.96 |
| 5% | Images | windows11-64-2009-shippable-qr | fission tp6 | 5,476,009.51 -> 5,215,644.35 |
| 3% | Images | macosx1015-64-shippable-qr | fission tp6 | 5,932,814.72 -> 5,726,110.45 |
For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=39216
Description
•