Closed Bug 1069687 Opened 10 years ago Closed 4 years ago

Robocop tests run against the default profile

Categories

(Firefox for Android Graveyard :: Testing, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: rnewman, Unassigned, Mentored)

References

Details

(Whiteboard: [has bitrotted patch])

Attachments

(1 file, 2 obsolete files)

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.
And in BaseTest#runTest: mProfile is /storage/sdcard0/tests/profile
This seems to be the smallest possible change for sanity: clear the profile's prefs prior to running the test. Testing now.
Attachment #8491921 - Flags: review?(mark.finkle)
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Now without spurious import.
Attachment #8491930 - Flags: review?(mark.finkle)
Attachment #8491921 - Attachment is obsolete: true
Attachment #8491921 - Flags: review?(mark.finkle)
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*
Attachment #8491930 - Flags: review?(mark.finkle)
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.
Attachment #8491948 - Flags: review?(mark.finkle)
Attachment #8491930 - Attachment is obsolete: true
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
Attachment #8491948 - Flags: review?(mark.finkle) → review+
It very probably does, but I'll fix the tests!
How are you launching Robocop? I wonder if something's up with that. 'cuz I'm pretty sure we're okay on instrumentation.
(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.
(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.
(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.
(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.
This'll take a few iterations of test fixes to step through, so expect tiny follow-up reviews.
Blocks: 1045053
This has newfound relevance for something I'm doing in Bug 1077590. Marking as blocking that, just 'cos See Also is pretty vague.
Blocks: 1077590
Assignee: rnewman → nobody
Mentor: gbrown, nalexander
Status: ASSIGNED → NEW
Whiteboard: [has bitrotted patch]
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.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: