1069687 - Robocop tests run against the default profile

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: Testing, defect)

Description

[Richard Newman [:rnewman]]

Testing current fx-team, so I don't know if this changed recently.

Instrumenting:

diff --git a/mobile/android/base/GeckoSharedPrefs.java b/mobile/android/base/GeckoSharedPrefs.java
--- a/mobile/android/base/GeckoSharedPrefs.java
+++ b/mobile/android/base/GeckoSharedPrefs.java
@@ -96,16 +96,17 @@ public final class GeckoSharedPrefs {
      * flag.
      */
     public static SharedPreferences forProfile(Context context, EnumSet<Flags> flags) {
         String profileName = GeckoProfile.get(context).getName();
         if (profileName == null) {
             throw new IllegalStateException("Could not get current profile name");
         }

+        Log.i("GeckoXXX", "Opening SharedPreferences: " + profileName);
         return forProfileName(context, profileName, flags);
     }

     public static SharedPreferences forProfileName(Context context, String profileName) {
         return forProfileName(context, profileName, EnumSet.noneOf(Flags.class));
     }

     /**

Yields the horror:

09-18 16:12:07.631 I/GeckoXXX(26421): Opening SharedPreferences: default

Similarly, instrumenting browser.js:

GeckoXXX: /storage/emulated/legacy/tests/profile


The profile directory is deleted between app runs, but *SharedPreferences is not*. That means that on developer machines, tests run with a config that's half fresh, half stale.

We should not do this.

Comment 1

[Richard Newman [:rnewman]]

And in BaseTest#runTest:

mProfile is /storage/sdcard0/tests/profile

Comment 2

[Richard Newman [:rnewman]]

Attachment: Clear per-profile preferences for the provided profile prior to Robocop test run. v1

This seems to be the smallest possible change for sanity: clear the profile's prefs prior to running the test.

Testing now.

Comment 3

[Richard Newman [:rnewman]]

Attachment: Clear per-profile preferences for the provided profile prior to Robocop test run. v2

Now without spurious import.

Comment 4

[Richard Newman [:rnewman]]

Comment on attachment 8491930 [details] [diff] [review]
Clear per-profile preferences for the provided profile prior to Robocop test run. v2

Actually, this needs a new version -- this uses a profile path, so *tableflip*

Comment 5

[Richard Newman [:rnewman]]

Attachment: Specify a unique profile name for each Robocop test run. v1

Here's a better approach.

By passing a profile name, we use a unique prefs file for each test run.

They don't get cleaned up, but that's not a problem on infra, and developers can always use Clear Data.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8491948 [details] [diff] [review]
Specify a unique profile name for each Robocop test run. v1

Let's hope this does not make TBPL unhappy

Comment 7

[Richard Newman [:rnewman]]

It very probably does, but I'll fix the tests!

Comment 8

[Nick Alexander :nalexander [he/him]]

How are you launching Robocop?  I wonder if something's up with that.  'cuz I'm pretty sure we're okay on instrumentation.

Comment 9

[Nick Alexander :nalexander [he/him]]

(In reply to Richard Newman [:rnewman] from comment #5)
> Created attachment 8491948 [details] [diff] [review]
> Specify a unique profile name for each Robocop test run. v1
> 
> Here's a better approach.
> 
> By passing a profile name, we use a unique prefs file for each test run.

I'm not confident that -p and -P interact well together.  Try run, please.

> They don't get cleaned up, but that's not a problem on infra, and developers
> can always use Clear Data.

That's not true -- we run out of inodes on the Pandas occasionally.

Comment 10

[Richard Newman [:rnewman]]

(In reply to Nick Alexander :nalexander from comment #8)
> How are you launching Robocop?  I wonder if something's up with that.  'cuz
> I'm pretty sure we're okay on instrumentation.

Mach. And the same is true for Eclipse, IIRC.

Comment 11

[Richard Newman [:rnewman]]

(In reply to Nick Alexander :nalexander from comment #9)

> That's not true -- we run out of inodes on the Pandas occasionally.

I was under the impression that we uninstall the app between test runs. That'll clean up these files.

Comment 12

[Nick Alexander :nalexander [he/him]]

(In reply to Richard Newman [:rnewman] from comment #11)
> (In reply to Nick Alexander :nalexander from comment #9)
> 
> > That's not true -- we run out of inodes on the Pandas occasionally.
> 
> I was under the impression that we uninstall the app between test runs.
> That'll clean up these files.

Ah, I reckon you're correct.

Comment 13

[Richard Newman [:rnewman]]

https://tbpl.mozilla.org/?tree=Try&rev=efb9c870ba44

Comment 14

[Richard Newman [:rnewman]]

This'll take a few iterations of test fixes to step through, so expect tiny follow-up reviews.

Comment 15

[Richard Newman [:rnewman]]

This has newfound relevance for something I'm doing in Bug 1077590. Marking as blocking that, just 'cos See Also is pretty vague.

Comment 16

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.