902560 - All FakeProfileTestCase tests fail in setUp() if fake profile dir is not removed

Status: RESOLVED FIXED

(Android Background Services Graveyard :: Build & Test, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

FakeProfileTestCase attempts to create a new fake profile directory in FakeProfileTestCase.setUp(). If mkdir() fails on this directory (e.g. the directory already exists), setUp() will throw an IllegalStateException. A call to tearDown() should delete the fake profile directory and alleviate this but if tearDown() is not called (e.g. process crash), all future tests will fail in setUp() on mkdir().

This is especially pertinent because tests that fail in setUp() don't get to call tearDown() and so this fake profile dir will exist even after the current integration test suite has finished running. Thus ALL future test runs will fail, even in subsequent runs, until this directory is deleted.

This could cause horrors with automated testing rigs so if we want to move these tests to tbpl, we should fix this (bug 709353).

I came across this by overriding tearDown and not calling super.tearDown() from a subclass, which is another way to come across this bug.

Comment 1

[Richard Newman [:rnewman]]

Should (a) use a computed random dir for this, and (b) blow it away in setUp if it exists, no?

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Sounds ideal to me.

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

We should use the changes in bug 870925 that add the FakeProfileTestCase.getCacheSuffix implementation.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

r? - https://github.com/mozilla-services/android-sync/pull/364

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

> We should use the changes in bug 870925 that add the FakeProfileTestCase.getCacheSuffix implementation.

This can land, and bug 870925 will add the randomization rnewman mentioned in comment 1 (a).

Comment 6

[Nick Alexander :nalexander [he/him]]

lgtm with naming nit (on github).

Comment 7

[Nick Alexander :nalexander [he/him]]

Don't forget that tests/ stuff lands on m-c now.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: 01: Patch

r+ via gh.

https://hg.mozilla.org/integration/fx-team/rev/f576e5ae6ef7

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

The randomness mentioned in comment 1 is helpful but not necessary to fix this bug.

This randomness will land in bug 870925.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f576e5ae6ef7