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

Proposed by Jeff Lane on 2012-03-08
Status: Merged
Approved by: Javier Collado on 2012-03-20
Approved revision: 1315
Merged at revision: 1326
Proposed branch: lp:~bladernr/checkbox/949435-remove-hard-coded-paths
Merge into: lp:checkbox
Diff against target: 102 lines (+21/-7)
5 files modified
debian/changelog (+9/-0)
jobs/graphics.txt.in (+1/-1)
jobs/suspend.txt.in (+2/-2)
scripts/gpu_test (+2/-1)
scripts/xrandr_cycle (+7/-3)
To merge this branch: bzr merge lp:~bladernr/checkbox/949435-remove-hard-coded-paths
Reviewer Review Type Date Requested Status
Javier Collado (community) 2012-03-08 Approve on 2012-03-20
Review via email: mp+96681@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
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) wrote :

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`

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) wrote :

> 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) wrote :

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

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 2012-03-08 14:31:17 +0000
+++ debian/changelog 2012-03-09 16:17:41 +0000
@@ -1,3 +1,12 @@
1checkbox (0.13.5) precise; urgency=low
2
3 [Jeff Lane]
4 * Modified xrandr_cycle and gpu_test to not use hard-coded checkbox paths.
5 Now they uses os.path to find it's home and adds a --screnshot-dir option
6 to specify the location to store screenshots (LP: #949435)
7
8 -- Jeff Lane <jeff@ubuntu.com> Thu, 08 Mar 2012 18:10:44 -0500
9
1checkbox (0.13.4) precise; urgency=low10checkbox (0.13.4) precise; urgency=low
211
3 [Brendan Donegan]12 [Brendan Donegan]
413
=== 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 @@
109name: graphics/cycle_resolution109name: graphics/cycle_resolution
110requires: package.name == 'xorg'110requires: package.name == 'xorg'
111depends: graphics/VESA_drivers_not_in_use111depends: graphics/VESA_drivers_not_in_use
112command: xrandr_cycle112command: xrandr_cycle --screenshot-dir $CHECKBOX_DATA
113_description:113_description:
114 PURPOSE:114 PURPOSE:
115 This test cycles through the detected video modes115 This test cycles through the detected video modes
116116
=== 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 @@
216name: suspend/cycle_resolutions_after_suspend216name: suspend/cycle_resolutions_after_suspend
217requires: package.name == 'xorg'217requires: package.name == 'xorg'
218depends: suspend/suspend_advanced graphics/cycle_resolution218depends: suspend/suspend_advanced graphics/cycle_resolution
219command: xrandr_cycle --keyword=after_suspend219command: xrandr_cycle --keyword=after_suspend --screenshot-dir $CHECKBOX_DATA
220_description:220_description:
221 PURPOSE:221 PURPOSE:
222 This test will cycle through the detected display modes222 This test will cycle through the detected display modes
@@ -232,7 +232,7 @@
232_description:232_description:
233 This test will check to make sure supported video modes work after a suspend and resume.233 This test will check to make sure supported video modes work after a suspend and resume.
234 This is done automatically by taking screenshots and uploading them as an attachment.234 This is done automatically by taking screenshots and uploading them as an attachment.
235command: xrandr_cycle --keyword=after_suspend235command: xrandr_cycle --keyword=after_suspend --screenshot-dir $CHECKBOX_DATA
236236
237plugin: attachment237plugin: attachment
238name: suspend/xrandr_screens_after_suspend.tar.gz238name: suspend/xrandr_screens_after_suspend.tar.gz
239239
=== 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 @@
100 """100 """
101101
102 def run(self):102 def run(self):
103 flv = os.environ['CHECKBOX_SHARE'] + '/data/websites/Flash_Video.flv'103 script_home = os.path.split(os.path.dirname(os.path.realpath(__file__)))[0]
104 flv = os.path.join(script_home + '/data/websites/Flash_Video.flv')
104 source = """105 source = """
105<html><head><meta http-equiv="refresh" content="5"></head><body>106<html><head><meta http-equiv="refresh" content="5"></head><body>
106<p>This page will reload and play this Flash video every 5s.</p>107<p>This page will reload and play this Flash video every 5s.</p>
107108
=== 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 @@
12parser = argparse.ArgumentParser()12parser = argparse.ArgumentParser()
13parser.add_argument('--keyword', default='',13parser.add_argument('--keyword', default='',
14 help='A keyword to distinguish the screenshots taken in this run of the script')14 help='A keyword to distinguish the screenshots taken in this run of the script')
1515parser.add_argument('--screenshot-dir',
16 default=os.environ['HOME'],
17 help='Specify a directory to store screenshots in. Default is %(default)s')
16args = parser.parse_args()18args = parser.parse_args()
1719
20
18device_context = '' # track what device's modes we are looking at21device_context = '' # track what device's modes we are looking at
19modes = [] # keep track of all the devices and modes discovered22modes = [] # keep track of all the devices and modes discovered
20current_modes = [] # remember the user's current settings for cleanup later23current_modes = [] # remember the user's current settings for cleanup later
@@ -62,7 +65,8 @@
6265
63# Now we have a list of the modes we need to test. So let's do just that.66# Now we have a list of the modes we need to test. So let's do just that.
64profile_path = os.environ['HOME'] + '/.shutter/profiles/'67profile_path = os.environ['HOME'] + '/.shutter/profiles/'
65screenshot_path = os.environ['CHECKBOX_DATA'] + 'xrandr_screens'68screenshot_path = os.path.join(args.screenshot_dir,'xrandr_screens')
69script_home = os.path.split(os.path.dirname(os.path.realpath(__file__)))[0]
6670
67if args.keyword: screenshot_path = screenshot_path + '_' + args.keyword71if args.keyword: screenshot_path = screenshot_path + '_' + args.keyword
6872
@@ -80,7 +84,7 @@
80 pass84 pass
8185
82try:86try:
83 shutil.copy(os.environ['CHECKBOX_SHARE'] + '/data/settings/shutter.xml',87 shutil.copy(script_home + '/data/settings/shutter.xml',
84 profile_path)88 profile_path)
85except IOError:89except IOError:
86 pass90 pass

Subscribers

People subscribed via source and target branches