Closed Bug 1459161 Opened 3 years ago Closed 3 years ago

test_accounts.js doesn't correctly test for a bad call for fxa.getAssertion("noaudience")


(Firefox :: Firefox Accounts, defect)

Not set



Firefox 62
Tracking Status
firefox61 --- wontfix
firefox62 --- fixed


(Reporter: standard8, Assigned: markh)




(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:

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.
No longer blocks: 1458235
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
Comment on attachment 8979164 [details]
Bug 1459161 - fix various small issues with FxA's test_accounts.js.
Attachment #8979164 - Flags: review?(eoger) → review+
Pushed by
fix various small issues with FxA's test_accounts.js. r=eoger
Blocks: 1452706
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
You need to log in before you can comment on or make changes to this bug.