Closed
Bug 1285663
Opened 8 years ago
Closed 8 years ago
During Android tests, chmod less and/or more efficiently to save time
Categories
(Firefox for Android Graveyard :: Testing, defect, P2)
Tracking
(firefox50 fixed)
RESOLVED
FIXED
Firefox 50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: gbrown, Assigned: gbrown)
Details
Attachments
(1 file, 1 obsolete file)
3.53 KB,
patch
|
bc
:
review+
|
Details | Diff | Splinter Review |
During a mochitest run, the harness uses devicemanager to chmod all the files in the profile, multiple times. That seems to take a lot of time. There are several inefficiencies: - the profile is pushed multiple times - chmod is executed on every file individually rather than using wildcards or -R - chmod seems not to do anything on /sdcard, at least on the emulator We've done it this way for good reasons, but maybe it can be improved -- worth a look.
Assignee | ||
Updated•8 years ago
|
Priority: -- → P2
Assignee | ||
Comment 1•8 years ago
|
||
Assignee | ||
Comment 2•8 years ago
|
||
As far as I can tell, chmod on sdcard never does anything; skipping it on sdcard speeds up profile setup significantly. I also remove an extra profile push from mochitests. This profile was always over-written before running any tests -- completely extraneous. These two optimizations are enough for me: Running 'mach mochitest' locally against an emulator, there's no longer any significant pause without good cause. https://treeherder.mozilla.org/#/jobs?repo=try&revision=82bf0e9c29cb
Attachment #8770648 -
Attachment is obsolete: true
Attachment #8770774 -
Flags: review?(bob)
Comment 3•8 years ago
|
||
Comment on attachment 8770774 [details] [diff] [review] eliminate extra profile push; do not chmod /sdcard Review of attachment 8770774 [details] [diff] [review]: ----------------------------------------------------------------- r+ with a solution to the removeDir question. ::: testing/mochitest/runtestsremote.py @@ -189,5 @@ > - self._dm.chmodDir(self.remoteProfile) > - except mozdevice.DMError: > - self.log.error( > - "Automation Error: Unable to copy profile to device.") > - raise The profile push is handled in buildURLOptions, but it does not remove the existing profile dir. Should we also add the removeDir to buildURLOptions? ::: testing/mozbase/mozdevice/mozdevice/devicemanagerADB.py @@ +651,5 @@ > raise DMError("Timeout exceeded for _checkCmd call after %d retries." % retries) > > def chmodDir(self, remoteDir, mask="777"): > if (self.dirExists(remoteDir)): > + if '/sdcard' in remoteDir: At first I worried that this wouldn't catch all instances of paths on the external storage, but I can't find an instance where a path that we would actually use wouldn't contain '/sdcard'. Can you think of any? If we did have a path to external storage that did not include '/sdcard', we could determine if it was on external storage by looking at the owner of a file or directory, but I don't think this is actually necessary. Thoughts?
Attachment #8770774 -
Flags: review?(bob) → review+
Assignee | ||
Comment 4•8 years ago
|
||
(In reply to Bob Clary [:bc:] from comment #3) > The profile push is handled in buildURLOptions, but it does not remove the > existing profile dir. Should we also add the removeDir to buildURLOptions? We don't need to explicitly removeDir because now pushDir does a removeDir before pushing. > At first I worried that this wouldn't catch all instances of paths on the > external storage, but I can't find an instance where a path that we would > actually use wouldn't contain '/sdcard'. Can you think of any? No. I think /mnt/sdcard, /sdcard, /storage/sdcard are common. > If we did have a path to external storage that did not include '/sdcard', we > could determine if it was on external storage by looking at the owner of a > file or directory, but I don't think this is actually necessary. Thoughts? If we miss a case or two, it's not so bad -- it just won't be as efficient on those devices. I'm happy if I can speed up the 90%.
Pushed by gbrown@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/dd48d5640b5c Optimize profile pushes and do not chmod files on /sdcard; r=bc
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/dd48d5640b5c
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•