749907 - Common BrowserID module needs tests

Status: RESOLVED WONTFIX

(Firefox :: Firefox Accounts, defect)

Description

[Anant Narayanan [:anant]]

We need some assurance that the methods in the common BrowserID module actually do what they claim to...

Comment 1

[Anant Narayanan [:anant]]

Attachment: Tests for BrowserID module - v1

These are browser-chrome tests to verify functionality provided by the common BrowserID module. I'll briefly describe each test, there are 4, and they are numbered, because order matters:

1. Verifies that getAssertion does not take illegal arguments and that no assertion is retrieved because there is no user currently signed in to BrowserID.

2. Opens marketplace.mozilla.org, and calls getAssertionWithLogin which will result in a BrowserID login popup. The test scripts a login as a test user and verifies that a correct assertion was retrieved.

3. Attempts to retrieve an assertion for google.com silently, and verifies that we receive a correct assertion. This should succeed, because we're now signed in to BrowserID as a result of test 2.

4. Attempts to retrieve an assertion for myapps.mozillalabs.com using the same email that was used to sign in to marketplace.mozilla.org, and verifies that a correct one is received. This should also work silently as a side effect of test 2.

Now, these tests are all a bit weird, we're depending on external sites (though I will note that BrowserID, the Marketplace, and the Dashboard are all controlled by Mozilla). I don't think they're suitable for check-in, simply because they are too sensitive to timeouts - we only have 30 seconds for each tests - and Marketplace/BrowserID are heavy sites that we can't rely on loading fast every time.

However, they serve great as tests one can run locally to verify that the BrowserID module is working as intended. It's been very useful to me as I iterate both on the BrowserID module and the AITC client.

Comment 2

[Anant Narayanan [:anant]]

The tests don't all pass, besides the intermittent timeouts (based on network disruption or simply server lag), I get the following persistent failures:

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/services/identity/tests/browser_identity_1_fresh.js | leaked window property: BrowserID
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/services/identity/tests/browser_identity_2_login.js | an unexpected uncaught JS exception reported through window.onerror - SyntaxError: XML tag name mismatch (expected img) at https://marketplace.mozilla.org/en-US/login:121
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/services/identity/tests/browser_identity_2_login.js | an unexpected uncaught JS exception reported through window.onerror - SyntaxError: XML can't be the whole program at https://marketplace.mozilla.org/en-US/login:148
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/services/identity/tests/browser_identity_2_login.js | an unexpected uncaught JS exception reported through window.onerror - SyntaxError: XML can't be the whole program at https://marketplace.mozilla.org/en-US/login:153

The last 3 are simply errors on the marketplace login page, but I'm not sure how to go about marking them as 'EXPECTED-FAIL'ures.

The first one seems worth looking into, but it only occurs on the first test. I'm guessing it has something to do with XPCOMUtils.defineLazyGetter, the BrowserID object is created in the first test but is available for all the others.

Clint, if you have any tips, I'd love to hear them!

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Unfortunately I don't think it's going to be feasible to have these tests in-tree that depend on the network. If we're going to land this web dependency in the product, we should probably focus on separately testing the web-side functionality (I imagine browserid.org has its own automated testing/monitoring?) and client-side-only functionality (using in-tree tests), and finding some other mechanism for monitoring their interaction (outside of our buildbot testing infra).

Comment 4

[Gregory Szorc [:gps]]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #3)
> Unfortunately I don't think it's going to be feasible to have these tests
> in-tree that depend on the network.

Yup. We're well aware of that. We'll try to do as much as we can with mocking in xpcshell and mochitests. We'll supplement that with something like Sync's TPS tests (which are checked in to the tree but don't run as part of the normal test process; instead they run on separate hardware with network access). In the immediate term, we may even need some manual testing :/

Comment 5

[Anant Narayanan [:anant]]

Attachment: Tests for BrowserID module - v1.1

Updated to match changes in bug 745345. I was able to fix the BrowserID "leak" by doing Cu.import(..., obj) instead of Cu.import(...).

Comment 6

[Richard Newman [:rnewman]]

I don't see a BrowserID module in services/common. Should this bug be in Core :: Identity?

Comment 7

[Ryan Kelly [:rfkelly]]

Re-categorizing to Firefox Accounts; we may still want some of these tests for the BrowserID stuff we use in Firefox Accounts, and any untested code that's not needed for FxA should just be removed.