Closed
Bug 1459161
Opened 7 years ago
Closed 6 years ago
test_accounts.js doesn't correctly test for a bad call for fxa.getAssertion("noaudience")
Categories
(Firefox :: Firefox Accounts, defect)
Firefox
Firefox Accounts
Tracking
()
RESOLVED
FIXED
Firefox 62
People
(Reporter: standard8, Assigned: markh)
References
Details
Attachments
(1 file)
In test_accounts.js, test_getAssertion there is currently this check:
```
do_check_throws(async function() {
await fxa.getAssertion("nonaudience");
});
```
do_check_throws is defined later in the file: https://searchfox.org/mozilla-central/rev/c0d81882c7941c4ff13a50603e37095cdab0d1ea/services/fxaccounts/tests/xpcshell/test_accounts.js#1513-1529
There's two issues with this:
- do_check_throws isn't designed to deal with async functions, so it is unlikely that any actual throw would be caught.
- do_check_throws doesn't complain if the second parameter (result) is not passed and the function doesn't throw.
The code was added in bug 909967.
As far as I can tell, fxa.getAssertion never rejects or asserts called in this test at this point.
Looking at getAssertion's code, it appears there's various cases where it returns null, these look to have been added in bug 967120, after bug 909967.
I think the test probably has the right intentions, but is wrongly set up for this case.
Ideally, a new implementation should drop do_check_throws and use either Assert.rejects or Assert.throws instead.
Assignee | ||
Comment 1•6 years ago
|
||
Thanks Mark - I don't think that specific check has any value given the rest of the test, so I'll put a change up that removes that check and do_check_throws entirely.
I also noticed the test is doing a number of other dumb things, including pulling on Sync causing it to spew many errors, and also leaving a number of email polls still running, so I'll push a revision that fixes all of these and ask Ed to review.
Assignee: nobody → markh
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8979164 [details]
Bug 1459161 - fix various small issues with FxA's test_accounts.js.
https://reviewboard.mozilla.org/r/245418/#review251420
Attachment #8979164 -
Flags: review?(eoger) → review+
Pushed by mhammond@skippinet.com.au:
https://hg.mozilla.org/integration/autoland/rev/3fd5dd6c1bb4
fix various small issues with FxA's test_accounts.js. r=eoger
Comment 5•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•