936185 - [Settings] Move shims into desktop helper

Status: RESOLVED FIXED

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

Description

[Kevin Grandon :kgrandon]

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.

Comment 1

[Kevin Grandon :kgrandon]

Attachment: 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.

Comment 2

[Evelyn Hung]

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.

Comment 3

[Evelyn Hung]

(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!

Comment 4

[Kevin Grandon :kgrandon]

(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 5

[Kevin Grandon :kgrandon]

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!

Comment 6

[Zac C (:zac)]

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 7

[Zac C (:zac)]

Comment on attachment 828889 [details] [review]
Github pull request

r+ on tests/green travis.. did not review code.

Comment 8

[Kevin Grandon :kgrandon]

Thanks for the reviews!

Landed in master: https://github.com/mozilla-b2g/gaia/commit/91ecaba0e827a3c42c2553b8f308b726707fbdf9

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Backed out for Gaia UI test failures.
https://github.com/mozilla-b2g/gaia/commit/a013dd9a0bd69d7b8516ac55c893d3c01b8cedcf

https://tbpl.mozilla.org/php/getParsedLog.php?id=30483416&tree=B2g-Inbound

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 828889 [details] [review]
Github pull request

https://github.com/mozilla-b2g/gaia/pull/13636

Comment 11

[Kevin Grandon :kgrandon]

Attachment: Gaia patch - rebased

Comment 12

[Kevin Grandon :kgrandon]

Attachment: Bug-936185-Include-desktop-shims-for-gaia-profile.patch

Comment 13

[Kevin Grandon :kgrandon]

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 14

[Kevin Grandon :kgrandon]

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!

Comment 15

[Aki Sasaki (not active)]

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

Comment 16

[Aki Sasaki (not active)]

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 17

[Aki Sasaki (not active)]

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.

Comment 18

[John Hopkins (:jhopkins)]

in production

Comment 19

[Kevin Grandon :kgrandon]

You guys are awesome, thanks!

Comment 20

[Kevin Grandon :kgrandon]

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