704618 - devicemanagerSUT pushFile does not accept a directory as a destination

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Geoff Brown [:gbrown]]

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.

Comment 1

[Geoff Brown [:gbrown]]

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 2

[Geoff Brown [:gbrown]]

Attachment: patch to allow pushFile(local-file, remote-directory)

Comment 3

[cmtalbert]

Comment on attachment 576298 [details] [diff] [review]
patch to allow pushFile(local-file, remote-directory)

># HG changeset patch
># Parent c2aefd373d84af026da44bdf19195fed6c33afae
># User Geoff Brown <[e-mail]>
>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.

Comment 4

[Geoff Brown [:gbrown]]

The patch appears to cause trouble for mochitest-remote -- I'll look into it.

Comment 5

[Geoff Brown [:gbrown]]

Attachment: patch to allow pushFile(local-file, remote-directory)

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

Comment 6

[Joel Maher ( :jmaher ) (UTC-4) (on leave, back October 1st)]

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.

Comment 7

[Geoff Brown [:gbrown]]

Passes on try - all android-xul tests are green:

https://tbpl.mozilla.org/?tree=Try&rev=5ecce5c9fc42

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/26bac72ef060

Comment 9

[Marco Bonardo [::mak]]

https://hg.mozilla.org/mozilla-central/rev/26bac72ef060