missing await in browser.pkcs11 test suite

RESOLVED FIXED in Firefox 58

Status

enhancement
P3
normal
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: wouter, Assigned: wouter)

Tracking

58 Branch
mozilla58
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

2 years ago
While reviewing the file test_pkcs11_management.js added in bug 1357391 for inspiration for my own PKCS#11 management add-on, I noticed that I forgot one "await" in the test suite. The result is that there will be an assertTrue() on a Promise (which will always be successful) rather than on its resolved result.

This shouldn't cause the test suite to succeed when it shouldn't, because if the module isn't successfully loaded then the next batch of tests that try to use it will fail, but it's confusing and not proper.
Assignee

Updated

2 years ago
Summary: missing await → missing await in browser.pkcs11 test suite
Comment hidden (mozreview-request)

Comment 2

2 years ago
mozreview-review
Comment on attachment 8915171 [details]
Bug 1405709 - Add missing await to browser.pkcs11 test suite

https://reviewboard.mozilla.org/r/186424/#review191970
Attachment #8915171 - Flags: review?(tomica) → review+
Assignee: nobody → w
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee

Updated

2 years ago
Keywords: checkin-needed

Comment 3

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f5d73b59a8da
Add missing await to browser.pkcs11 test suite r=zombie
Keywords: checkin-needed
Priority: -- → P3
https://hg.mozilla.org/mozilla-central/rev/f5d73b59a8da
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Thanks for the patch, :wouter! Your contribution has been added to the add-ons recognition wiki: https://wiki.mozilla.org/Add-ons/Contribute/Recognition#October_2017

If you'd like to set up a profile on mozillians.org, I'd be happy to vouch for you. 

Welcome onboard!

Comment 6

2 years ago
Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.
Flags: needinfo?(w)

Comment 7

2 years ago
Added "qe-verify-" flag (test-only change).
Flags: needinfo?(w) → qe-verify-

Updated

Last year
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.