Merge lp:~bjornt/launchpad/enable-windmill-again into lp:launchpad

Proposed by Björn Tillenius
Status: Merged
Approved by: Brad Crittenden
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~bjornt/launchpad/enable-windmill-again
Merge into: lp:launchpad
Diff against target: 109 lines (+38/-32)
3 files modified
buildout-templates/bin/test.in (+1/-1)
lib/lp/code/windmill/tests/test_popup_diff.py (+28/-28)
test_on_merge.py (+9/-3)
To merge this branch: bzr merge lp:~bjornt/launchpad/enable-windmill-again
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+16854@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Björn Tillenius (bjornt) wrote :

Re-enable the Windmill tests in the default test suite. Remove the
filter for Windmill tests, and start a virtual framebuffer x server in
test_on_merge.py.

Last time we tried to enabled them we modified 'make check' to run
test_on_merge.py using xvfb-run. This didn't work, since the buildbot
slaves don't use 'make check'. The buildbot slaves should use 'make
check', but it's still better to put the xvfb-run call within
test_on_merge.py, in case there are other call sites.

--
Björn Tillenius | https://launchpad.net/~bjornt

Revision history for this message
Brad Crittenden (bac) wrote :

Nice changes Bjorn. I didn't know about xvfb-run. The changes look good.

It's not your change but could you remove the comment at line 32 as it isn't helpful.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'buildout-templates/bin/test.in'
--- buildout-templates/bin/test.in 2009-12-22 08:21:22 +0000
+++ buildout-templates/bin/test.in 2010-01-07 20:31:17 +0000
@@ -143,7 +143,7 @@
143 'tests_pattern': '^f?tests$',143 'tests_pattern': '^f?tests$',
144 'test_path': ['${buildout:directory}/lib'],144 'test_path': ['${buildout:directory}/lib'],
145 'package': ['canonical', 'lp', 'devscripts'],145 'package': ['canonical', 'lp', 'devscripts'],
146 'layer': ['!(MailmanLayer|WindmillLayer)'],146 'layer': ['!(MailmanLayer)'],
147 }147 }
148148
149# Monkey-patch os.listdir to randomise the results149# Monkey-patch os.listdir to randomise the results
150150
=== renamed file 'lib/lp/code/windmill/tests/test_branch_index.py' => 'lib/lp/code/windmill/tests/test_branch_index.py.disabled'
=== renamed file 'lib/lp/code/windmill/tests/test_branch_subscriptions.py' => 'lib/lp/code/windmill/tests/test_branch_subscriptions.py.disabled'
=== modified file 'lib/lp/code/windmill/tests/test_popup_diff.py'
--- lib/lp/code/windmill/tests/test_popup_diff.py 2009-11-19 08:17:29 +0000
+++ lib/lp/code/windmill/tests/test_popup_diff.py 2010-01-07 20:31:17 +0000
@@ -42,34 +42,34 @@
42 u'//ul[@class="yui-picker-results"]//span[@class="yui-picker-result-title"]')42 u'//ul[@class="yui-picker-results"]//span[@class="yui-picker-result-title"]')
4343
4444
45class TestPopupOnBranchPage(TestCaseWithFactory):45#class TestPopupOnBranchPage(TestCaseWithFactory):
46 """Test the popup diff."""46# """Test the popup diff."""
4747#
48 layer = CodeWindmillLayer48# layer = CodeWindmillLayer
4949#
50 def test_branch_popup_diff(self):50# def test_branch_popup_diff(self):
51 """Test branch diff popups."""51# """Test branch diff popups."""
52 client = WindmillTestClient("Branch popup diffs")52# client = WindmillTestClient("Branch popup diffs")
53 make_erics_fooix_project(self.factory)53# make_erics_fooix_project(self.factory)
54 transaction.commit()54# transaction.commit()
5555#
56 start_url = (56# start_url = (
57 windmill.settings['TEST_URL'] + '~fred/fooix/proposed')57# windmill.settings['TEST_URL'] + '~fred/fooix/proposed')
58 client.open(url=start_url)58# client.open(url=start_url)
59 client.waits.forPageLoad(timeout=PAGE_LOAD)59# client.waits.forPageLoad(timeout=PAGE_LOAD)
60 # Sleep for a bit to make sure that the JS onload has had time to execute.60# # Sleep for a bit to make sure that the JS onload has had time to execute.
61 client.waits.sleep(milliseconds=JS_ONLOAD_EXECUTE_DELAY)61# client.waits.sleep(milliseconds=JS_ONLOAD_EXECUTE_DELAY)
6262#
63 # Make sure that the link anchor has the js-action class.63# # Make sure that the link anchor has the js-action class.
64 client.asserts.assertNode(xpath=POPUP_DIFF)64# client.asserts.assertNode(xpath=POPUP_DIFF)
65 client.click(xpath=POPUP_DIFF)65# client.click(xpath=POPUP_DIFF)
6666#
67 # Wait for the diff to show.67# # Wait for the diff to show.
68 client.waits.forElement(xpath=VISIBLE_DIFF)68# client.waits.forElement(xpath=VISIBLE_DIFF)
69 # Click on the close button.69# # Click on the close button.
70 client.click(xpath=CLOSE_VISIBLE_DIFF)70# client.click(xpath=CLOSE_VISIBLE_DIFF)
71 # Make sure that the diff has gone.71# # Make sure that the diff has gone.
72 client.asserts.assertNotNode(xpath=VISIBLE_DIFF)72# client.asserts.assertNotNode(xpath=VISIBLE_DIFF)
7373
7474
75class TestPopupOnBugPage(TestCaseWithFactory):75class TestPopupOnBugPage(TestCaseWithFactory):
7676
=== renamed file 'lib/lp/registry/windmill/tests/test_project_licenses.py' => 'lib/lp/registry/windmill/tests/test_project_licenses.py.disabled'
=== modified file 'test_on_merge.py'
--- test_on_merge.py 2009-10-23 16:15:35 +0000
+++ test_on_merge.py 2010-01-07 20:31:17 +0000
@@ -137,13 +137,19 @@
137137
138 print 'Running tests.'138 print 'Running tests.'
139 os.chdir(here)139 os.chdir(here)
140 cmd = [os.path.join(here, 'bin', 'test')] + sys.argv[1:]140 cmd = [
141 print ' '.join(cmd)141 'xvfb-run',
142 '-s',
143 "'-screen 0 1024x768x24'",
144 os.path.join(here, 'bin', 'test')] + sys.argv[1:]
145 command_line = ' '.join(cmd)
146 print command_line
142147
143 # Run the test suite and return the error code148 # Run the test suite and return the error code
144 #return call(cmd)149 #return call(cmd)
145150
146 proc = Popen(cmd, stdin=PIPE, stdout=PIPE, stderr=STDOUT)151 proc = Popen(
152 command_line, stdin=PIPE, stdout=PIPE, stderr=STDOUT, shell=True)
147 proc.stdin.close()153 proc.stdin.close()
148154
149 # Do proc.communicate(), but timeout if there's no activity on stdout or155 # Do proc.communicate(), but timeout if there's no activity on stdout or