Checkbox

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

  1. 1153894-clarify-mediacard-instructions
  2. Merge into trunk
Proposed by Jeff Lane 
Status: Merged
Approved by: Zygmunt Krynicki
Approved revision: 2105
Merged at revision: 2115
Proposed branch: lp:~bladernr/checkbox/1153894-clarify-mediacard-instructions
Merge into: lp:checkbox
Diff against target: 152 lines (+46/-20)
3 files modified
debian/changelog (+5/-0)
jobs/mediacard.txt.in (+13/-9)
scripts/removable_storage_test (+28/-11)
To merge this branch: bzr merge lp:~bladernr/checkbox/1153894-clarify-mediacard-instructions
Reviewer Review Type Date Requested Status
Zygmunt Krynicki (community) Approve
Jeff Lane  Needs Resubmitting
Review via email: mp+162259@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Jeff Lane  (bladernr) wrote :

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

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

   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 updated
2099. By Jeff Lane 

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

2100. By Brendan Donegan

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

2101. By Jeff Lane 

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

2102. By Jeff Lane 

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

2103. By Jeff Lane 

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

Revision history for this message
Jeff Lane  (bladernr) wrote :

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
Revision history for this message
Jeff Lane  (bladernr) wrote :

Made some changes based on Zygmunt's comments in IRC

lp:~bladernr/checkbox/1153894-clarify-mediacard-instructions updated
2104. By Jeff Lane 

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 

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

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

+1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
15 using and set video type and plugin accordingly. (LP: #1100594)15 using and set video type and plugin accordingly. (LP: #1100594)
16 * scripts/network_check: added ability to specify custom target URL for16 * scripts/network_check: added ability to specify custom target URL for
17 debugging failures (LP: #1128017)17 debugging failures (LP: #1128017)
18 * scripts/removable_storage_test: Added error handling to trap OSError on
19 non-writable media and modified output to handle subsequent
20 ZeroDivisionError issues when summarizing test results.
21 jobs/media.txt.in: Modified instructions for SD/SDHC to specify using
22 UNLOCKED cards to avoid issues when testing read-only media (LP: #1153894)
1823
19 [ Sylvain Pineau ]24 [ Sylvain Pineau ]
20 * jobs/suspend.txt.in, scripts/gpu_test: Remove the need of running the script25 * jobs/suspend.txt.in, scripts/gpu_test: Remove the need of running the script
2126
=== 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 @@
85_description:85_description:
86 PURPOSE:86 PURPOSE:
87 This test will check that the systems media card reader can87 This test will check that the systems media card reader can
88 detect the insertion of a Secure Digital (SD) media card88 detect the insertion of an UNLOCKED Secure Digital (SD) media card
89 STEPS:89 STEPS:
90 1. Click "Test" and insert an SD card into the reader.90 1. Click "Test" and insert an UNLOCKED SD card into the reader.
91 If a file browser opens up, you can safely close it.91 If a file browser opens up, you can safely close it.
92 (Note: this test will time-out after 20 seconds.)92 (Note: this test will time-out after 20 seconds.)
93 2. Do not remove the device after this test.93 2. Do not remove the device after this test.
@@ -126,9 +126,10 @@
126_description:126_description:
127 PURPOSE:127 PURPOSE:
128 This test will check that the systems media card reader can128 This test will check that the systems media card reader can
129 detect the insertion of an SD card after the system has been suspended129 detect the insertion of an UNLOCKED SD card after the system
130 has been suspended
130 STEPS:131 STEPS:
131 1. Click "Test" and insert an SD card into the reader.132 1. Click "Test" and insert an UNLOCKED SD card into the reader.
132 If a file browser opens up, you can safely close it.133 If a file browser opens up, you can safely close it.
133 (Note: this test will time-out after 20 seconds.)134 (Note: this test will time-out after 20 seconds.)
134 2. Do not remove the device after this test.135 2. Do not remove the device after this test.
@@ -152,7 +153,8 @@
152_description:153_description:
153 PURPOSE:154 PURPOSE:
154 This test will check that the system correctly detects 155 This test will check that the system correctly detects
155 the removal of an SD card from the systems card reader after the system has been suspended.156 the removal of an SD card from the systems card reader
157 after the system has been suspended.
156 STEPS:158 STEPS:
157 1. Click "Test" and remove the SD card from the reader.159 1. Click "Test" and remove the SD card from the reader.
158 (Note: this test will time-out after 20 seconds.)160 (Note: this test will time-out after 20 seconds.)
@@ -176,9 +178,10 @@
176_description:178_description:
177 PURPOSE:179 PURPOSE:
178 This test will check that the systems media card reader can180 This test will check that the systems media card reader can
179 detect the insertion of a Secure Digital High-Capacity (SDHC) media card181 detect the insertion of a UNLOCKED Secure Digital High-Capacity
182 (SDHC) media card
180 STEPS:183 STEPS:
181 1. Click "Test" and insert an SDHC card into the reader.184 1. Click "Test" and insert an UNLOCKED SDHC card into the reader.
182 If a file browser opens up, you can safely close it.185 If a file browser opens up, you can safely close it.
183 (Note: this test will time-out after 20 seconds.)186 (Note: this test will time-out after 20 seconds.)
184 2. Do not remove the device after this test.187 2. Do not remove the device after this test.
@@ -217,9 +220,10 @@
217_description:220_description:
218 PURPOSE:221 PURPOSE:
219 This test will check that the systems media card reader can222 This test will check that the systems media card reader can
220 detect the insertion of an SDHC media card after the system has been suspended223 detect the insertion of an UNLOCKED SDHC media card after the
224 system has been suspended
221 STEPS:225 STEPS:
222 1. Click "Test" and insert an SDHC card into the reader.226 1. Click "Test" and insert an UNLOCKED SDHC card into the reader.
223 If a file browser opens up, you can safely close it.227 If a file browser opens up, you can safely close it.
224 (Note: this test will time-out after 20 seconds.)228 (Note: this test will time-out after 20 seconds.)
225 2. Do not remove the device after this test.229 2. Do not remove the device after this test.
226230
=== 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 @@
115 return True115 return True
116116
117 def write_file(self, data, dest):117 def write_file(self, data, dest):
118 with open(dest, 'wb', 0) as outfile:118 try:
119 outfile = open(dest, 'wb', 0)
120 except OSError as exc:
121 logging.error("Unable to open %s for writing.", dest)
122 logging.error(" %s", exc)
123 return False
124 with outfile:
119 try:125 try:
120 outfile.write(self.data)126 outfile.write(self.data)
121 except IOError as exc:127 except IOError as exc:
@@ -130,7 +136,8 @@
130 try:136 try:
131 os.unlink(target)137 os.unlink(target)
132 except OSError as exc:138 except OSError as exc:
133 logging.error("Unable to remove tempfile %s: %s", target, exc)139 logging.error("Unable to remove tempfile %s", target)
140 logging.error(" %s", exc)
134141
135 def _probe_disks(self):142 def _probe_disks(self):
136 """143 """
@@ -519,23 +526,33 @@
519 test.clean_up(file)526 test.clean_up(file)
520 total_write_time = sum(write_times)527 total_write_time = sum(write_times)
521 avg_write_time = total_write_time / args.count528 avg_write_time = total_write_time / args.count
522 avg_write_speed = ((529 try:
523 total_write_size / total_write_time)530 avg_write_speed = ((
524 / 1024 / 1024)531 total_write_size / total_write_time)
525 iteration_write_times.append(total_write_time)532 / 1024 / 1024)
526 print("\t[Iteration %s] Average Speed: %0.4f"533 except ZeroDivisionError:
527 % (iteration, avg_write_speed))534 avg_write_speed = 0.00
535 finally:
536 iteration_write_times.append(total_write_time)
537 print("\t[Iteration %s] Average Speed: %0.4f"
538 % (iteration, avg_write_speed))
528 for iteration in range(args.iterations):539 for iteration in range(args.iterations):
529 iteration_write_time = sum(iteration_write_times)540 iteration_write_time = sum(iteration_write_times)
530 print("\tSummary:")541 print("\tSummary:")
531 print("\t\tTotal Data Written: %0.4f MB"542 print("\t\tTotal Data Attempted: %0.4f MB"
532 % iteration_write_size)543 % iteration_write_size)
533 print("\t\tTotal Time to write: %0.4f secs"544 print("\t\tTotal Time to write: %0.4f secs"
534 % iteration_write_time)545 % iteration_write_time)
535 print("\t\tAverage Write Time: %0.4f secs" %546 print("\t\tAverage Write Time: %0.4f secs" %
536 (iteration_write_time / args.iterations))547 (iteration_write_time / args.iterations))
537 print("\t\tAverage Write Speed: %0.4f MB/s" %548 try:
538 (iteration_write_size / iteration_write_time))549 avg_write_speed = (iteration_write_size /
550 iteration_write_time)
551 except ZeroDivisionError:
552 avg_write_speed = 0.00
553 finally:
554 print("\t\tAverage Write Speed: %0.4f MB/s" %
555 avg_write_speed)
539 finally:556 finally:
540 for key in range(args.count):557 for key in range(args.count):
541 test.clean_up(test_files[key].tfile.name)558 test.clean_up(test_files[key].tfile.name)

Subscribers

People subscribed via source and target branches