Merge lp:~bladernr/checkbox/1153894-clarify-mediacard-instructions into lp:checkbox

[Jeff Lane ]

Clarifies the SD/SDHC mediacard instructions to specify UNLOCKED cards. At least one user apparently tried running storage tests against a locked SD card causing an obvious failure.

[Jeff Lane  (bladernr)]

On further contemplation, I also took the liberty of adding better error handling to trap the OSError that is thrown when users try this test on locked SD cards and provide better output than a traceback. I also added some error handling to take care of the math that throws a ZeroDivisionError when we're unable to write data to the devices.

New output on a failed device looks like this:
bladernr@klaatu:~/development/git-workspace/checkbox$ ./scripts/removable_storage_test --memorycard sdio
Found the following mounted sdio partitions:
    /dev/mmcblk0p1 : /media/bladernr/5747-AD2E : None bits/s
--------------------
/dev/mmcblk0p1 (Total Data Size / iteration: 1.0005 MB):
ERROR:root:Unable to open /media/bladernr/5747-AD2E/tmp3_qmnf.0 for writing.
ERROR:root: [Errno 30] Read-only file system: '/media/bladernr/5747-AD2E/tmp3_qmnf.0'
ERROR:root:Failed to copy /tmp/tmp3_qmnf to /media/bladernr/5747-AD2E/tmp3_qmnf.0
ERROR:root:Unable to remove tempfile /media/bladernr/5747-AD2E/tmp3_qmnf.0
ERROR:root: [Errno 30] Read-only file system: '/media/bladernr/5747-AD2E/tmp3_qmnf.0'
 [Iteration 0] Average Speed: 0.0000
 Summary:
  Total Data Written: 1.0005 MB
  Total Time to write: 0.0000 secs
  Average Write Time: 0.0000 secs
  Average Write Speed: 0.0000 MB/s
WARNING:root:Completed 1 test iterations, but there were errors

[Zygmunt Krynicki (zyga)]

   Total Data Written: 1.0005 MB

So we managed to write one megabyte to a read-only filesystem? neat!

107 outfile.write(self.data)

You probably want to see how much data was actually written and count that instead

review: Needs Fixing

[lp:~bladernr/checkbox/1153894-clarify-mediacard-instructions]

2099. By Jeff Lane  on 2013-05-03

"[r=zkrynicki][bug=1103647][author=bladernr] automatic merge by tarmac"

2100. By Brendan Donegan on 2013-05-03

"[r=zkrynicki][bug=1093718][author=brendan-donegan] automatic merge by tarmac"

2101. By Jeff Lane  on 2013-05-03

"[r=brendan-donegan][bug=1100594][author=bladernr] automatic merge by tarmac"

2102. By Jeff Lane  on 2013-05-03

"[r=zkrynicki][bug=1128017][author=bladernr] automatic merge by tarmac"

2103. By Jeff Lane  on 2013-05-03

"[r=zkrynicki][bug=1116681][author=bladernr] automatic merge by tarmac"

[Jeff Lane  (bladernr)]

this stupid commit message stuff in git is confusing... I apparently squashed the wrong thing but the messages DO reflect the changes done.

Anyway, fixed the output, had to redo the whole branch because I messed it all up trying to rebase on the latest trunk which caused git to delete all my changes :( I finally just gave up and redid it from scratch.

review: Needs Resubmitting

[Jeff Lane  (bladernr)]

Made some changes based on Zygmunt's comments in IRC

[lp:~bladernr/checkbox/1153894-clarify-mediacard-instructions]

2104. By Jeff Lane  on 2013-05-06

scripts: removable_storage_test: Resolved Tracebacks

removable_storage_test no longer tracebacks on locked SD/SDHC cards and
provides better output on OSError. Also handles ZeroDivisionErrors that
could occur in the summary when no data is written during testing.

2105. By Jeff Lane  on 2013-05-06

jobs: mediacard.txt.in: SD/SDHC test instructions now specify UNLOCKED media cards

[Zygmunt Krynicki (zyga)]

+1

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2013-05-06 19:19:21 +0000
+++ debian/changelog	2013-05-06 21:48:25 +0000
@@ -15,6 +15,11 @@
     using and set video type and plugin accordingly. (LP: #1100594)
   * scripts/network_check: added ability to specify custom target URL for
     debugging failures (LP: #1128017)
+  * scripts/removable_storage_test: Added error handling to trap OSError on
+    non-writable media and modified output to handle subsequent
+    ZeroDivisionError issues when summarizing test results.
+    jobs/media.txt.in: Modified instructions for SD/SDHC to specify using
+    UNLOCKED cards to avoid issues when testing read-only media (LP: #1153894)
 
   [ Sylvain Pineau ]
   * jobs/suspend.txt.in, scripts/gpu_test: Remove the need of running the script

=== modified file 'jobs/mediacard.txt.in'
--- jobs/mediacard.txt.in	2013-05-01 16:25:38 +0000
+++ jobs/mediacard.txt.in	2013-05-06 21:48:25 +0000
@@ -85,9 +85,9 @@
 _description:
  PURPOSE:
      This test will check that the systems media card reader can
-     detect the insertion of a Secure Digital (SD) media card
+     detect the insertion of an UNLOCKED Secure Digital (SD) media card
  STEPS:
-     1. Click "Test" and insert an SD card into the reader.
+     1. Click "Test" and insert an UNLOCKED SD card into the reader.
         If a file browser opens up, you can safely close it.
         (Note: this test will time-out after 20 seconds.)
      2. Do not remove the device after this test.
@@ -126,9 +126,10 @@
 _description:
  PURPOSE:
      This test will check that the systems media card reader can
-     detect the insertion of an SD card after the system has been suspended
+     detect the insertion of an UNLOCKED SD card after the system
+     has been suspended
  STEPS:
-     1. Click "Test" and insert an SD card into the reader.
+     1. Click "Test" and insert an UNLOCKED SD card into the reader.
         If a file browser opens up, you can safely close it.
         (Note: this test will time-out after 20 seconds.)
      2. Do not remove the device after this test.
@@ -152,7 +153,8 @@
 _description:
  PURPOSE:
      This test will check that the system correctly detects 
-     the removal of an SD card from the systems card reader after the system has been suspended.
+     the removal of an SD card from the systems card reader 
+     after the system has been suspended.
  STEPS:
      1. Click "Test" and remove the SD card from the reader.
         (Note: this test will time-out after 20 seconds.)
@@ -176,9 +178,10 @@
 _description:
  PURPOSE:
      This test will check that the systems media card reader can
-     detect the insertion of a Secure Digital High-Capacity (SDHC) media card
+     detect the insertion of a UNLOCKED Secure Digital High-Capacity 
+     (SDHC) media card
  STEPS:
-     1. Click "Test" and insert an SDHC card into the reader.
+     1. Click "Test" and insert an UNLOCKED SDHC card into the reader.
         If a file browser opens up, you can safely close it.
         (Note: this test will time-out after 20 seconds.)
      2. Do not remove the device after this test.
@@ -217,9 +220,10 @@
 _description:
  PURPOSE:
      This test will check that the systems media card reader can
-     detect the insertion of an SDHC media card after the system has been suspended
+     detect the insertion of an UNLOCKED SDHC media card after the 
+     system has been suspended
  STEPS:
-     1. Click "Test" and insert an SDHC card into the reader.
+     1. Click "Test" and insert an UNLOCKED SDHC card into the reader.
         If a file browser opens up, you can safely close it.
         (Note: this test will time-out after 20 seconds.)
      2. Do not remove the device after this test.

=== modified file 'scripts/removable_storage_test'
--- scripts/removable_storage_test	2012-10-09 11:34:04 +0000
+++ scripts/removable_storage_test	2013-05-06 21:48:25 +0000
@@ -115,7 +115,13 @@
                 return True
 
     def write_file(self, data, dest):
-        with open(dest, 'wb', 0) as outfile:
+        try:
+            outfile = open(dest, 'wb', 0)
+        except OSError as exc:
+            logging.error("Unable to open %s for writing.", dest)
+            logging.error("  %s", exc)
+            return False
+        with outfile:
             try:
                 outfile.write(self.data)
             except IOError as exc:
@@ -130,7 +136,8 @@
         try:
             os.unlink(target)
         except OSError as exc:
-            logging.error("Unable to remove tempfile %s: %s", target, exc)
+            logging.error("Unable to remove tempfile %s", target)
+            logging.error("  %s", exc)
 
     def _probe_disks(self):
         """
@@ -519,23 +526,33 @@
                                 test.clean_up(file)
                             total_write_time = sum(write_times)
                             avg_write_time = total_write_time / args.count
-                            avg_write_speed = ((
-                                total_write_size / total_write_time)
-                                / 1024 / 1024)
-                            iteration_write_times.append(total_write_time)
-                            print("\t[Iteration %s] Average Speed: %0.4f"
-                                  % (iteration, avg_write_speed))
+                            try:
+                                avg_write_speed = ((
+                                    total_write_size / total_write_time)
+                                    / 1024 / 1024)
+                            except ZeroDivisionError:
+                                avg_write_speed = 0.00
+                            finally:
+                                iteration_write_times.append(total_write_time)
+                                print("\t[Iteration %s] Average Speed: %0.4f"
+                                      % (iteration, avg_write_speed))
                         for iteration in range(args.iterations):
                             iteration_write_time = sum(iteration_write_times)
                         print("\tSummary:")
-                        print("\t\tTotal Data Written: %0.4f MB"
+                        print("\t\tTotal Data Attempted: %0.4f MB"
                               % iteration_write_size)
                         print("\t\tTotal Time to write: %0.4f secs"
                               % iteration_write_time)
                         print("\t\tAverage Write Time: %0.4f secs" %
                               (iteration_write_time / args.iterations))
-                        print("\t\tAverage Write Speed: %0.4f MB/s" %
-                              (iteration_write_size / iteration_write_time))
+                        try:
+                            avg_write_speed = (iteration_write_size /
+                                              iteration_write_time)
+                        except ZeroDivisionError:
+                            avg_write_speed = 0.00
+                        finally:
+                            print("\t\tAverage Write Speed: %0.4f MB/s" %
+                                  avg_write_speed)
                 finally:
                     for key in range(args.count):
                         test.clean_up(test_files[key].tfile.name)