615643 - Heisenfailure in test_service_verifyLogin in xulrunner test suite

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Richard Newman [:rnewman]]

Passes in xpcshell-tests, and when run on its own.

Investigating now.

Comment 1

[Richard Newman [:rnewman]]

Failure is that the passphrase is *not* undefined when evaluating part of verifyLogin:

            // We have no way of verifying the passphrase right now,
            // so wait until remoteSetup to do so.
            // Just make the most trivial checks.
            if (!this.passphrase) {
              this._log.warn("No passphrase in verifyLogin.");
              Status.login = LOGIN_FAILED_NO_PASSPHRASE;
              return false;
            }

            // Go ahead and do remote setup, so that we can determine 
            // conclusively that our passphrase is correct.
            if (this._remoteSetup()) {


This is not what the test expects. Because the passphrase exists, verifyLogin goes right ahead and starts setting up the server, returning true.

Why does the passphrase exist? Because it's an Identity, and thus it's fetched from the login manager after being set by a previous test. Debug logging:

---
Service.Main	WARN	No username in verifyLogin.
Try again with username and password set.
Service.Main	DEBUG	Caching URLs under storage user base: http://localhost:8080/1.0/johndoe/
Identity	INFO	Password is null, looking up...
Identity	INFO	Password is now mnkp3i34hkd877ggey2y9ddk74
Passphrase: mnkp3i34hkd877ggey2y9ddk74
---


There are two solutions here:

* Clean up the login manager after every test that performs operations which might persist a login: login(), verifyLogin(), ...

* Clean up the login manager prior to running tests that expect a clean environment ("if you want a clean house, clean it yourself").

The latter simply requires the insertion of

    Weave.Svc.Login.removeAllLogins();

at the appropriate spot. The former is a lot more work, and susceptible to bitrot.

Unless I hear violent disagreement, the latter is the patch I'll be attaching shortly.

Comment 2

[Philipp von Weitershausen [:philikon]]

(In reply to comment #1)
> There are two solutions here:
> 
> * Clean up the login manager after every test that performs operations which
> might persist a login: login(), verifyLogin(), ...
> 
> * Clean up the login manager prior to running tests that expect a clean
> environment ("if you want a clean house, clean it yourself").
> 
> The latter simply requires the insertion of
> 
>     Weave.Svc.Login.removeAllLogins();
> 
> at the appropriate spot. The former is a lot more work, and susceptible to
> bitrot.

It is, though it's kind of nice to know and explicitly state (by means of a cleanup) which places actually end up persisting logins. It used to be more explicit, but with the changes to verifyLogin() that has changed now. It's not that I think this poses a big problem, just generally being cautious here...

> Unless I hear violent disagreement, the latter is the patch I'll be attaching
> shortly.

Make it so :)

Comment 3

[Richard Newman [:rnewman]]

Attachment: Wipe logins before running test.

Comment 4

[Richard Newman [:rnewman]]

Pushed:

http://hg.mozilla.org/services/fx-sync/rev/244e9d1b50d8

Thanks Philipp!