Merge lp:~sylvain-pineau/checkbox/bug1048262 into lp:checkbox

[Sylvain Pineau]

graphics_stress_test now exits with a proper error message if rendercheck is not installed (i.e: x11-apps in
Precise)

[Zygmunt Krynicki (zyga)]

+ except(OSError):

The syntax is wrong, there is no need for parentheses here

In addition I would add a test for errno (exc.errno == errno.ENOENT) as the catch is otherwise too strong IMHO

As for the rest, what is the point of moving that code around:

59 + rendercheck = RenderCheckWrapper(args.temp)
60 + tests = rendercheck.get_suites_list()
61 +

review: Needs Information

[Sylvain Pineau (sylvain-pineau)]

Good catch, I'm going to fix the except block with your recommendation. But i've just found another script providing the exact same class: rendercheck_test. I will apply the same fix.

The point of moving the call to get_suites_list is to exit on error earlier and not let the script performing useless operations.

[lp:~sylvain-pineau/checkbox/bug1048262]

1658. By Sylvain Pineau on 2012-09-12

Fix scripts/graphics_stress_test and scripts/rendercheck_test exception blocks to abort the test only if the error is errno.ENOENT

[Sylvain Pineau (sylvain-pineau)]

Fixed the exception blocks in scripts/graphics_stress_test and scripts/rendercheck_test as they provide the same class: RenderCheck

review: Needs Resubmitting

[Zygmunt Krynicki (zyga)]

+1

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2012-09-12 09:28:33 +0000
+++ debian/changelog	2012-09-12 12:19:58 +0000
@@ -87,6 +87,9 @@
     names > 50 chars (LP: #1046274)
   * scripts/camera_test, jobs/camera.txt.in: Added a 10s timeout to the camera
     still test (LP: #990133)
+  * scripts/graphics_stress_test, scripts/rendercheck_test, jobs/stress.txt.in: 
+    Exit with proper error message if rendercheck is not installed.
+    (LP: #1048262)
 
   [Zygmunt Krynicki]
   * Fixed simple duplicate 'the' mistakes (LP: #1040022)

=== modified file 'jobs/stress.txt.in'
--- jobs/stress.txt.in	2012-08-31 12:12:52 +0000
+++ jobs/stress.txt.in	2012-09-12 12:19:58 +0000
@@ -132,7 +132,7 @@
 requires:
  package.name == 'x11-apps'
 user: root
-command: graphics_stress_test -d -o $CHECKBOX_DATA/graphics-stress-results && echo "Graphics Stress Test completed successfully" || echo "Graphics Stress Test completed, but there are errors. Please see the log $CHECKBOX_DATA/graphics-stress-results for details"
+command: graphics_stress_test -d -o $CHECKBOX_DATA/graphics-stress-results && echo "Graphics Stress Test completed successfully" || echo "Graphics Stress Test completed, but there are errors. Please see the log $CHECKBOX_DATA/graphics-stress-results for details" && false
 _description:
  Run the graphics stress test. This test can take a few minutes.
 

=== modified file 'scripts/graphics_stress_test'
--- scripts/graphics_stress_test	2012-08-31 17:20:12 +0000
+++ scripts/graphics_stress_test	2012-09-12 12:19:58 +0000
@@ -207,10 +207,13 @@
             # and we don't want to store it in memory
             process = Popen(full_command, stdout=temp_file,
                             universal_newlines=True)
-        except(OSError):
-            logging.debug('Error: please make sure that rendercheck '
-                          'is installed.')
-            exit(1)
+        except OSError as exc:
+            if exc.errno == errno.ENOENT:
+                logging.error('Error: please make sure that rendercheck '
+                              'is installed.')
+                exit(1)
+            else:
+                raise
 
         exit_code = process.wait()
 
@@ -271,8 +274,17 @@
 
     def get_suites_list(self):
         '''Return a list of the available test suites'''
-        process = Popen(['rendercheck', '--help'], stdout=PIPE,
-                        stderr=PIPE, universal_newlines=True)
+        try:
+            process = Popen(['rendercheck', '--help'], stdout=PIPE,
+                            stderr=PIPE, universal_newlines=True)
+        except OSError as exc:
+            if exc.errno == errno.ENOENT:
+                logging.error('Error: please make sure that rendercheck '
+                              'is installed.')
+                exit(1)
+            else:
+                raise
+
         proc = process.communicate()[1].split('\n')
         found = False
         tests_pattern = re.compile('.*Available tests: *(.+).*')
@@ -356,7 +368,6 @@
         logfile_handler.setFormatter(logging.Formatter(format))
         logger.addHandler(logfile_handler)
         log_path = os.path.abspath(logfile)
-        logging.info("The log can be found at %s" % log_path)
 
     # Write only to stdout
     else:
@@ -371,6 +382,9 @@
 
     status = 0
 
+    rendercheck = RenderCheckWrapper(args.temp)
+    tests = rendercheck.get_suites_list()
+
     # Switch between the tty where X lives and tty10
     vt_wrap = VtWrapper()
     target_vt = 10
@@ -396,7 +410,7 @@
     else:
         logging.error('Error: please run X on a tty other than 10')
 
-    # Call sleep 10 times
+    # Call sleep x times
     logging.info('== Sleep test ==')
     sleep_test = SuspendWrapper()
     sleep_mode = 'mem'
@@ -417,7 +431,7 @@
         # Skip the test
         logging.info('Skipped (the system does not seem to support S3')
 
-    # Rotate the screen 10 times
+    # Rotate the screen x times
     # The app already rotates the screen 5 times
     logging.info('== Rotation test ==')
     rotation_test = RotationWrapper()
@@ -430,11 +444,8 @@
             logging.error('Failed')
             status = 1
 
-    # Call rendercheck 10 times
+    # Call rendercheck x times
     logging.info('== Rendercheck test ==')
-    rendercheck = RenderCheckWrapper(args.temp)
-    tests = rendercheck.get_suites_list()
-
     if rendercheck.run_test(tests, args.iterations,
                             args.debug) == 0:
         logging.info('Passed')

=== modified file 'scripts/rendercheck_test'
--- scripts/rendercheck_test	2012-08-25 16:45:00 +0000
+++ scripts/rendercheck_test	2012-09-12 12:19:58 +0000
@@ -62,10 +62,13 @@
             # and we don't want to store it in memory
             process = Popen(full_command, stdout=temp_file,
                             universal_newlines=True)
-        except(OSError):
-            logging.debug('Error: please make sure that rendercheck '
-                          'is installed.')
-            exit(1)
+        except OSError as exc:
+            if exc.errno == errno.ENOENT:
+                logging.error('Error: please make sure that rendercheck '
+                              'is installed.')
+                exit(1)
+            else:
+                raise
 
         exit_code = process.wait()
 
@@ -126,8 +129,17 @@
 
     def get_suites_list(self):
         '''Return a list of the available test suites'''
-        process = Popen(['rendercheck', '--help'], stdout=PIPE,
-                        stderr=PIPE, universal_newlines=True)
+        try:
+            process = Popen(['rendercheck', '--help'], stdout=PIPE,
+                            stderr=PIPE, universal_newlines=True)
+        except OSError as exc:
+            if exc.errno == errno.ENOENT:
+                logging.error('Error: please make sure that rendercheck '
+                              'is installed.')
+                exit(1)
+            else:
+                raise
+
         proc = process.communicate()[1].split('\n')
         found = False
         tests_pattern = re.compile('.*Available tests: *(.+).*')