Closed Bug 1197667 Opened 9 years ago Closed 9 years ago

B2G NetworkManager: 'allNetworkInfo' implementation missing

Categories

(Firefox OS Graveyard :: RIL, defect)

Product:
Firefox OS Graveyard
Component:
RIL
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox43 fixed)

Status:
RESOLVED FIXED
Milestone:
FxOS-S7 (18Sep)
Tracking Flags:
Tracking Status
firefox43 --- fixed

People

(Reporter: jessica, Assigned: jessica)

References

Details

Attachments

(2 files, 3 obsolete files)

Part 1: allNetworkInfo implementation. r=echen
(deleted), patch
jessica
: review+
Details | Diff | Splinter Review
Part 2: allNetworkInfo tests. r=echen
(deleted), patch
edgar
: review+
Details | Diff | Splinter Review
In bug 1167132, we exposed 'allNetworkInfo' instead of 'networkInterfaces', but I missed the implementation for it :(.
Attached patch patch, v1. (obsolete) (deleted) — — Splinter Review
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.
Comment on attachment 8651650 [details] [diff] [review] patch, v1. Edgar, may I have your review on this? Thanks.
Attachment #8651650 - Flags: review?(echen)
Blocks: 1167132
Blocks: 1187262
I'll add a test case for this to prevent future mistakes.
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+
Commit message amended.
Attachment #8651650 - Attachment is obsolete: true
Attachment #8653842 - Flags: review+
Attached patch Part 2: allNetworkInfo tests, v1. (obsolete) (deleted) — — Splinter Review
Attached patch Part 2: allNetworkInfo tests, v2. (obsolete) (deleted) — — Splinter Review
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 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+
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)
Attachment #8657988 - Flags: review?(echen) → review+
Attachment #8657988 - Attachment description: Part 2: allNetworkInfo tests, v3. → Part 2: allNetworkInfo tests. r=echen
https://hg.mozilla.org/mozilla-central/rev/115f1a4faea6 https://hg.mozilla.org/mozilla-central/rev/ab2b5214e325
Status: NEW → RESOLVED
Closed: 9 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.

Attachment

General

Creator:
Created:
Updated:
Size: