Merge lp:~bladernr/checkbox/949435-remove-hard-coded-paths into lp:checkbox

[Jeff Lane ]

Fixes bug 949435 regarding the hard coded checkbox paths in gpu_test and xrandr_cycle.

Also, bumped the changelog to 0.13.5 since 0.13.4 has been submitted to main.

[lp:~bladernr/checkbox/949435-remove-hard-coded-paths]

1314. By Jeff Lane  on 2012-03-08

modified the jobs that use xrandr_cycle to use the new screenshot-dir option

[Javier Collado (javier.collado)]

In `xrandr_cycle`, I don't like much the line in which `my_path` is being assigned because:

- is defined in line 20 and not used until line 87
- the name isn't very descriptive
- I'm not sure why `os.path.split(<whatever>)[0]` is used instead of `os.path.dirname`
  (even if it contains another call to `os.path.dirname`)

The last two comments also apply to `gpu_test`

[lp:~bladernr/checkbox/949435-remove-hard-coded-paths]

1315. By Jeff Lane  on 2012-03-09

xrandr_cycle: moved the path discovery down further in to the script to the area it's actually used. xrandr_cycle and gpu_test: renamed my_path to script_home and hoping that's a little more clear

[Jeff Lane  (bladernr)]

> In `xrandr_cycle`, I don't like much the line in which `my_path` is being
> assigned because:
>
> - is defined in line 20 and not used until line 87

Good point there... moved it.

> - the name isn't very descriptive

renamed in both scripts to script_home

> - I'm not sure why `os.path.split(<whatever>)[0]` is used instead of
> `os.path.dirname`
> (even if it contains another call to `os.path.dirname`)

bladernr@klaatu:~/development/949435-remove-hard-coded-checkbox-vars/scripts$ ./xrandr_cycle
path.realpath : /home/bladernr/development/949435-remove-hard-coded-checkbox-vars/scripts/xrandr_cycle
path.dirname : /home/bladernr/development/949435-remove-hard-coded-checkbox-vars/scripts
path.split : ('/home/bladernr/development/949435-remove-hard-coded-checkbox-vars', 'scripts')

ultimately,

os.path.split(os.path.dirname(os.path.realpath(__file__)))[0]

does exactly the same thing as

os.path.dirname(os.path.dirname(os.path.realpath(__file__)))

but looks a bit cleaner, IMHO.

[Javier Collado (javier.collado)]

I've executed the scripts from the branch and the modified jobs in a whitelist and they worked fine.

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2012-03-08 14:31:17 +0000
+++ debian/changelog	2012-03-09 16:17:41 +0000
@@ -1,3 +1,12 @@
+checkbox (0.13.5) precise; urgency=low
+
+  [Jeff Lane]
+  * Modified xrandr_cycle and gpu_test to not use hard-coded checkbox paths.
+    Now they uses os.path to find it's home and adds a --screnshot-dir option
+    to specify the location to store screenshots (LP: #949435)
+
+ -- Jeff Lane <[e-mail]>  Thu, 08 Mar 2012 18:10:44 -0500
+
 checkbox (0.13.4) precise; urgency=low
 
   [Brendan Donegan]
 
=== modified file 'jobs/graphics.txt.in'
--- jobs/graphics.txt.in	2012-03-07 00:16:27 +0000
+++ jobs/graphics.txt.in	2012-03-09 16:17:41 +0000
@@ -109,7 +109,7 @@
 name: graphics/cycle_resolution
 requires: package.name == 'xorg'
 depends: graphics/VESA_drivers_not_in_use
-command: xrandr_cycle
+command: xrandr_cycle --screenshot-dir $CHECKBOX_DATA
 _description:
  PURPOSE:
      This test cycles through the detected video modes
 
=== modified file 'jobs/suspend.txt.in'
--- jobs/suspend.txt.in	2012-03-06 17:44:32 +0000
+++ jobs/suspend.txt.in	2012-03-09 16:17:41 +0000
@@ -216,7 +216,7 @@
 name: suspend/cycle_resolutions_after_suspend
 requires: package.name == 'xorg'
 depends: suspend/suspend_advanced graphics/cycle_resolution
-command: xrandr_cycle --keyword=after_suspend
+command: xrandr_cycle --keyword=after_suspend --screenshot-dir $CHECKBOX_DATA
 _description:
  PURPOSE:
      This test will cycle through the detected display modes
@@ -232,7 +232,7 @@
 _description:
  This test will check to make sure supported video modes work after a suspend and resume.
  This is done automatically by taking screenshots and uploading them as an attachment.
-command: xrandr_cycle --keyword=after_suspend
+command: xrandr_cycle --keyword=after_suspend --screenshot-dir $CHECKBOX_DATA
 
 plugin: attachment
 name: suspend/xrandr_screens_after_suspend.tar.gz
 
=== modified file 'scripts/gpu_test'
--- scripts/gpu_test	2012-03-07 15:52:28 +0000
+++ scripts/gpu_test	2012-03-09 16:17:41 +0000
@@ -100,7 +100,8 @@
     """
 
     def run(self):
-        flv = os.environ['CHECKBOX_SHARE'] + '/data/websites/Flash_Video.flv'
+        script_home = os.path.split(os.path.dirname(os.path.realpath(__file__)))[0]
+        flv = os.path.join(script_home + '/data/websites/Flash_Video.flv')
         source = """
 <html><head><meta http-equiv="refresh" content="5"></head><body>
 <p>This page will reload and play this Flash video every 5s.</p>
 
=== modified file 'scripts/xrandr_cycle'
--- scripts/xrandr_cycle	2011-07-19 10:45:14 +0000
+++ scripts/xrandr_cycle	2012-03-09 16:17:41 +0000
@@ -12,9 +12,12 @@
 parser = argparse.ArgumentParser()
 parser.add_argument('--keyword', default='',
                     help='A keyword to distinguish the screenshots taken in this run of the script')
-
+parser.add_argument('--screenshot-dir',
+                    default=os.environ['HOME'],
+                    help='Specify a directory to store screenshots in. Default is %(default)s')
 args = parser.parse_args()
 
+
 device_context = ''   # track what device's modes we are looking at
 modes = []            # keep track of all the devices and modes discovered
 current_modes = []    # remember the user's current settings for cleanup later
@@ -62,7 +65,8 @@
 
 # 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.environ['CHECKBOX_DATA'] + 'xrandr_screens'
+screenshot_path = os.path.join(args.screenshot_dir,'xrandr_screens')
+script_home = os.path.split(os.path.dirname(os.path.realpath(__file__)))[0]
 
 if args.keyword: screenshot_path = screenshot_path + '_' + args.keyword
 
@@ -80,7 +84,7 @@
     pass
 
 try:
-    shutil.copy(os.environ['CHECKBOX_SHARE'] + '/data/settings/shutter.xml',
+    shutil.copy(script_home + '/data/settings/shutter.xml',
                 profile_path)
 except IOError:
     pass