1306140 - Add tests to ensure there is always one default homepanel after HomePanel migrations

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect, P1)

Description

[Andrzej Hunt :ahunt]

HomePanelConfig migrations that resulted in no default panel being set have bitten us twice:
 - Bug 1304777
 - Bug 1264136

We should add some tests that ideally walk through every permutation, to ensure that we never end up without a default homepanel being set.

Comment 1

[Andrzej Hunt :ahunt]

Attachment: Bug 1306140 - Pre: move HomeConfig migration loop into separate method to help with testing

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

Comment 2

[Andrzej Hunt :ahunt]

Attachment: Bug 1306140 - Pre: make HomeConfig VERSION package-private to allow access for testing

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

Comment 3

[Andrzej Hunt :ahunt]

Attachment: Bug 1306140 - Add tests for HomeConfig migration

I am open to replacing "constellations" with a different name, but it was the best
I could come up with so far.

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

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8796268 [details]
Bug 1306140 - Pre: move HomeConfig migration loop into separate method to help with testing

https://reviewboard.mozilla.org/r/82174/#review80754

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8796269 [details]
Bug 1306140 - Pre: make HomeConfig VERSION package-private to allow access for testing

https://reviewboard.mozilla.org/r/82176/#review80756

::: mobile/android/base/java/org/mozilla/gecko/home/HomeConfigPrefsBackend.java:39
(Diff revision 1)
>  
>  public class HomeConfigPrefsBackend implements HomeConfigBackend {
>      private static final String LOGTAG = "GeckoHomeConfigBackend";
>  
>      // Increment this to trigger a migration.
> -    private static final int VERSION = 8;
> +    /* package-private */ static final int VERSION = 8;

If this is just for testing then you could add @VisibleForTesting instead of the comment here.

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8796270 [details]
Bug 1306140 - Add tests for HomeConfig migration

https://reviewboard.mozilla.org/r/82178/#review80766

Comment 7

[Andrzej Hunt :ahunt]

Comment on attachment 8796269 [details]
Bug 1306140 - Pre: make HomeConfig VERSION package-private to allow access for testing

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82176/diff/1-2/

Comment 8

[Andrzej Hunt :ahunt]

Comment on attachment 8796270 [details]
Bug 1306140 - Add tests for HomeConfig migration

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82178/diff/1-2/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/39b96ac89699
Pre: move HomeConfig migration loop into separate method to help with testing r=sebastian
https://hg.mozilla.org/integration/autoland/rev/aeccc63b252a
Pre: make HomeConfig VERSION package-private to allow access for testing r=sebastian
https://hg.mozilla.org/integration/autoland/rev/cdf154426881
Add tests for HomeConfig migration r=sebastian

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/39b96ac89699
https://hg.mozilla.org/mozilla-central/rev/aeccc63b252a
https://hg.mozilla.org/mozilla-central/rev/cdf154426881