Launchpad itself

Merge lp:~bjornt/launchpad/windmill-more-problems into lp:launchpad

  1. windmill-more-problems
  2. Merge into devel
Proposed by Björn Tillenius
Status: Merged
Merged at revision: not available
Proposed branch: lp:~bjornt/launchpad/windmill-more-problems
Merge into: lp:launchpad
Diff against target: 130 lines
3 files modified
lib/canonical/launchpad/windmill/testing/widgets.py (+3/-2)
lib/lp/code/windmill/tests/test_branch_index.py (+8/-3)
lib/lp/registry/windmill/tests/test_timeline_graph.py (+11/-23)
To merge this branch: bzr merge lp:~bjornt/launchpad/windmill-more-problems
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+14172@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Björn Tillenius (bjornt) wrote :

Some more fixes to Windmill tests. The whole suite is now passing
locally on my box.

The bulk of the changes is to wait for elements before doing anything
with them. I've also change a few places where we first wait for the
element, and then assert the exact thing. This isn't necessary, since
the wait is basically an assert with a timeout.

There is some bug in Windmill, though, that if you first wait, and then
assert the element directly after, it sometimes fails. That's why I've
change a lot of code to wait only, instead of trying to assert it. I
suspect that this is a race condition, in that waiting for an element
listens to some dom-modified event, which gets fired out slightly before
the dom has actually been updated, so the dom might not be ready yet
when the assertion happens. I'll look further into this to see what I
can find out, but we should be able to trust that waiting for an element
works.

And after all, it's a lot better to have an occasional 'spurious
success' than a spurious failure.

I also change one test to log in after loading a page, rather than
before. I've seen some strange error where the login succeeds, but then
when you open another page the user is logged out again.

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

Revision history for this message
Abel Deuring (adeuring) wrote :

> And after all, it's a lot better to have an occasional 'spurious
> success' than a spurious failure.

yeah... at least for fragile test environments ;)

r=me

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/windmill/testing/widgets.py'
--- lib/canonical/launchpad/windmill/testing/widgets.py 2009-08-18 13:22:07 +0000
+++ lib/canonical/launchpad/windmill/testing/widgets.py 2009-10-29 14:50:28 +0000
@@ -86,8 +86,9 @@
86 # And make sure it's actually saved on the server.86 # And make sure it's actually saved on the server.
87 client.open(url=self.url)87 client.open(url=self.url)
88 client.waits.forPageLoad(timeout=constants.PAGE_LOAD)88 client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
89 client.asserts.assertNode(89 client.waits.forElement(
90 xpath=widget_base + '/span[1]')90 xpath=widget_base + '/span[1]',
91 timeout=constants.FOR_ELEMENT)
91 client.asserts.assertText(92 client.asserts.assertText(
92 xpath=widget_base + '/span[1]', validator=self.new_value)93 xpath=widget_base + '/span[1]', validator=self.new_value)
9394
9495
=== modified file 'lib/lp/code/windmill/tests/test_branch_index.py'
--- lib/lp/code/windmill/tests/test_branch_index.py 2009-09-30 21:03:52 +0000
+++ lib/lp/code/windmill/tests/test_branch_index.py 2009-10-29 14:50:28 +0000
@@ -13,7 +13,7 @@
13from windmill.authoring import WindmillTestClient13from windmill.authoring import WindmillTestClient
1414
15from canonical.launchpad.windmill.testing.constants import (15from canonical.launchpad.windmill.testing.constants import (
16 PAGE_LOAD, SLEEP)16 FOR_ELEMENT, PAGE_LOAD, SLEEP)
17from canonical.launchpad.windmill.testing.lpuser import login_person17from canonical.launchpad.windmill.testing.lpuser import login_person
18from lp.code.windmill.testing import CodeWindmillLayer18from lp.code.windmill.testing import CodeWindmillLayer
19from lp.testing import TestCaseWithFactory19from lp.testing import TestCaseWithFactory
@@ -33,14 +33,15 @@
3333
34 client = WindmillTestClient("Branch status setting")34 client = WindmillTestClient("Branch status setting")
3535
36 login_person(eric, "test", client)
37
38 start_url = (36 start_url = (
39 windmill.settings['TEST_URL'] + branch.unique_name)37 windmill.settings['TEST_URL'] + branch.unique_name)
40 client.open(url=start_url)38 client.open(url=start_url)
41 client.waits.forPageLoad(timeout=PAGE_LOAD)39 client.waits.forPageLoad(timeout=PAGE_LOAD)
40 login_person(eric, "test", client)
4241
43 # Click on the element containing the branch status.42 # Click on the element containing the branch status.
43 client.waits.forElement(
44 id=u'branch-details-status-value', timeout=PAGE_LOAD)
44 client.click(id=u'branch-details-status-value')45 client.click(id=u'branch-details-status-value')
45 client.waits.forElement(46 client.waits.forElement(
46 xpath=u'//div[contains(@class, "yui-ichoicelist-content")]')47 xpath=u'//div[contains(@class, "yui-ichoicelist-content")]')
@@ -55,6 +56,10 @@
5556
56 # Reload the page and make sure the change sticks.57 # Reload the page and make sure the change sticks.
57 client.open(url=start_url)58 client.open(url=start_url)
59 client.waits.forPageLoad(timeout=PAGE_LOAD)
60 client.waits.forElement(
61 xpath=u'//span[@id="branch-details-status-value"]/span',
62 timeout=FOR_ELEMENT)
58 client.asserts.assertText(63 client.asserts.assertText(
59 xpath=u'//span[@id="branch-details-status-value"]/span',64 xpath=u'//span[@id="branch-details-status-value"]/span',
60 validator=u'Experimental')65 validator=u'Experimental')
6166
=== modified file 'lib/lp/registry/windmill/tests/test_timeline_graph.py'
--- lib/lp/registry/windmill/tests/test_timeline_graph.py 2009-10-09 22:08:12 +0000
+++ lib/lp/registry/windmill/tests/test_timeline_graph.py 2009-10-29 14:50:28 +0000
@@ -27,26 +27,21 @@
27 timeout=u'8000')27 timeout=u'8000')
28 link_xpath = '//div/a[@href="/firefox/trunk"]'28 link_xpath = '//div/a[@href="/firefox/trunk"]'
2929
30 # waits.forElement() is called multiple times because there30 self.client.waits.forElement(xpath=link_xpath)
31 # were sporadic errors where waits.forElement() would succeed but
32 # the following assertNode() would fail 5% of the time.
33 for i in range(5):
34 self.client.waits.forElement(xpath=link_xpath)
35 self.client.asserts.assertNode(xpath=link_xpath)
3631
37 def test_project_timeline_graph(self):32 def test_project_timeline_graph(self):
38 """Test that the timeline graph loads on /$project page."""33 """Test that the timeline graph loads on /$project page."""
3934
40 self.client.open(url=u'http://launchpad.dev:8085/firefox')35 self.client.open(url=u'http://launchpad.dev:8085/firefox')
4136
42 self.client.waits.forElement(id=u'timeline-loading', timeout=u'20000')37 self.client.waits.forElementProperty(
38 id=u'timeline-loading',
39 option=u'style.display|none',
40 timeout=u'20000')
43 self.client.waits.forElementProperty(41 self.client.waits.forElementProperty(
44 id=u'timeline-iframe',42 id=u'timeline-iframe',
45 option=u'style.display|block',43 option=u'style.display|block',
46 timeout=u'8000')44 timeout=u'8000')
47 self.client.asserts.assertProperty(
48 id=u'timeline-loading',
49 validator=u'style.display|none')
5045
51 def test_series_timeline_graph(self):46 def test_series_timeline_graph(self):
52 """Test that the timeline graph loads on /$project/$series page."""47 """Test that the timeline graph loads on /$project/$series page."""
@@ -59,30 +54,23 @@
59 timeout=u'8000')54 timeout=u'8000')
60 self.client.waits.forElement(id=u'timeline-loading', timeout=u'20000')55 self.client.waits.forElement(id=u'timeline-loading', timeout=u'20000')
6156
62 # waits.forElementProperty() is called multiple times because there57 self.client.waits.forElementProperty(
63 # were sporadic errors where waits.forElement() would succeed but
64 # the following assertProperty() would fail 5% of the time.
65 for i in range(5):
66 self.client.waits.forElementProperty(
67 id=u'timeline-loading',
68 option=u'style.display|none')
69 self.client.asserts.assertProperty(
70 id=u'timeline-loading',58 id=u'timeline-loading',
71 validator=u'style.display|none')59 option=u'style.display|none')
7260
73 def test_all_series_timeline_graph(self):61 def test_all_series_timeline_graph(self):
74 """Test that the timeline graph loads on /$project/+series page."""62 """Test that the timeline graph loads on /$project/+series page."""
7563
76 self.client.open(url=u'http://launchpad.dev:8085/firefox/+series')64 self.client.open(url=u'http://launchpad.dev:8085/firefox/+series')
7765
78 self.client.waits.forElement(id=u'timeline-loading', timeout=u'20000')66 self.client.waits.forElement(
67 id=u'timeline-loading',
68 option=u'style.display|none',
69 timeout=u'20000')
79 self.client.waits.forElementProperty(70 self.client.waits.forElementProperty(
80 id=u'timeline-iframe',71 id=u'timeline-iframe',
81 option=u'style.display|block',72 option=u'style.display|block',
82 timeout=u'8000')73 timeout=u'8000')
83 self.client.asserts.assertProperty(
84 id=u'timeline-loading',
85 validator=u'style.display|none')
8674
87def test_suite():75def test_suite():
88 return unittest.TestLoader().loadTestsFromName(__name__)76 return unittest.TestLoader().loadTestsFromName(__name__)