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

Status: RESOLVED FIXED

(Firefox :: Firefox Accounts, defect)

Description

[Mark Banner (:standard8)]

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.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

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.

Comment 2

[Mark Hammond [:markh] [:mhammond]]

Attachment: Bug 1459161 - fix various small issues with FxA's test_accounts.js.

Review commit: https://reviewboard.mozilla.org/r/245418/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245418/

Comment 3

[Edouard Oger [:eoger]]

Comment on attachment 8979164 [details]
Bug 1459161 - fix various small issues with FxA's test_accounts.js.

https://reviewboard.mozilla.org/r/245418/#review251420

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3fd5dd6c1bb4
fix various small issues with FxA's test_accounts.js. r=eoger

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/3fd5dd6c1bb4