732768 - Eliminate shared state and concurrency problems in tests

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Android Sync, defect, P2)

Description

[Richard Newman [:rnewman]]

I was having some annoyances with tests that led me through a rats nest of problems.

I fixed many of them.

Improvements:

* We now shut down our worker queues in finish. This exposed a whole raft of bugs; we can't, for example, delegate the failure callbacks for when the queues are shut down!
* We now throw exceptions if our state is incorrect to handle a method. That means you get a synchronous exception if you try to finish twice, or begin twice, or begin after finish.
* There was no synchronization around a single shared session slot. Enormous possibility for error. Now sessions are passed around, and there is no global RepositorySession in tests; the only remaining global test helper is the wait helper itself.
* Callbacks are now deferred correctly in more places.
* Some methods that should notify -- e.g., finish! -- did not. They're now Runnables.
* I lifted the shared middleware stuff out of Crypto5MiddlewareRepository.
* Session state wasn't safe. Now it is, and operations such as "run this runnable if we're active and our queue isn't shut down" are now atomic.
* Items could be left in the WaitHelper queue between tests. I added a sanity method to assert if that's the case. It caught bugs.

Sorry that this is going to be an ugly pull request, but this was all very tangled, and I eventually gave up trying to produce incremental commits.

Comment 1

[Richard Newman [:rnewman]]

https://github.com/mozilla-services/android-sync/pull/97

Comment 2

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9d737f060432

Comment 3

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/9d737f060432