Closed Bug 936185 Opened 9 years ago Closed 9 years ago

[Settings] Move shims into desktop helper

Categories

(Firefox OS Graveyard :: Gaia::Settings, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kgrandon, Assigned: kgrandon)

Details

(Keywords: perf, Whiteboard: [c= p=3 s=2013.11.22 u=])

Attachments

(2 files, 1 obsolete file)

Currently in the settings app we load a lot of shims in the runtime. We would see some minor perf wins if we move all of these into the desktop helper.

In addition I'd like to see more panels work so I'm going to build out some of these shims. The goal is to have a mostly working wifi, call, carrier, and bluetooth panels available for desktop testing.
Attached file Github pull request (obsolete) —
Hi Evelyn,

Wondering if you could give this a quick review. This pull request moves some shim code into the desktop helper, and adds additional functionality to the shims. I want to be able to access and use almost all of the settings panels, so here's the start of that work.

I also want to add a few dropdowns in the future which will allow us to enable/disable certain features for ease of testing things like flatfish in the browser.
Attachment #828889 - Flags: review?(ehung)
Comment on attachment 828889 [details] [review]
Github pull request

This change totally makes sense to me, r+ if it passes travis tests.
Thanks for your work.
Attachment #828889 - Flags: review?(ehung) → review+
(In reply to Kevin Grandon :kgrandon from comment #1)
> Created attachment 828889 [details] [review]
> Github pull request
> 
> Hi Evelyn,
> 
> Wondering if you could give this a quick review. This pull request moves
> some shim code into the desktop helper, and adds additional functionality to
> the shims. I want to be able to access and use almost all of the settings
> panels, so here's the start of that work.
> 
> I also want to add a few dropdowns in the future which will allow us to
> enable/disable certain features for ease of testing things like flatfish in
> the browser.

Do you have a meta bug for tracking? I'd like to hear your plan. Thanks!
(In reply to Evelyn Hung [:evelyn] from comment #3)
> (In reply to Kevin Grandon :kgrandon from comment #1)
> Do you have a meta bug for tracking? I'd like to hear your plan. Thanks!

No bug filed yet, but I will be thinking about it and filing one soon. I guess I'm mainly thinking about hardware feature control such as SIM card state, and hopefully better resolution support. I will file soon and CC you and the settings people.
Comment on attachment 828889 [details] [review]
Github pull request

Hi Zac,

I've made a simple change to the travis gaia-ui-test script to include our desktop shims inside of the profile. You can find the change here: https://github.com/mozilla-b2g/gaia/pull/13486/files#diff-be3d635bffedf306b771f88f2b6c197bR14

I was wondering if you could take a quick look and review that part, or reassign the review. The shims are needed as we are moving mock wifi into extensions instead of being in the codebase. Thanks!
Attachment #828889 - Flags: review?(zcampbell)
Kgrandon, I can't really judge the code but if the tests are passing on Jenkins OK then that is as best as I can interpret. (I retriggered the run and it's green now)
Comment on attachment 828889 [details] [review]
Github pull request

r+ on tests/green travis.. did not review code.
Attachment #828889 - Flags: review?(zcampbell) → review+
Thanks for the reviews!

Landed in master: https://github.com/mozilla-b2g/gaia/commit/91ecaba0e827a3c42c2553b8f308b726707fbdf9
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [c= p=2 s= u=] → [c= p=2 s=2013.11.22 u=]
Attached file Gaia patch - rebased
Attachment #828889 - Attachment is obsolete: true
Attachment #831046 - Flags: review+
I've updated the http://hg.mozilla.org/build/mozharness/file/12895b172413/mozharness/mozilla/gaia.py#l58 file here with an ENV var to include desktop shims as well.

I'm also trying to use the moz-git-tools to run this on try, but not sure I did this correctly. Link to build: https://tbpl.mozilla.org/?tree=Try&rev=1d9524c50a5e
Comment on attachment 831087 [details] [diff] [review]
Bug-936185-Include-desktop-shims-for-gaia-profile.patch

Review of attachment 831087 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Aki - I think you may be the right person to review this patch.

The idea is that we want to build gaia with an ENV var to specify additional desktop shims. The code for that is in the gaia patch here: https://github.com/mozilla-b2g/gaia/pull/13636/files#diff-b67911656ef5d18c4ae36cb6741b7965R688

It would also be nice to be able to run this on try and do a full build - do you know if that's possible? Thanks!
Attachment #831087 - Flags: review?(aki)
Attachment #831087 - Flags: review?(aki) → review+
Aiui, this patch will affect both gaia-ui-tests and marionette tests, since both inherit GaiaMixin.

(In reply to Kevin Grandon :kgrandon from comment #14)
> It would also be nice to be able to run this on try and do a full build - do
> you know if that's possible? Thanks!

I'm not entirely sure; it may be possible to test on Cedar.
Cedar is based off mozilla-central, and uses the mozharness default branch (where this patch would land at first).
Every other mozilla-central-based repo uses the mozharness production branch; we merge changes from default->production to make them live.
So if we landed on default, and didn't merge to production yet, then this patch would only affect Cedar.

https://tbpl.mozilla.org/?tree=Cedar
I used ash for this.

https://tbpl.mozilla.org/php/getParsedLog.php?id=30506711&tree=Ash&full=1 looks good, and it didn't seem to negatively affect marionette.
Comment on attachment 831087 [details] [diff] [review]
Bug-936185-Include-desktop-shims-for-gaia-profile.patch

https://hg.mozilla.org/build/mozharness/rev/bc537b691617

This will be merged to production within the next day or so.
in production
You guys are awesome, thanks!
Ok - landing the gaia patch again and hoping that the new mozharness stuff works on try. Unfortunately there's no way to run gaia in try right now AFAIK - if this is possible, please let me know.

Gaia patch landed in master: https://github.com/mozilla-b2g/gaia/commit/de6f6d7ab771db12914f853fefc2c0363da19028
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Whiteboard: [c= p=2 s=2013.11.22 u=] → [c= p=3 s=2013.11.22 u=]
You need to log in before you can comment on or make changes to this bug.