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
1=== modified file 'debian/changelog'
2--- debian/changelog 2012-03-08 14:31:17 +0000
3+++ debian/changelog 2012-03-09 16:17:41 +0000
4@@ -1,3 +1,12 @@
5+checkbox (0.13.5) precise; urgency=low
6+
7+ [Jeff Lane]
8+ * Modified xrandr_cycle and gpu_test to not use hard-coded checkbox paths.
9+ Now they uses os.path to find it's home and adds a --screnshot-dir option
10+ to specify the location to store screenshots (LP: #949435)
11+
12+ -- Jeff Lane <jeff@ubuntu.com> Thu, 08 Mar 2012 18:10:44 -0500
13+
14 checkbox (0.13.4) precise; urgency=low
15
16 [Brendan Donegan]
17
18=== modified file 'jobs/graphics.txt.in'
19--- jobs/graphics.txt.in 2012-03-07 00:16:27 +0000
20+++ jobs/graphics.txt.in 2012-03-09 16:17:41 +0000
21@@ -109,7 +109,7 @@
22 name: graphics/cycle_resolution
23 requires: package.name == 'xorg'
24 depends: graphics/VESA_drivers_not_in_use
25-command: xrandr_cycle
26+command: xrandr_cycle --screenshot-dir $CHECKBOX_DATA
27 _description:
28 PURPOSE:
29 This test cycles through the detected video modes
30
31=== modified file 'jobs/suspend.txt.in'
32--- jobs/suspend.txt.in 2012-03-06 17:44:32 +0000
33+++ jobs/suspend.txt.in 2012-03-09 16:17:41 +0000
34@@ -216,7 +216,7 @@
35 name: suspend/cycle_resolutions_after_suspend
36 requires: package.name == 'xorg'
37 depends: suspend/suspend_advanced graphics/cycle_resolution
38-command: xrandr_cycle --keyword=after_suspend
39+command: xrandr_cycle --keyword=after_suspend --screenshot-dir $CHECKBOX_DATA
40 _description:
41 PURPOSE:
42 This test will cycle through the detected display modes
43@@ -232,7 +232,7 @@
44 _description:
45 This test will check to make sure supported video modes work after a suspend and resume.
46 This is done automatically by taking screenshots and uploading them as an attachment.
47-command: xrandr_cycle --keyword=after_suspend
48+command: xrandr_cycle --keyword=after_suspend --screenshot-dir $CHECKBOX_DATA
49
50 plugin: attachment
51 name: suspend/xrandr_screens_after_suspend.tar.gz
52
53=== modified file 'scripts/gpu_test'
54--- scripts/gpu_test 2012-03-07 15:52:28 +0000
55+++ scripts/gpu_test 2012-03-09 16:17:41 +0000
56@@ -100,7 +100,8 @@
57 """
58
59 def run(self):
60- flv = os.environ['CHECKBOX_SHARE'] + '/data/websites/Flash_Video.flv'
61+ script_home = os.path.split(os.path.dirname(os.path.realpath(__file__)))[0]
62+ flv = os.path.join(script_home + '/data/websites/Flash_Video.flv')
63 source = """
64 <html><head><meta http-equiv="refresh" content="5"></head><body>
65 <p>This page will reload and play this Flash video every 5s.</p>
66
67=== modified file 'scripts/xrandr_cycle'
68--- scripts/xrandr_cycle 2011-07-19 10:45:14 +0000
69+++ scripts/xrandr_cycle 2012-03-09 16:17:41 +0000
70@@ -12,9 +12,12 @@
71 parser = argparse.ArgumentParser()
72 parser.add_argument('--keyword', default='',
73 help='A keyword to distinguish the screenshots taken in this run of the script')
74-
75+parser.add_argument('--screenshot-dir',
76+ default=os.environ['HOME'],
77+ help='Specify a directory to store screenshots in. Default is %(default)s')
78 args = parser.parse_args()
79
80+
81 device_context = '' # track what device's modes we are looking at
82 modes = [] # keep track of all the devices and modes discovered
83 current_modes = [] # remember the user's current settings for cleanup later
84@@ -62,7 +65,8 @@
85
86 # Now we have a list of the modes we need to test. So let's do just that.
87 profile_path = os.environ['HOME'] + '/.shutter/profiles/'
88-screenshot_path = os.environ['CHECKBOX_DATA'] + 'xrandr_screens'
89+screenshot_path = os.path.join(args.screenshot_dir,'xrandr_screens')
90+script_home = os.path.split(os.path.dirname(os.path.realpath(__file__)))[0]
91
92 if args.keyword: screenshot_path = screenshot_path + '_' + args.keyword
93
94@@ -80,7 +84,7 @@
95 pass
96
97 try:
98- shutil.copy(os.environ['CHECKBOX_SHARE'] + '/data/settings/shutter.xml',
99+ shutil.copy(script_home + '/data/settings/shutter.xml',
100 profile_path)
101 except IOError:
102 pass

Subscribers

People subscribed via source and target branches