Closed
Bug 996452
Opened 10 years ago
Closed 10 years ago
[NFC] Add emulator rf_discover test case
Categories
(Firefox OS Graveyard :: NFC, defect)
Tracking
(tracking-b2g:backlog)
People
(Reporter: dimi, Assigned: dimi)
Details
(Whiteboard: [p=2])
Attachments
(1 file, 2 obsolete files)
4.02 KB,
patch
|
Details | Diff | Splinter Review |
After landing Bug 984397 , we will need a test case for emulator rf_discover
Assignee | ||
Updated•10 years ago
|
Whiteboard: [p=2]
Assignee | ||
Updated•10 years ago
|
Summary: [NDC] Add emulator rf_discover test case → [NFC] Add emulator rf_discover test case
Assignee | ||
Comment 1•10 years ago
|
||
Updated•10 years ago
|
blocking-b2g: --- → backlog
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #8406680 -
Attachment is obsolete: true
Attachment #8408878 -
Flags: review?(tzimmermann)
Comment 3•10 years ago
|
||
Comment on attachment 8408878 [details] [diff] [review] rf_discover test case patch V1 Review of attachment 8408878 [details] [diff] [review]: ----------------------------------------------------------------- In general this looks good and the test passed locally. You can r+ me with this one question addressed and the style cleanups. I ran gjslint, which complained about a few style errors. I'd like to seem them fixed, if they were introduced by the patch and the fix doesn't violate any other policies. For test_nfc_manager_tech_discovered.js it is Line 7, E:0131: Single-quoted string preferred over double-quoted string. Line 7, E:0110: Line too long (84 characters). Line 10, E:0001: Extra space after "NCI_LAST_NOTIFICATION" and for head.js it's Line 6, E:0131: Single-quoted string preferred over double-quoted string. Line 17, E:0131: Single-quoted string preferred over double-quoted string. Line 24, E:0131: Single-quoted string preferred over double-quoted string. Line 32, E:0010: Missing semicolon at end of line ::: dom/nfc/tests/marionette/test_nfc_manager_tech_discovered.js @@ +56,5 @@ > +// DISCOVERY -> W4_ALL_DISCOVERIES -> W4_HOST_SELECT -> POLL_ACTIVE > +function testRfDiscover() { > + log('Running \'testRfDiscover\''); > + window.navigator.mozSetMessageHandler( > + 'nfc-manager-tech-discovered', handleTechnologyDiscoveredRE0); Will this callback run for every discovered RE, or just after activating one of them? Because the handler turns off NFC, and if that happens on the first detected RE, the results for the second RE are undefined.
Attachment #8408878 -
Flags: review?(tzimmermann) → review+
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #3) > Comment on attachment 8408878 [details] [diff] [review] > rf_discover test case patch V1 > > Review of attachment 8408878 [details] [diff] [review]: > ----------------------------------------------------------------- > > In general this looks good and the test passed locally. You can r+ me with > this one question addressed and the style cleanups. > > I ran gjslint, which complained about a few style errors. I'd like to seem > them fixed, if they were introduced by the patch and the fix doesn't violate > any other policies. For test_nfc_manager_tech_discovered.js it is > > Line 7, E:0131: Single-quoted string preferred over double-quoted string. > Line 7, E:0110: Line too long (84 characters). > Line 10, E:0001: Extra space after "NCI_LAST_NOTIFICATION" > > and for head.js it's > > Line 6, E:0131: Single-quoted string preferred over double-quoted string. > Line 17, E:0131: Single-quoted string preferred over double-quoted string. > Line 24, E:0131: Single-quoted string preferred over double-quoted string. > Line 32, E:0010: Missing semicolon at end of line > > ::: dom/nfc/tests/marionette/test_nfc_manager_tech_discovered.js > @@ +56,5 @@ > > +// DISCOVERY -> W4_ALL_DISCOVERIES -> W4_HOST_SELECT -> POLL_ACTIVE > > +function testRfDiscover() { > > + log('Running \'testRfDiscover\''); > > + window.navigator.mozSetMessageHandler( > > + 'nfc-manager-tech-discovered', handleTechnologyDiscoveredRE0); > > Will this callback run for every discovered RE, or just after activating one > of them? Because the handler turns off NFC, and if that happens on the first > detected RE, the results for the second RE are undefined. This callback will only run after activating , :)
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8408878 -
Attachment is obsolete: true
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Dimi Lee[:dimi][:dlee] from comment #4) > (In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #3) > > Comment on attachment 8408878 [details] [diff] [review] > > rf_discover test case patch V1 > > > > Review of attachment 8408878 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > In general this looks good and the test passed locally. You can r+ me with > > this one question addressed and the style cleanups. > > > > I ran gjslint, which complained about a few style errors. I'd like to seem > > them fixed, if they were introduced by the patch and the fix doesn't violate > > any other policies. For test_nfc_manager_tech_discovered.js it is > > > > Line 7, E:0131: Single-quoted string preferred over double-quoted string. > > Line 7, E:0110: Line too long (84 characters). > > Line 10, E:0001: Extra space after "NCI_LAST_NOTIFICATION" > > > > and for head.js it's > > > > Line 6, E:0131: Single-quoted string preferred over double-quoted string. > > Line 17, E:0131: Single-quoted string preferred over double-quoted string. > > Line 24, E:0131: Single-quoted string preferred over double-quoted string. > > Line 32, E:0010: Missing semicolon at end of line From MDN coding style, i think we prefer using double-quoted string for js. So i will not modify this part , are you fine with this ?
Flags: needinfo?(tzimmermann)
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/509937940bc6
Flags: in-testsuite+
Keywords: checkin-needed
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/509937940bc6
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•9 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
You need to log in
before you can comment on or make changes to this bug.
Description
•