Merge lp:~roadmr/checkbox/1329728-fix-xrandr-cycle into lp:checkbox

[Daniel Manrique]

    providers:checkbox: Fix template and exception handling in xrandr_cycle

    The script now looks for its template file in several locations, is more
    strict with exception handling, and provides better error messages if
    things don't work as expected.

    Due to bin/lib split in plainbox providers, it needs a bit of
    configuration to be run manually (i.e. from outside checkbox or
    plainbox), but the script will provide guidance on the environment
    variable to set for this to work.

    providers:checkbox: Fix template and exception handling in xrandr_cycle

    The script now looks for its template file in several locations, is more
    strict with exception handling, and provides better error messages if
    things don't work as expected.

    Due to bin/lib split in plainbox providers, it needs a bit of
    configuration to be run manually (i.e. from outside checkbox or
    plainbox), but the script will provide guidance on the environment
    variable to set for this to work.

[lp:~roadmr/checkbox/1329728-fix-xrandr-cycle]

3082. By Zygmunt Krynicki on 2014-06-18

"automatic merge by tarmac [r=roadmr][bug=1092925][author=zkrynicki]"

3083. By Zygmunt Krynicki on 2014-06-18

"automatic merge by tarmac [r=roadmr][bug=1301227][author=zkrynicki]"

3084. By Daniel Manrique on 2014-06-18

"providers:plainbox: convert sleep_test to python3.

INcludes some pep8 fixes.

Note that I just did the overall conversion to ensure the script runs; however, since it depends on specific markers logged by the kernel in syslog, and those have changed in the past year, the script no longer works (it does suspend but then always outputs "fail"). Some more work is needed to identify marker strings and properly set state based on them.

 [r=zkrynicki][bug=1303941][author=roadmr]"

3085. By Daniel Manrique on 2014-06-18

"providers:checkbox: update usb3 testing jobs to use --driver

This is more reliable than checking for a specific threshold. [r=zkrynicki][bug=1331528][author=roadmr]"

[Zygmunt Krynicki (zyga)]

launchpad u no save inline comments without this fussy textarea

[Zygmunt Krynicki (zyga)]

and needs fixn'

review: Needs Fixing

[lp:~roadmr/checkbox/1329728-fix-xrandr-cycle]

3086. By Daniel Manrique on 2014-06-18

"providers:checkbox: Add device requirement for firewire tests [r=zkrynicki][bug=1305013][author=roadmr]"

3087. By Sylvain Pineau on 2014-06-19

"automatic merge by tarmac [r=sylvain-pineau][bug=1305640][author=sylvain-pineau]"

3088. By Po-Hsu Lin on 2014-06-19

"automatic merge by tarmac [r=roadmr][bug=1330399][author=cypressyew]"

3089. By Zygmunt Krynicki on 2014-06-19

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

3090. By Daniel Manrique on 2014-06-20

"checkbox-support:parsers:udev: Handle case where a candidate network device has no driver
 [r=zkrynicki][bug=1319757][author=roadmr]"

3091. By Po-Hsu Lin on 2014-06-20

"automatic merge by tarmac [r=zkrynicki][bug=1332476][author=cypressyew]"

3092. By Daniel Manrique on 2014-06-20

"make piglit/tarball less verbose. [r=zkrynicki][bug=1169875][author=roadmr]"

3093. By Launchpad Translations on behalf of checkbox-dev on 2014-06-21

Launchpad automatic translations update.

3094. By Chris Gregan on 2014-06-25

"automatic merge by tarmac [r=roadmr][bug=][author=cgregan]"

3095. By Launchpad Translations on behalf of checkbox-dev on 2014-06-26

Launchpad automatic translations update.

3096. By Daniel Manrique on 2014-06-26

"providers:checkbox: update graphics_driver to handle NULL display spec on some NVidia cards [r=zkrynicki][bug=1329607][author=roadmr]"

3097. By Chris Gregan on 2014-06-26

"automatic merge by tarmac [r=roadmr][bug=][author=cgregan]"

3098. By Ashley Lai on 2014-06-27

"automatic merge by tarmac [r=roadmr][bug=1328943][author=alai]"

3099. By Daniel Manrique on 2014-07-02

"The effective exit code of the script could potentially be "1", this forces the offending checks to end with "0" because we care about the text not the return code. [r=roadmr][bug=][author=roadmr]"

3100. By Po-Hsu Lin on 2014-07-02

"automatic merge by tarmac [r=roadmr][bug=][author=cypressyew]"

3101. By Daniel Manrique on 2014-07-02

"automatic merge by tarmac [r=roadmr][bug=1333956][author=roadmr]"

3102. By Jeff Lane  on 2014-07-03

"network script now determines max_speed from ethtool and link_speed from sys/class/net/DEV/speed for more accurate results. [r=roadmr][bug=1336320][author=bladernr]"

3103. By Daniel Manrique on 2014-07-03

"Fixes some problems with the network script (bug 1337537, a small problem with ethtool invocation, some minor string fixes). [r=bladernr][bug=1337537][author=roadmr]"

3104. By Launchpad Translations on behalf of checkbox-dev on 2014-07-04

Launchpad automatic translations update.

3105. By Daniel Manrique on 2014-07-04

"checkbox-support:parsers:udevadm: Refine media card heuristics.

Per https://bugs.launchpad.net/checkbox-support/+bug/1337270, some SSD
hard drives have model names which made our card reader regular
expression mistake them for card readers (specifically the string mSATA
looked like a memory stick to our naive regex). This adds a simple
negative lookahead to ensure MS only matches if it's not followed
immediately by "ata". The regex is case-insensitive. [r=zkrynicki][bug=1337270][author=roadmr]"

3106. By Po-Hsu Lin on 2014-07-04

"automatic merge by tarmac [r=sylvain-pineau][bug=1331302][author=cypressyew]"

3107. By Sylvain Pineau on 2014-07-04

"automatic merge by tarmac [r=roadmr][bug=][author=sylvain-pineau]"

3108. By Sylvain Pineau on 2014-07-07

"automatic merge by tarmac [r=sylvain-pineau][bug=1331302][author=checkbox-dev]"

3109. By Daniel Manrique on 2014-07-09

"Fixes the requirement expression for power-management/tickless_idle. [r=bladernr][bug=1333624][author=roadmr]"

3110. By Chris Gregan on 2014-07-10

"automatic merge by tarmac [r=roadmr][bug=][author=cgregan]"

3111. By Mike Rushton on 2014-07-10

"added ipv6 support via bug 1013273 [r=roadmr][bug=1013273][author=leftyfb]"

3112. By Sylvain Pineau on 2014-07-11

"automatic merge by tarmac [r=sylvain-pineau][bug=][author=sylvain-pineau]"

3113. By Jeff Lane  on 2014-07-11

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

3114. By Daniel Manrique on 2014-07-11

providers:checkbox: Fix template and exception handling in xrandr_cycle

The script now looks for its template file in several locations, is more
strict with exception handling, and provides better error messages if
things don't work as expected.

Due to bin/lib split in plainbox providers, it needs a bit of
configuration to be run manually (i.e. from outside checkbox or
plainbox), but the script will provide guidance on the environment
variable to set for this to work.

3115. By Daniel Manrique on 2014-07-11

providers:checkbox: xrandr_cycle pep8 and assorted fixes

[Daniel Manrique (roadmr)]

Fixed and resubmitting. Some clarification is needed so this is potentially not ready yet. Let me know.

[Zygmunt Krynicki (zyga)]

+1, thanks

I'm still partially -1 on the 'cannot find .xml file' problem. I don't think our scripts should go all the way to getting that to work (we certainly don't do it consistently). Still, this is a system problem and you shouldn't need to fix it in this particular instance.

review: Approve

[Daniel Manrique (roadmr)]

Flipping the approve flag based on the previous comment (?)

=== modified file 'providers/plainbox-provider-checkbox/bin/xrandr_cycle'
--- providers/plainbox-provider-checkbox/bin/xrandr_cycle	2014-03-20 14:01:58 +0000
+++ providers/plainbox-provider-checkbox/bin/xrandr_cycle	2014-07-11 20:58:06 +0000
@@ -1,13 +1,14 @@
 #!/usr/bin/env python3
 
+import argparse
+import errno
+import os
+import re
+import shutil
 import subprocess
-import argparse
+import sys
 import tarfile
-import shutil
 import time
-import sys
-import os
-import re
 
 parser = argparse.ArgumentParser()
 parser.add_argument('--keyword', default='',
@@ -68,7 +69,19 @@
 # Now we have a list of the modes we need to test.  So let's do just that.
 profile_path = os.environ['HOME'] + '/.shutter/profiles/'
 screenshot_path = os.path.join(args.screenshot_dir, 'xrandr_screens')
-script_home = os.path.split(os.path.dirname(os.path.realpath(__file__)))[0]
+
+# Where to find the shutter.xml template? Two possible locations.
+shutter_xml_template = None
+
+if 'PLAINBOX_PROVIDER_DATA' in os.environ:
+    shutter_xml_template = os.path.join(os.environ['PLAINBOX_PROVIDER_DATA'],
+                                        "settings", "shutter.xml")
+else:
+    shutter_xml_template = os.path.join(os.path.split(os.path.dirname(
+                                        os.path.realpath(__file__)))[0],
+                                       "data",
+                                        "settings",
+                                        "shutter.xml")
 
 if args.keyword:
     screenshot_path = screenshot_path + '_' + args.keyword
@@ -76,21 +89,27 @@
 regex = re.compile(r'filename="[^"\r\n]*"')
 
 # Keep the shutter profile in place before starting
-try:
-    os.makedirs(profile_path)
-except OSError:
-    pass
-
-try:
-    os.makedirs(screenshot_path)
-except OSError:
-    pass
-
-try:
-    shutil.copy(script_home + '/data/settings/shutter.xml',
-                profile_path)
-except IOError:
-    pass
+
+# Any errors creating the directories or copying the template is fatal,
+# since things won't work if we fail.
+try:
+    os.makedirs(profile_path, exist_ok=True)
+    os.makedirs(screenshot_path, exist_ok=True)
+except OSError as excp:
+    raise SystemExit("ERROR: Unable to create "
+                     "required directories: {}".format(excp))
+
+try:
+    shutil.copy(shutter_xml_template, profile_path)
+except (IOError, OSError) as excp:
+    print("ERROR: Unable to copy {} to {}: {}".format(shutter_xml_template,
+                                                      profile_path,
+                                                      excp))
+    if excp.errno == errno.ENOENT:
+        print("Try setting PLAINBOX_PROVIDER_DATA to the the data path of a")
+        print("provider shipping the 'shutter.xml' template file, usually ")
+        print("found under /usr/share.")
+    raise SystemExit()
 
 try:
     old_profile = open(profile_path + 'shutter.xml', 'r')
@@ -102,7 +121,8 @@
     new_profile.close()
     old_profile.close()
 except:
-    pass
+    raise SystemExit("ERROR: While updating folder name "
+                     "in shutter profile: {}".format(sys.exc_info()))
 
 for mode in modes:
     cmd = 'xrandr --output ' + mode[0] + ' --mode ' + mode[1]
@@ -134,7 +154,9 @@
         except:
             print("""Could not configure screenshot tool -
                      you may need to install the package 'shutter',
-                     or check that %s exists.""" % profile_path)
+                     or check that {}/{} exists and is writable.""".format(
+                profile_path,
+                'shutter.xml'))
 
         message = 'Set mode ' + mode[1] + ' for output ' + mode[0]
         success_messages.append(message)