Closed Bug 1036982 Opened 5 years ago Closed 5 years ago

B2G runners need some additional profile setup for emulators/devices

Categories

(Testing :: Mozbase, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla33

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(1 file, 2 obsolete files)

B2G devices don't read everything from the profile like firefox/b2g desktop. For example webapps are read from /data/local/webapps, settings are read from /system/b2g/defaults/settings.json and extensions are read from /system/b2g/distribution/bundles.

Since mozrunner is responsible for "starting gecko with a profile" it makes sense for the B2G runners to detect if these things exist in the profile and move them to the proper b2g specific locations.
This patch worked for me locally, but I won't ask for review just yet because I made some changes that need to be tested.
This patch works, though unfortunately it's kind of ugly. I'm flagging you jgriffin since you are familiar with some of the b2g idiosyncrasies related to the profile.. feel free to redirect though. It does two main things:

1) Make sure settings.db is updated with any user-specified settings.json files found in the profile.

2) Make sure /data/local/webapps is replaced by the 'webapps' directory in the profile, if it exists.

Here's a try run along with some other mozrunner patches:
https://tbpl.mozilla.org/?tree=Try&rev=f72159819a50
Attachment #8453819 - Attachment is obsolete: true
Attachment #8455528 - Flags: review?(jgriffin)
Comment on attachment 8455528 [details] [diff] [review]
Do additional b2g specific profile setup/cleanup

Oops, looks like this busts mochitests. Probably the overwriting /data/local/webapps thing.
Attachment #8455528 - Flags: review?(jgriffin)
I fixed the mochitest problem. There's a Mnw intermittent orange which the sheriffs said they haven't seen before. I'm not quite sure how it would be related to this patch, but I guess it's possible. I re-triggered a few times to get a better idea.

https://tbpl.mozilla.org/?tree=Try&rev=3f0aa29095e7&showall=1
Attachment #8455528 - Attachment is obsolete: true
Comment on attachment 8456425 [details] [diff] [review]
Do additional b2g specific profile setup/cleanup

I'm not really sure what to make of that first Mnw orange. I can't find any other instances of it anywhere else. So either I got unlucky and hit a rare intermittent, or somehow my patch is causing it (I still think this is unlikely, but possible).

I'll ask for review for now anyway.
Attachment #8456425 - Flags: review?(jgriffin)
Comment on attachment 8456425 [details] [diff] [review]
Do additional b2g specific profile setup/cleanup

Review of attachment 8456425 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/mozbase/mozrunner/mozrunner/application.py
@@ +150,5 @@
> +
> +        # On devices, the webapps are located in /data/local/webapps instead of the profile.
> +        # In some cases we may need to replace the existing webapps, in others we may just
> +        # need to leave them in the profile. If the system app is present in the profile
> +        # webapps, it's a good indication that they should replace the existing ones wholesale.

What happens if we don't do this, and just leave the webapps in the profile?  Are they actually used at all?

@@ +166,5 @@
> +                if self.dm.fileExists(path):
> +                    self.dm.removeFile(path)
> +            self.dm.pushDir(extension_dir, self.remote_bundles_dir)
> +
> +    def cleanup_profile(self):

Do we need to restore the original webapps here too, if we overwrote them?
Attachment #8456425 - Flags: review?(jgriffin) → review+
(In reply to Jonathan Griffin (:jgriffin) from comment #6)
> ::: testing/mozbase/mozrunner/mozrunner/application.py
> @@ +150,5 @@
> > +
> > +        # On devices, the webapps are located in /data/local/webapps instead of the profile.
> > +        # In some cases we may need to replace the existing webapps, in others we may just
> > +        # need to leave them in the profile. If the system app is present in the profile
> > +        # webapps, it's a good indication that they should replace the existing ones wholesale.
> 
> What happens if we don't do this, and just leave the webapps in the profile?
> Are they actually used at all?

No, they don't get used at all. These generated profiles are meant to work with b2g desktop (which doesn't have a /data/local). I think because the webapps already exist in /data/local the ones in the profile get ignored.

I had to set it up this way because mochitests also create a 'webapps' folder in the profile (based on http://mxr.mozilla.org/mozilla-central/source/testing/profiles/webapps_mochitest.json), except this time we *have* to leave them in the profile. I also tried merging the webapps in the profile with the ones in /data/local, but this didn't work because the appId's all collided with each other. It might be possible to write some code that merges them properly by combining both webapps.json files and re-registering all the apps.. but that is a pretty large project all on its own. I can file a bug to tackle this in the future though if you think it's important.

> @@ +166,5 @@
> > +                if self.dm.fileExists(path):
> > +                    self.dm.removeFile(path)
> > +            self.dm.pushDir(extension_dir, self.remote_bundles_dir)
> > +
> > +    def cleanup_profile(self):
> 
> Do we need to restore the original webapps here too, if we overwrote them?

It's not very obvious, but they actually will get restored by the runner because they got added to self.backup_files. Maybe instead of that, I should just make them get explicitly backed up here instead.
Thanks for the explanations wrt the webapps.  I'd probably punt on adding code to merge the webapps until we actually need it.
https://hg.mozilla.org/mozilla-central/rev/162358e77cc0
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.