Closed Bug 704618 Opened 8 years ago Closed 8 years ago

devicemanagerSUT pushFile does not accept a directory as a destination

Categories

(Testing :: General, defect)

ARM
Android
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla11

People

(Reporter: gbrown, Assigned: gbrown)

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
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.
The patch appears to cause trouble for mochitest-remote -- I'll look into it.
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 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+
Passes on try - all android-xul tests are green:

https://tbpl.mozilla.org/?tree=Try&rev=5ecce5c9fc42
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/26bac72ef060
Status: NEW → ASSIGNED
Keywords: checkin-needed
Hardware: x86 → ARM
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/26bac72ef060
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.