This depends on the emulator patches. Some basic tests should be OK for now.

Failed to build Gecko with the patches. The error occurred while processing the following file: /home/vicamo/WorkSpace/mozilla/b2g/qemu/jb/arm/master/2/gecko/dom/nfc/tests/moz.build The error was triggered on line 7 of this file: MODULE = 'test_dom_nfc'

Oopsi, bitrotted because of bug 93904. Should work now.

Comment on attachment 8341683 [details] [diff] [review] [01] Bug 935525: Added NFC test infrastructure to build scripts (v2) Review of attachment 8341683 [details] [diff] [review]: ----------------------------------------------------------------- have to add to testing/marionette/client/marionette/tests/unit-tests.ini as well

Comment on attachment 831540 [details] [diff] [review] [02] Bug 935525: Added marionette test for turning NFC on and off Review of attachment 831540 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/nfc/tests/marionette/manifest.ini @@ +2,5 @@ > b2g=true > browser=false > qemu=true > + > +[test_nfc_enable.js] test_nfc_enable"d".js

Comment on attachment 831542 [details] [diff] [review] [03] Bug 935525: Added Marionette test for nfc-manager-tech-discovered Review of attachment 831542 [details] [diff] [review]: ----------------------------------------------------------------- This part crashes emulator. See https://github.com/mozilla-b2g/platform_external_qemu/pull/52#issuecomment-32143430

I saw Vicamo has already given some comments before, Vicamo, would you like to review this? (I got some 1.3+ bugs and may not review these soon).

Comment on attachment 8341683 [details] [diff] [review] [01] Bug 935525: Added NFC test infrastructure to build scripts (v2) Review of attachment 8341683 [details] [diff] [review]: ----------------------------------------------------------------- See comment 7. Or these cases won't be activated by default.

Still get crashed after sending 'nfc ntf rf_intf_activated 0' either by test script or manually.

I found: I/USERIAL_LINUX( 47): USERIAL_Open(): enter I/USERIAL_LINUX( 47): USERIAL_Open() device: /dev/bcm2079x port=5, uart_port=0 WAKE_DELAY(0) WRITE_DELAY(0) POWER_ON_DELAY(300) PRE_POWER_OFF_DELAY(0) POST_POWER_OFF_DELAY(0) I/USERIAL_LINUX( 47): USERIAL_Open unable to open /dev/bcm2079x I/USERIAL_LINUX( 47): USERIAL_Open(): exit Does that matter?

Same assertion triggered.

Comment on attachment 8362899 [details] [diff] [review] [02] Bug 935525: Added Marionette test for turning NFC on and off (v2) Review of attachment 8362899 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/nfc/tests/marionette/test_nfc_enabled.js @@ +3,5 @@ > + > +MARIONETTE_TIMEOUT = 30000; > + > +SpecialPowers.addPermission('nfc', true, document); > +SpecialPowers.addPermission('settings', true, document); We'd better not using `{add,remove}Permission` in newly committed code. Please use `pushPermissions` instead.

Comment on attachment 8362900 [details] [diff] [review] [03] Bug 935525: Added Marionette test for nfc-manager-tech-discovered (v2) Review of attachment 8362900 [details] [diff] [review]: ----------------------------------------------------------------- This part still leads to emulator crash but probably should not be the one to blame. ::: dom/nfc/tests/marionette/test_nfc_manager_tech_discovered.js @@ +3,5 @@ > + > +MARIONETTE_TIMEOUT = 30000; > + > +SpecialPowers.addPermission('nfc-manager', true, document); > +SpecialPowers.addPermission('settings', true, document); ditto.

Comment on attachment 8362900 [details] [diff] [review] [03] Bug 935525: Added Marionette test for nfc-manager-tech-discovered (v2) Review of attachment 8362900 [details] [diff] [review]: ----------------------------------------------------------------- With fix in bug 916863, these test cases finally turn green!

Added fixes from previous review.

Added fixes from previous review.

Added fixes from previous review.

Comment on attachment 8366537 [details] [diff] [review] [01] Bug 935525: Added NFC test infrastructure to build scripts (v3) Review of attachment 8366537 [details] [diff] [review]: ----------------------------------------------------------------- Ask :jgriffin's review as well.

Comment on attachment 8366541 [details] [diff] [review] [02] Bug 935525: Added Marionette test for turning NFC on and off (v3) Review of attachment 8366541 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/nfc/tests/marionette/test_nfc_enabled.js @@ +63,5 @@ > + finish(); > +} > + > +SpecialPowers.pushPermissions( > + [{'type':'settings', 'allow':true, 'context':document}], runNextTest()); s/runNextTest()/runNextTest/

Comment on attachment 8366542 [details] [diff] [review] [03] Bug 935525: Added Marionette test for nfc-manager-tech-discovered (v3) Review of attachment 8366542 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/nfc/tests/marionette/test_nfc_manager_tech_discovered.js @@ +73,5 @@ > +} > + > +SpecialPowers.pushPermissions( > + [{'type':'nfc-manager', 'allow':true, context:document}, > + {'type':'settings', 'allow':true, context:document}], runNextTest()); ditto

Comment on attachment 8370823 [details] [diff] [review] [03] Bug 935525: Added Marionette test for nfc-manager-tech-discovered (v4) Review of attachment 8370823 [details] [diff] [review]: ----------------------------------------------------------------- All verified with current emulator-jb build. Thank you!

Comment on attachment 8366537 [details] [diff] [review] [01] Bug 935525: Added NFC test infrastructure to build scripts (v3) Review of attachment 8366537 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/nfc/tests/marionette/manifest.ini @@ +1,4 @@ > +[DEFAULT] > +b2g=true > +browser=false > +qemu=true We can only run these tests on emulator-jb or above, so there are still something to do here. :(

backedout from inbound on request from tdz in 855c5a0c6828

(In reply to Carsten Book [:Tomcat] from comment #31) > backedout from inbound on request from tdz in 855c5a0c6828 Oops, I don't know they're already in inbound :X

v5 of the Marionette tests immediately succeed on systems without NFC. This makes them work on both, emulator and emulator-js.

s/emulator-js/emulator-jb

Comment on attachment 8372285 [details] [diff] [review] [02] Bug 935525: Added Marionette test for turning NFC on and off (v5) Review of attachment 8372285 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/nfc/tests/marionette/test_nfc_enabled.js @@ +64,5 @@ > + if ('mozNfc' in window.navigator) { > + runNextTest(); > + } else { > + // succeed immediately on systems without NFC > + ok(true, 'Failed on system without NFC'); s/Failed/Skipped/. Please also have a |log(...)| line to print on console directly. |ok(...)| is only printed in output of ｀adb logcat`

Comment on attachment 8372287 [details] [diff] [review] [03] Bug 935525: Added Marionette test for nfc-manager-tech-discovered (v5) Review of attachment 8372287 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/nfc/tests/marionette/test_nfc_manager_tech_discovered.js @@ +73,5 @@ > + if ('mozNfc' in window.navigator) { > + runNextTest(); > + } else { > + // succeed immediately on systems without NFC > + ok(true, 'Failed on system without NFC'); ditto.

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #37) > Comment on attachment 8372285 [details] [diff] [review] > [02] Bug 935525: Added Marionette test for turning NFC on and off (v5) > > Review of attachment 8372285 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/nfc/tests/marionette/test_nfc_enabled.js > @@ +64,5 @@ > > + if ('mozNfc' in window.navigator) { > > + runNextTest(); > > + } else { > > + // succeed immediately on systems without NFC > > + ok(true, 'Failed on system without NFC'); > > s/Failed/Skipped/. Please also have a |log(...)| line to print on console > directly. |ok(...)| is only printed in output of ｀adb logcat` Correct me if I'm wrong, but it seems that this 'bogus' test is necessary. When I called finish() directly, Marionette complained that no tests have been executed. So I added the ok(), and 'Failed...' is just the error message.

(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #39) > (In reply to Vicamo Yang [:vicamo][:vyang] from comment #37) > > s/Failed/Skipped/. Please also have a |log(...)| line to print on console > > directly. |ok(...)| is only printed in output of ｀adb logcat` > > Correct me if I'm wrong, but it seems that this 'bogus' test is necessary. > When I called finish() directly, Marionette complained that no tests have > been executed. So I added the ok(), and 'Failed...' is just the error > message. Yes, that's necessary, and that's why I used "also". :)

OK, next round. I added some logging to the tests. I also moved the generic code into a separate file.

Comment on attachment 8373910 [details] [diff] [review] [03] Bug 935525: Added Marionette test for nfc-manager-tech-discovered (v6) Review of attachment 8373910 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/nfc/tests/marionette/test_nfc_manager_tech_discovered.js @@ +13,5 @@ > + > +function activateRE0() { > + var cmd = 'nfc ntf rf_intf_activated 0'; > + log('Executing \'' + cmd + '\''); > + runEmulatorCmd(cmd, function(result) { Should have noticed. Whenever we're calling built-in `runEmulatorCmd()`, we have to manually keep track of its execution count and make sure all of the calls are already finished before calling `finish()` at the end of the test script. Please see https://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/tests/marionette/head.js#270 for example.

Ok, great. I'll update and land the patches then. Thanks a lot!

https://hg.mozilla.org/integration/b2g-inbound/rev/6e1e721b044a https://hg.mozilla.org/integration/b2g-inbound/rev/43b33c890e57 https://hg.mozilla.org/integration/b2g-inbound/rev/5dc42ca15601 https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=5dc42ca15601

https://hg.mozilla.org/mozilla-central/rev/6e1e721b044a https://hg.mozilla.org/mozilla-central/rev/43b33c890e57 https://hg.mozilla.org/mozilla-central/rev/5dc42ca15601