B2G NetworkManager: 'allNetworkInfo' implementation missing

RESOLVED FIXED in Firefox 43

Status

Firefox OS
RIL
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jessica, Assigned: jessica)

Tracking

unspecified
FxOS-S7 (18Sep)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

2 years ago
In bug 1167132, we exposed 'allNetworkInfo' instead of 'networkInterfaces', but I missed the implementation for it :(.
(Assignee)

Comment 1

2 years ago
Created attachment 8651650 [details] [diff] [review]
patch, v1.
(Assignee)

Comment 2

2 years ago
Turns out that only TetheringService is using NetworkManager.allNetworkInfo and it uses a default value if it can not find a proper one, so we didn't catch this issue.
(Assignee)

Comment 3

2 years ago
Comment on attachment 8651650 [details] [diff] [review]
patch, v1.

Edgar, may I have your review on this? Thanks.
Attachment #8651650 - Flags: review?(echen)

Updated

2 years ago
Blocks: 1167132
(Assignee)

Updated

2 years ago
Blocks: 1187262
(Assignee)

Comment 4

2 years ago
I'll add a test case for this to prevent future mistakes.

Comment 5

2 years ago
Comment on attachment 8651650 [details] [diff] [review]
patch, v1.

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

Looks good, but please still provide a test case for allNetworkInfo. Thank you.
Attachment #8651650 - Flags: review?(echen) → review+
(Assignee)

Comment 6

2 years ago
Created attachment 8653842 [details] [diff] [review]
Part 1: allNetworkInfo implementation. r=echen

Commit message amended.
Attachment #8651650 - Attachment is obsolete: true
Attachment #8653842 - Flags: review+
(Assignee)

Comment 7

2 years ago
Created attachment 8653843 [details] [diff] [review]
Part 2: allNetworkInfo tests, v1.
(Assignee)

Comment 8

2 years ago
Created attachment 8654743 [details] [diff] [review]
Part 2: allNetworkInfo tests, v2.

Changes since v1:
- use settings to enable/disable wifi, otherwise wifi test cases using settings will fail afterwards.

Edgar, may I have your review on the test case part? Thanks.
Attachment #8653843 - Attachment is obsolete: true
Attachment #8654743 - Flags: review?(echen)

Comment 9

2 years ago
Comment on attachment 8654743 [details] [diff] [review]
Part 2: allNetworkInfo tests, v2.

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

Thank you.
Attachment #8654743 - Flags: review?(echen) → review+
(Assignee)

Comment 10

2 years ago
Created attachment 8657988 [details] [diff] [review]
Part 2: allNetworkInfo tests. r=echen

Sorry Edgar, this should be the correct patch, using settings to enable/disable wifi, else wifi tests will fail. Another thing that I modified, is to restore wifi to its original state.

Would you mind reviewing again? Thanks.
Attachment #8654743 - Attachment is obsolete: true
Attachment #8657988 - Flags: review?(echen)

Updated

2 years ago
Attachment #8657988 - Flags: review?(echen) → review+
(Assignee)

Updated

2 years ago
Attachment #8657988 - Attachment description: Part 2: allNetworkInfo tests, v3. → Part 2: allNetworkInfo tests. r=echen
(Assignee)

Comment 11

2 years ago
try looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a17d5ca77aa6&group_state=expanded&exclusion_profile=false

Comment 12

2 years ago
https://hg.mozilla.org/integration/b2g-inbound/rev/115f1a4faea6
https://hg.mozilla.org/integration/b2g-inbound/rev/ab2b5214e325
https://hg.mozilla.org/mozilla-central/rev/115f1a4faea6
https://hg.mozilla.org/mozilla-central/rev/ab2b5214e325
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S7 (18Sep)
You need to log in before you can comment on or make changes to this bug.