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

Proposed by Sylvain Pineau
Status: Merged
Merged at revision: 1419
Proposed branch: lp:~sylvain-pineau/checkbox/gpu_test_2to3
Merge into: lp:checkbox
Diff against target: 195 lines (+57/-31) (has conflicts)
2 files modified
debian/changelog (+5/-0)
scripts/gpu_test (+52/-31)
Text conflict in debian/changelog
To merge this branch: bzr merge lp:~sylvain-pineau/checkbox/gpu_test_2to3
Reviewer Review Type Date Requested Status
Marc Tardif (community) Approve
Sylvain Pineau (community) Needs Resubmitting
Review via email: mp+109111@code.launchpad.net

Description of the change

gpu_test conversion to python 3.
pexpect dependency removed, and Flash/firefox test is now launched with normal user.

To post a comment you must log in.
1417. By Sylvain Pineau

debian/changelog updated

Revision history for this message
Marc Tardif (cr3) wrote :

1. I noticed you are now checking that the script runs as root. Instead of checking for the UID, I would suggest you check for the effective UID. What do you think?

The rest looks good as far as the conversion is concerned, and I really like how you had the courage to replace pexpect in the gpu_test script. If you have a moment, I have a few syntactic suggestions:

1. When using the with statement, the variable assigned to the output of the given function is typically only used within the scope of the code block. This is not a problem with this merge request, nor does it prevent the code from working because Python variables continue existing outside of some code blocks. So, I would suggest using the self.tempfile variable instead of f outside the with code block:

        os.chmod(self.tempfile, stat.S_IROTH)
        subprocess.call(
            'sudo -H -u %s firefox %s' % (os.getenv('SUDO_USER'), self.tempfile),
            stdout=open(os.devnull, 'w'),
            stderr=subprocess.STDOUT,
            shell=True)

2. When specifying an encoding to the encode() method, I would suggest not passing anything instead of "utf-8" that is the default value anyways.

3. I noticed that you changed the type of x_res, y_res, hsize and vsize to integers. So, instead of casting the division of those variables to int, you might like to use integer division instead:

  DesktopSwitch = ChangeViewport(
    hsize, vsize, x_res // hsize, y_res // vsize)

review: Needs Fixing
1418. By Sylvain Pineau

Fixed minor syntax issues after peer review

Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

Fixed the code after Marc's review

review: Needs Resubmitting
Revision history for this message
Marc Tardif (cr3) wrote :

Thanks, merging!

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-06-07 15:39:38 +0000
+++ debian/changelog 2012-06-07 16:12:18 +0000
@@ -70,9 +70,14 @@
7070
7171
72 [Sylvain Pineau]72 [Sylvain Pineau]
73<<<<<<< TREE
73 * [FEATURE] Python 2 to 3 conversion:74 * [FEATURE] Python 2 to 3 conversion:
74 * scripts/gst_pipeline_test. Migrated to PyGI.75 * scripts/gst_pipeline_test. Migrated to PyGI.
75 * Fullscreen playbacks are now performed with Totem instead of gst_pipeline_test. 76 * Fullscreen playbacks are now performed with Totem instead of gst_pipeline_test.
77=======
78 * [FEATURE] Python 2 to 3 conversion:
79 * scripts/gpu_test
80>>>>>>> MERGE-SOURCE
76 * scripts/pm_log_check: added a slightly modified version of OEM team's pm_check.py81 * scripts/pm_log_check: added a slightly modified version of OEM team's pm_check.py
77 script to analyze pm_test logs82 script to analyze pm_test logs
78 * jobs/stress.txt.in: add OEM team's stress tests (including reboot and poweroff)83 * jobs/stress.txt.in: add OEM team's stress tests (including reboot and poweroff)
7984
=== modified file 'scripts/gpu_test'
--- scripts/gpu_test 2012-05-22 18:01:21 +0000
+++ scripts/gpu_test 2012-06-07 16:12:18 +0000
@@ -1,11 +1,11 @@
1#!/usr/bin/python1#!/usr/bin/python3
22
3import re3import re
4import os4import os
5import stat
5import sys6import sys
6import time7import time
7import pexpect8import subprocess
8from subprocess import call, STDOUT
9from threading import Thread9from threading import Thread
10from tempfile import NamedTemporaryFile10from tempfile import NamedTemporaryFile
11from math import cos, sin11from math import cos, sin
@@ -17,10 +17,14 @@
17 """17 """
1818
19 def run(self):19 def run(self):
20 pexpect.run('glxgears -geometry 400x400', timeout=None)20 subprocess.call(
21 'glxgears -geometry 400x400',
22 stdout=open(os.devnull, 'w'),
23 stderr=subprocess.STDOUT,
24 shell=True)
2125
22 def terminate(self):26 def terminate(self):
23 call('wmctrl -i -c %s' % self.id, shell=True)27 subprocess.call('wmctrl -i -c %s' % self.id, shell=True)
2428
2529
26class RotateGlxThread(Thread):30class RotateGlxThread(Thread):
@@ -40,8 +44,9 @@
40 x = int(200 * self.offset + 100 * sin(j * 0.2))44 x = int(200 * self.offset + 100 * sin(j * 0.2))
41 y = int(200 * self.offset + 100 * cos(j * 0.2))45 y = int(200 * self.offset + 100 * cos(j * 0.2))
42 coords = "%s,%s" % (x, y)46 coords = "%s,%s" % (x, y)
43 call('wmctrl -i -r %s -e 0,%s,-1,-1' % (self.id, coords),47 subprocess.call(
44 shell=True)48 'wmctrl -i -r %s -e 0,%s,-1,-1' % (self.id, coords),
49 shell=True)
45 time.sleep(0.002 * self.offset)50 time.sleep(0.002 * self.offset)
46 if self.cancel:51 if self.cancel:
47 return52 return
@@ -60,11 +65,11 @@
60 def run(self):65 def run(self):
61 while(1):66 while(1):
62 for i in range(self.size):67 for i in range(self.size):
63 call('wmctrl -s %s' % i, shell=True)68 subprocess.call('wmctrl -s %s' % i, shell=True)
64 time.sleep(0.5)69 time.sleep(0.5)
65 if self.cancel:70 if self.cancel:
66 # Switch back to workspace #171 # Switch back to workspace #1
67 call('wmctrl -s 0', shell=True)72 subprocess.call('wmctrl -s 0', shell=True)
68 return73 return
6974
7075
@@ -85,12 +90,13 @@
85 while(1):90 while(1):
86 for i in range(self.hsize):91 for i in range(self.hsize):
87 for j in range(self.vsize):92 for j in range(self.vsize):
88 call('wmctrl -o %s,%s' % (self.xsize * j, self.ysize * i),93 subprocess.call(
89 shell=True)94 'wmctrl -o %s,%s' % (self.xsize * j, self.ysize * i),
95 shell=True)
90 time.sleep(0.5)96 time.sleep(0.5)
91 if self.cancel:97 if self.cancel:
92 # Switch back to viewport #198 # Switch back to viewport #1
93 call('wmctrl -o 0,0', shell=True)99 subprocess.call('wmctrl -o 0,0', shell=True)
94 return100 return
95101
96102
@@ -112,13 +118,18 @@
112118
113 with NamedTemporaryFile(suffix='.html', delete=False) as f:119 with NamedTemporaryFile(suffix='.html', delete=False) as f:
114 self.tempfile = f.name120 self.tempfile = f.name
115 f.write("%s" % source)121 f.write(source.encode())
116 f.close()122 f.close()
117 call('firefox %s' % f.name, stdout=open(os.devnull, 'w'),123 os.chmod(self.tempfile, stat.S_IROTH)
118 stderr=STDOUT, shell=True)124 subprocess.call(
125 'sudo -H -u %s firefox %s' % (os.getenv('SUDO_USER'),
126 self.tempfile),
127 stdout=open(os.devnull, 'w'),
128 stderr=subprocess.STDOUT,
129 shell=True)
119130
120 def terminate(self):131 def terminate(self):
121 call('wmctrl -c firefox', shell=True)132 subprocess.call('wmctrl -c firefox', shell=True)
122 os.unlink(self.tempfile)133 os.unlink(self.tempfile)
123134
124135
@@ -143,15 +154,19 @@
143 f = open('/var/log/kern.log', 'r')154 f = open('/var/log/kern.log', 'r')
144 with f:155 with f:
145 if re.findall(r'gpu\s+hung', f.read(), flags=re.I):156 if re.findall(r'gpu\s+hung', f.read(), flags=re.I):
146 print "GPU hung Detected"157 print("GPU hung Detected")
147 sys.exit(1)158 sys.exit(1)
148159
149160
150def main():161def main():
162 if not (os.geteuid() == 0):
163 print("Must be run as root.")
164 return 1
165
151 check_gpu()166 check_gpu()
152 GlxWindows = []167 GlxWindows = []
153 GlxRotate = []168 GlxRotate = []
154 call("pkill 'glxgears|firefox'", shell=True)169 subprocess.call("pkill 'glxgears|firefox'", shell=True)
155170
156 FlashVideo = FlashVideoThread()171 FlashVideo = FlashVideoThread()
157 FlashVideo.start()172 FlashVideo.start()
@@ -160,33 +175,39 @@
160 GlxWindows.append(GlxThread())175 GlxWindows.append(GlxThread())
161 GlxWindows[i].start()176 GlxWindows[i].start()
162 time.sleep(0.5)177 time.sleep(0.5)
163 windows = pexpect.run('wmctrl -l | grep glxgears')178 windows = subprocess.check_output(
179 'wmctrl -l | grep glxgears',
180 shell=True)
164 for app in sorted(windows.splitlines(), reverse=True):181 for app in sorted(windows.splitlines(), reverse=True):
165 if not 'glxgears' in app:182 if not b'glxgears' in app:
166 continue183 continue
167 GlxWindows[i].id = re.match(r'^(0x\w+)', app).group(0)184 GlxWindows[i].id = str(
185 re.match(b'^(0x\w+)', app).group(0), 'utf-8')
168 break186 break
169 GlxRotate.append(RotateGlxThread(GlxWindows[i].id, i + 1))187 GlxRotate.append(RotateGlxThread(GlxWindows[i].id, i + 1))
170 GlxRotate[i].start()188 GlxRotate[i].start()
171189
172 if is_unity_2d_running():190 if is_unity_2d_running():
173 size = int(pexpect.run(191 size = int(subprocess.check_output(
174 'gconftool --get /apps/metacity/general/num_workspaces'))192 'gconftool --get /apps/metacity/general/num_workspaces',
193 shell=True))
175 DesktopSwitch = ChangeWorkspace(size)194 DesktopSwitch = ChangeWorkspace(size)
176 else:195 else:
177 (x_res, y_res) = re.search(r'DG:\s+(\d+)x(\d+)',196 (x_res, y_res) = re.search(b'DG:\s+(\d+)x(\d+)',
178 pexpect.run('wmctrl -d')).groups()197 subprocess.check_output('wmctrl -d', shell=True)).groups()
179 hsize = int(pexpect.run(198 hsize = int(subprocess.check_output(
180 'gconftool --get /apps/compiz-1/general/screen0/options/hsize'))199 'gconftool --get /apps/compiz-1/general/screen0/options/hsize',
181 vsize = int(pexpect.run(200 shell=True))
182 'gconftool --get /apps/compiz-1/general/screen0/options/vsize'))201 vsize = int(subprocess.check_output(
202 'gconftool --get /apps/compiz-1/general/screen0/options/vsize',
203 shell=True))
183 DesktopSwitch = ChangeViewport(204 DesktopSwitch = ChangeViewport(
184 hsize, vsize, int(x_res) / hsize, int(y_res) / vsize)205 hsize, vsize, int(x_res) // hsize, int(y_res) // vsize)
185 DesktopSwitch.start()206 DesktopSwitch.start()
186207
187 time.sleep(20)208 time.sleep(20)
188 # Suspend/resume the SUT209 # Suspend/resume the SUT
189 call('fwts -q s3 --s3-sleep-delay=30', shell=True)210 subprocess.call('fwts -q s3 --s3-sleep-delay=30', shell=True)
190 time.sleep(20)211 time.sleep(20)
191212
192 for i in range(2):213 for i in range(2):

Subscribers

People subscribed via source and target branches