Closed
Bug 704618
Opened 10 years ago
Closed 10 years ago
devicemanagerSUT pushFile does not accept a directory as a destination
Categories
(Testing :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla11
People
(Reporter: gbrown, Assigned: gbrown)
Details
Attachments
(1 file, 1 obsolete file)
1.95 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
devicemanagerADB allows both: dm.pushFile("myfile", "/mnt/sdcard/tests/myfile") and dm.pushFile("myfile", "/mnt/sdcard/tests") The calls have the same result: myfile is pushed to /mnt/sdcard/tests/myfile. devicemanagerSUT only allows: dm.pushFile("myfile", "/mnt/sdcard/tests/myfile") If a directory is specified as the destination, the SUT version of pushFile fails.
![]() |
Assignee | |
Comment 1•10 years ago
|
||
This difference between ADB and SUT is one of the reasons why "make xpcshell-tests-remote" fails with SUT (but not with ADB). I don't know of any other tests that use pushFile like this, so another approach to resolving this issue is to change the remote xpcshell tests.
![]() |
Assignee | |
Comment 2•10 years ago
|
||
Comment on attachment 576298 [details] [diff] [review] patch to allow pushFile(local-file, remote-directory) ># HG changeset patch ># Parent c2aefd373d84af026da44bdf19195fed6c33afae ># User Geoff Brown <gbrown@mozilla.com> >Bug 704618: allow directory as a destination argument to SUT pushFile(); r=jmaher > >diff --git a/build/mobile/devicemanagerSUT.py b/build/mobile/devicemanagerSUT.py >--- a/build/mobile/devicemanagerSUT.py >+++ b/build/mobile/devicemanagerSUT.py >@@ -261,16 +261,18 @@ class DeviceManagerSUT(DeviceManager): > # returns: > # success: True > # failure: False > def pushFile(self, localname, destname): > if (os.name == "nt"): > destname = destname.replace('\\', '/') > > if (self.debug >= 3): print "in push file with: " + localname + ", and: " + destname >+ if (self.isDir(destname)): >+ destname = os.path.join(destname, os.path.basename(localname)) This will fail on windows, you need to force it to use '/' and create the path that way: destname = destname + '/' + os.path.basename(localname) Pretty cool if that's all we have to do. Did you test this? It's so simple, and for some reason I had this idea this would be hard. Thanks, flag Joel for followup review, I'm going to be out tomorrow.
![]() |
Assignee | |
Comment 4•10 years ago
|
||
The patch appears to cause trouble for mochitest-remote -- I'll look into it.
![]() |
Assignee | |
Comment 5•10 years ago
|
||
I found I had to use dirExists instead of isDir (isDir throws an exception when the path does not exist). I also corrected the issue identified by ctalbert -- thanks! mochitest-remote passes now. The change to pushDir is not strictly necessary for this bug, but corrects a problem I noted while debugging: Some paths passed to pushFile had double path-separators: /mnt/sdcard/tests/mochitest//some.file
Attachment #576298 -
Attachment is obsolete: true
Attachment #576775 -
Flags: review?(jmaher)
Comment 6•10 years ago
|
||
Comment on attachment 576775 [details] [diff] [review] patch to allow pushFile(local-file, remote-directory) Review of attachment 576775 [details] [diff] [review]: ----------------------------------------------------------------- r=me. please run through try server for android-xul tests which use sut agent.
Attachment #576775 -
Flags: review?(jmaher) → review+
![]() |
Assignee | |
Comment 7•10 years ago
|
||
Passes on try - all android-xul tests are green: https://tbpl.mozilla.org/?tree=Try&rev=5ecce5c9fc42
Keywords: checkin-needed
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/26bac72ef060
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•