826492 - [mozdevice] DeviceManager.mkDirs() does not work if there is a directory with the same name as the file

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Mark Côté [:mcote]]

DeviceManager.mkDirs() is a bit strange; it takes a path but intends to create only the directory underneath the full path.  For example, mkDirs('/a/b/c') would only create '/a/b/'.  I guess the expectation is that the path is to a file, and we want to be creating only its directory.  Why this logic isn't the responsibility of the caller of mkDirs(), I don't know; regardless, there is a subtle bug.

If you call mkDirs() with a path that has at least one directory with the same name as the "file" (e.g. last) part of the path, only the path up to that part will be created.  mkDirs('/a/b/c/c') will result in only '/a/b/' being created, as would mkDirs('/a/b/foo/bar/foo').

Here's a log from a Python shell, displaying this bug (with a few extra print statements):

>>> from mozdevice.devicemanagerSUT import DeviceManagerSUT
>>> dm = DeviceManagerSUT('192.168.1.146')
>>> dm.listFiles('/mnt/sdcard/mkdirstest')
['test']
>>> dm.listFiles('/mnt/sdcard/mkdirstest/test')
[]
>>> dm.mkDirs('/mnt/sdcard/mkdirstest/test/foo/foo')
checking for /mnt/sdcard/mkdirstest/test/foo
parts: ['', 'mnt', 'sdcard', 'mkdirstest', 'test', 'foo', 'foo']
mkDir /mnt
mkDir /mnt/sdcard
mkDir /mnt/sdcard/mkdirstest
mkDir /mnt/sdcard/mkdirstest/test
>>>

The reason is pretty simple:  we check if each part is *equal to* the last part.  We should be checking for identity, not equality.

Comment 1

[Mark Côté [:mcote]]

Forgot the last line of the log, indicating that the 'foo' subdirectory was not created:

>>> dm.listFiles('/mnt/sdcard/mkdirstest/test')
[]

Comment 2

[Mark Côté [:mcote]]

Attachment: Check for identity instead of equality when checking path parts in mkDirs()

Simple fix.

Comment 3

[William Lachance (:wlach)]

Comment on attachment 697683 [details] [diff] [review]
Check for identity instead of equality when checking path parts in mkDirs()

Could you do a unit test for this too?

Comment 4

[Mark Côté [:mcote]]

Attachment: Check for identity instead of equality when checking path parts in mkDirs()

Good idea for a test.  I squashed it into my last patch.  I also cleaned up coding style; I may do the other test files too, for consistency.

The test fails (well, hangs) without the accompanying devicemanager.py change, and passes with it, as expected.

Comment 5

[William Lachance (:wlach)]

Comment on attachment 698884 [details] [diff] [review]
Check for identity instead of equality when checking path parts in mkDirs()

Great, thanks!

Comment 6

[Mark Côté [:mcote]]

https://github.com/mozilla/mozbase/commit/5a9e6f5a36631fa3d110aea2bc376e9adea39311