Merge lp://staging/~ctf/checkbox/bug944623_usb-storage-preinserted_0.9 into lp://staging/checkbox

Please follow PEP 8 in the first instance:

def GetDiskInfo(self, device):

should be

def get_disk_info(self, device):

and so on...

update revision:1292, better naming, and '-l' flag to umount to deal the devices fails on umount

Looks good to me... I've tested this on 10.04.4 and Oneiric and it seems to work well there...

A couple points though:

the job usb/storage-preinserted contains the following:

plugin: shell
name: usb/storage-preinserted
requires: package.name == 'linux'
command: removable_storage_test -l usb && removable_storage_test usb

What's the point of this now? removable_storage_test -l usb will always pass, regardless of whether there's a USB device plugged in or not:

bladernr@ubuntu:~$ ./removable_storage_test -l usb; echo $?
removable devices currently mounted:
None
removable devices currently not mounted:
None
0
bladernr@ubuntu:~$ ./removable_storage_test -l usb; echo $?
removable devices currently mounted:
None
removable devices currently not mounted:
/dev/sdb1
0

Additionally, I'm hoping that returning it back to that original behaviour in this case will also cause the test to fail if it tries running the actual wrtie/read test with no USB devs present. Currently, the test portion itself will ALSO successfully run and exit with a PASS, regardless of the presence of USB devices:

bladernr@ubuntu:~$ sudo ./removable_storage_test usb; echo $?
Found the following mounted usb partitions:
Running usb file transfer test for 1 iterations
Creating Temp Data file
File name is :/tmp/tmpnG3s4u
File size is 1048576 bytes
Parent hash is: 2c5628acef5019147c6a704663d81e53
Successfully completed 1 usb file transfer test iterations
0

^^^ That was after removing all USB devices... this, like -l, should trigger a failure here, rather than a blanket pass.

So I think this needs to be tweaked to generate a fail if no USB devices are found at all.

My thinking is:

USB present and mounted: list should PASS, test should PASS if sums match
USB present and NOT mounted: list should PASS, test should PASS IF mount is successful AND sums match
USB not present: list should FAIL, test should FAIL

Finally, the test should ALSO generate FAIL and error messages if the mount() and unmount() calls are unsuccessful. They look to spit out an error message, but don't return a FAIL code, so even though the action fails, the script could still pass. I would also be happier if both the mount() and unmount() calls generated some sort of status output as well... just so we know what's happening. For example:

bladernr@ubuntu:~$ sudo ./removable_storage_test usb
Found the following mounted usb partitions:
/dev/sdb1 : /tmp/tmpCUH4Ae
Running usb file transfer test for 1 iterations
Creating Temp Data file
File name is :/tmp/tmpW989Go
File size is 1048576 bytes
Parent hash is: 232cda95d615ac2cdef86717bc7552b7
Copying /tmp/tmpW989Go to /tmp/tmpCUH4Ae
Hashing copy on /tmp/tmpCUH4Ae
Hash of /tmp/tmpCUH4Ae/tmpW989Go on /tmp/tmpCUH4Ae is 232cda95d615ac2cdef86717bc7552b7
Successfully completed 1 usb file transfer test iterations

I would prefer to see a "Now mounting $DEVICE" and "Now unmounting $DEVICE" in the output so we know visually that both operations were successful.

Also, re PEP 8, that's my bad... I wrote the original, so that's all on me (and you guys who passed it without complaint) heh... thank you Tim for correcting the function call names.

Hi Jeff,
Thanks for the feedback, I forgot to use len() to get the count of the removable devices, so it passed while event no removable devices available.

But there's some known bug, I still need to fix it. I will continue it next week.

known bug
--------------------
removable devices currently mounted:
None
removable devices currently not mounted:
/dev/sdb1
/dev/sdc1
--------------------
Error: can't mount /dev/sdb1
Error: can't mount /dev/sdc1
Traceback (most recent call last):
  File "./removable_storage_test", line 276, in <module>
    sys.exit(main())
  File "./removable_storage_test", line 227, in main
    error += 1
UnboundLocalError: local variable 'error' referenced before assignment

yeah, that's just a trivial error...

you call the variable errors at line 198 and reference it as error at line 227. All the other references to 'errors' look correct though.

Jeff,
I did the test as you did as below.
>>>>>>>>>>>> no storage inserted
$ ./removable_storage_test -l usb; echo $?
No removable drives were detected, aborting
1

>>>>>>>>>>>>>>>>>>> one storage inserted, not mounted by system
$ ./removable_storage_test -l usb; echo $?
--------------------
removable devices currently mounted:
None
removable devices currently not mounted:
/dev/sdb1
--------------------
0

>>>>>>>>>>>>>>>>>>> one storage inserted, mounted by system(desktop)
$ ./removable_storage_test -l usb; echo $?
--------------------
removable devices currently mounted:
/dev/sdb1 : /media/18a863d8-cfd5-4360-bfda-03896f870d9d
removable devices currently not mounted:
None
--------------------
0

>>>>>>>>>>>>>>>>>>> no storage inserted.
$ sudo ./removable_storage_test usb; echo $?
No removable drives were detected, aborting
1

>>>>>>>>>>>>>>>>>>> one storage inserted, not mounted by system
$ sudo ./removable_storage_test usb; echo $?
Now mounting /dev/sdb1 on /tmp/tmpzPTne6
Found the following mounted usb partitions:
/dev/sdb1 : /tmp/tmpzPTne6
--------------------
Running usb file transfer test for 1 iterations
Creating Temp Data file
File name is :/tmp/tmpNMKWxg
File size is 1048576 bytes
Parent hash is: 32325ce1f4090a54f9164c285eb3218b
Copying /tmp/tmpNMKWxg to /tmp/tmpzPTne6
Hashing copy on /tmp/tmpzPTne6
Hash of /tmp/tmpzPTne6/tmpNMKWxg on /tmp/tmpzPTne6 is 32325ce1f4090a54f9164c285eb3218b
Now umounting /dev/sdb1 on /tmp/tmpzPTne6
Successfully completed 1 usb file transfer test iterations
0

>>>>>>>>>>>>>>>>>>> one storage inserted mounted by system, another one not mounted by system
$ sudo ./removable_storage_test usb; echo $?
Now mounting /dev/sdc1 on /tmp/tmpWMEjfC
Found the following mounted usb partitions:
/dev/sdb1 : /media/18a863d8-cfd5-4360-bfda-03896f870d9d
/dev/sdc1 : /tmp/tmpWMEjfC
--------------------
Running usb file transfer test for 1 iterations
Creating Temp Data file
File name is :/tmp/tmpIpQU3r
File size is 1048576 bytes
Parent hash is: 75f852c051c370b6b1b4f0560c2e6e5f
Copying /tmp/tmpIpQU3r to /media/18a863d8-cfd5-4360-bfda-03896f870d9d
Hashing copy on /media/18a863d8-cfd5-4360-bfda-03896f870d9d
Hash of /media/18a863d8-cfd5-4360-bfda-03896f870d9d/tmpIpQU3r on /media/18a863d8-cfd5-4360-bfda-03896f870d9d is 75f852c051c370b6b1b4f0560c2e6e5f
Copying /tmp/tmpIpQU3r to /tmp/tmpWMEjfC
Hashing copy on /tmp/tmpWMEjfC
Hash of /tmp/tmpWMEjfC/tmpIpQU3r on /tmp/tmpWMEjfC is 75f852c051c370b6b1b4f0560c2e6e5f
Now umounting /dev/sdc1 on /tmp/tmpWMEjfC
Successfully completed 1 usb file transfer test iterations
0

Awesome. Looks good! Approve.