Add explicit test coverage for BuiltInThemes calls to AddonManager.maybeInstallBuiltinAddon
Categories
(Firefox :: Theme, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox133 | --- | fixed |
People
(Reporter: rpl, Assigned: rpl)
References
Details
(Whiteboard: [addons-jira])
Attachments
(1 file)
As previously agreed with Dao, as a followup to Bug 1921199 we should be also make sure to cover explicitly in automated tests the calls to AddonManager.maybeInstallBuiltinAddon originated from BuiltInThemes, to increase the chances that a regression like the one fixed by Bug 1921199 would be hitting an explicit test failure and we would be catching it earlier.
|
||
Updated•5 months ago
|
|
Assignee | |
Comment 1•5 months ago
|
||
|
||
Comment 3•5 months ago
|
||
Backed out for causing failures at browser_BuiltInThemes_installs.js.
Backout link: https://hg.mozilla.org/integration/autoland/rev/ca5513d0544ce26a5dc1eedb9317b7222e0f632a
Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=9759bc2aa579879ae74ffe6126a7d190f09f9295
Failure log: https://treeherder.mozilla.org/logviewer?job_id=477037450&repo=autoland&lineNumber=14
|
|
Assignee | |
Comment 4•5 months ago
|
||
Unfortunately I couldn't get back to look into this sooner, I tried to reproduce it today but it seems I can't hit the same failure when running the test locally (on both linux opt and debug artifacts build, with or without socketprocess enabled thorugh prefs), locally it does also seem to pass --verify just fine.
I'm going to try to gather some more details from hitting the failure in a try push.
I'll keep the needinfo assigned to me, as a reminder to come back to this as soon as I can.
|
|
Assignee | |
Comment 5•5 months ago
|
||
I managed to gather enough from a try push failure (with an additional info log added to the test) to determine what was going on:
- the url from with the test was fetching the builtin theme manifest.json resource in the version of the patch hitting the failure looked like: resource://builtin-themes/light//manifest.json
- the double
/character didn't trigger a fetch error when the test is executed locally on artifacts build, but it does when running on the build infrastructure (I'm guessing because when the test runs on the build infrastructure the resource URI points to an assets inside the omni jar file, while in artifacts dir it may instead point to a file on the filesystem, I haven't confirmed that guess but it doesn't feel unlikely and would explain the different behavior I was experiencing while executing the test locally)
I just updated the patch with a small tweak to the test case to fix that (making sure the resource URI looks like resource://builtin-themes/light/manifest.json instead of resource://builtin-themes/light//manifest.json), and will push the patch to autoland again soon.
|
||
Comment 7•5 months ago
|
||
| bugherder | ||
Description
•