Create base unit test for Gaia System Customization work

RESOLVED FIXED

Status

Firefox OS
Gaia::System
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: qdot, Assigned: aus)

Tracking

unspecified
x86_64
Linux
Bug Flags:
in-moztrap -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Most System Customization work hinges on the sim card being switched out, which changes the MNC/MCC setting. It'd be nice to have a way to simulate this change in a marionette case, both for the emulator and for desktop B2G.

There's already some work on this done in the mobileconnection API, see

http://dxr.mozilla.org/mozilla-central/source/dom/network/tests/marionette/test_mobile_iccinfo.js

However most of this relies on the emulator's RIL mocking capabilities, which we don't have available in desktop.
(Assignee)

Comment 1

5 years ago
It looks like we can key off all customizations from by looking at this pref (and observing it for changes): "ril.lastKnownHomeNetwork". Hooray!
(Assignee)

Comment 2

5 years ago
Going to take a stab at a basic helper object to detect whether customizations should be applied. This helper will be the first piece to receive a unit-test. This should ensure that all other customizations can be triggered as expected.
Status: NEW → ASSIGNED
(Assignee)

Comment 3

5 years ago
RFC -- https://github.com/nullaus/gaia/compare/market-customizations

A helper object that can be used to trigger customizations for any application. Based on apps/system/js/operator_variant.js. Let me know what you guys think about this approach.
Looks like it might be missing the listener argument for function OperatorVariantListener?

Seems to make sense other than that, but I'm just about the last person that should be reviewing js. :)
Summary: Create base marionette test case for Gaia System Customization work → Create base unit test for Gaia System Customization work
(Assignee)

Comment 5

5 years ago
Created attachment 783378 [details]
Patch - v1 - OperatorVariantListener and Base Unit Tests
Attachment #783378 - Flags: review?
(Assignee)

Updated

5 years ago
Attachment #783378 - Flags: review? → review?(dflanagan)
Attachment #783378 - Attachment mime type: text/plain → text/html
(Assignee)

Updated

5 years ago
Attachment #783378 - Flags: review?(felash)
Attachment #783378 - Flags: review?(dflanagan)
Attachment #783378 - Flags: review?
Assignee: nobody → aus
(Assignee)

Updated

5 years ago
Attachment #783378 - Flags: review? → review?(gnarf37)
Comment on attachment 783378 [details]
Patch - v1 - OperatorVariantListener and Base Unit Tests

I've added my comments on github regarding the tests part, and some small comments on the actual code.

I don't feel I'm very qualified on this code (besides the tests) so I'm asking :timdream a review too.

Please ask me review again when my comments are fixed :)
Attachment #783378 - Flags: review?(timdream)
Attachment #783378 - Flags: review?(gnarf37)
Attachment #783378 - Flags: review?(felash)
Ghislain 'Aus' Lacroix changed story state to started in Pivotal Tracker
Comment on attachment 783378 [details]
Patch - v1 - OperatorVariantListener and Base Unit Tests

Sorry for the long delay ... I was loaded.

Many of the SIM-based customization involves setting a user-change-able default to the app. I wonder if the help library serves that well? I mean, if the user changes her/his SIM, does that really means we could overwrite whatever value he/she had already set implicitly?

Obviously the question should be asked when an app is actually using the library, so I am setting the r+ anyway.
Attachment #783378 - Flags: review?(timdream) → review+
(Assignee)

Comment 9

5 years ago
Hi Tim! Thanks for the review.

The applications can opt to do what they like. For our own stack of applications we will only apply the customizations _once_ EXCEPT in the case of Data, MMS, A-GPS and similar configuration items that are very dependent on the network the user is on.

That's why the helper itself supports both a one time check at run-time as well as the ability to listen for further changes in SIM cards.

Hope that clears things up. :)
(Assignee)

Updated

5 years ago
Attachment #783378 - Flags: review?(felash)
(Assignee)

Comment 10

5 years ago
Hi Julien,

I've updated a lot of the tests to use the latest and greatest mocks helper. I think we're good to go as long as you approve. :)
(Assignee)

Comment 11

5 years ago
Work for this is complete, merged to master.
Ghislain 'Aus' Lacroix changed story state to finished in Pivotal Tracker
Ghislain 'Aus' Lacroix changed story state to delivered in Pivotal Tracker
(In reply to Ghislain 'Aus' Lacroix from comment #11)
> Work for this is complete, merged to master.

Can you include a commit and move this to resolved fixed then?
(Assignee)

Comment 15

5 years ago
Resolved Fixed

Commit: https://github.com/nullaus/gaia/commit/510a4be9c0aa06b9cf2aa59297155c414b0137f4
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Ghislain> you specifically asked me a review for the test part, therefore you should not merge without my last review, even if Tim gave you r+ for the Operator Variant part.

You can merge after doing changes if the reviewer says "r+ with nits" but that's the only case.

I had a look and the patch looks ok to me but I'd appreciate that you wait next time. I really considered backing out the patch, you know :)
Comment on attachment 783378 [details]
Patch - v1 - OperatorVariantListener and Base Unit Tests

r=me for the test part
Attachment #783378 - Flags: review?(felash) → review+
master merge commit is 43557e5392ab37d4f8c1fa08184cf1541b249a54
Candice Serran changed story state to accepted in Pivotal Tracker

Updated

5 years ago
Flags: in-moztrap-
You need to log in before you can comment on or make changes to this bug.