Closed Bug 1055959 Opened 7 years ago Closed 6 years ago
Nfc Bad Session Id Error testcase fail
Run following command ./mach marionette-webapi ./gecko/dom/nfc/tests/marionette/manifest.ini Testcase testNfcBadSessionIdError in test_nfc_error_messages.js will fail. This is related to Bug 1050621, because the bug fixing change the behavior of nfcd (do not reset rf when receive deactivate notification). So when emulator deactivate remote end point and try to activate it again, it will fail to activate, I will fix this bug in emulator side.
7 years ago
There's something wrong in nci state transition. Deactivate() called from JS does not trigger state transition, as the result the next activate command will throw an error. According to NFC Controller Interface (NCI) Specification, NCI state should be changed after NFCC send RF_DEACTIVATE_RSP/NTF. Thanks
Comment on attachment 8687057 [details] [review] Qemu PR v1 Comments addressed in PR, please ask for feedback again after comments answered, thanks!
- Removed unused code - Code readability
Enable nfc test case
Attachment #8687762 - Flags: review?(allstars.chh)
6 years ago
Attachment #8687762 - Flags: review?(allstars.chh) → review+
Comment on attachment 8687752 [details] [review] external qemu PR Looks good to me, thanks!
Attachment #8687752 - Flags: feedback?(dlee) → feedback+
Comment on attachment 8687752 [details] [review] external qemu PR Hi! thanks for the fixing this bug. It's been a while since I worked on this code, so I had to dig through it to understand what the problem is. Let me know if I missed the point somewhere. I left a number of comments in the pull request. Please ask me for another review once you've gone through them.
Thanks for your review.
Comment on attachment 8688283 [details] [review] external qemu v2 Looks good now. Thanks for this fix! Please have a look at the nits I commented in the pull request before landing the patch.
Attachment #8688283 - Flags: review?(tzimmermann) → review+
- Fixed nits Thanks Thomas for your review.
I saw that you closed the old pull requests on Github. Instead, you can also just push the changes into your source branch on Github and the pull request should be updated automatically. Might be more comfortable.
(In reply to Thomas Nguyen[:tnguyen][:thomas][:nguyen] from comment #11) > Try result > https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ee8de98a10e It's not that easy because try won't pick-up your emulator changes by default. Only Gecko changes will be tested. To modify one of the other repositories, you have to modify the respective sources.xml file under b2g/config. For example to test the change to emulator-x86-kk, you'd have to change b2g/config/emulator-x86-kk/sources.xml. You have to add your Github account to the repositories and change the remote and revision of the non-Gecko project to point to your patch. If you push that change to try, try will fetch your non-Gecko modifications.
Try result looks ok. Seems that the test cases were not run. I will do as you said and run try again. Thanks
(In reply to Thomas Nguyen[:tnguyen][:thomas][:nguyen] from comment #14) > Try result looks ok. Seems that the test cases were not run. > I will do as you said and run try again. > Thanks I don't think we currently run automated NFC tests.
Thanks Thomas, Currently, marionette-webapi nfc tests were skipped in testing/marionette/client/marionette/tests/webapi-tests.ini). Also, some job flags would be changed to enable running test case in try. I will try to modify and see Remove checkin-needed to cherry-pick to other emulator platforms (ics, lollipop, jb, ...) and push to try.
Try (staging) result running marionette-webapi https://treeherder.allizom.org/#/jobs?repo=try&revision=bfd326e5ce7d&selectedJob=14334850&filter-tier=1 The test result failed (but it failed in test_cellbroadcast_umts_language_and_body.js, it's another story) All Nfc test cases passed.
You need to log in before you can comment on or make changes to this bug.