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
1=== modified file 'lib/canonical/launchpad/windmill/testing/widgets.py'
2--- lib/canonical/launchpad/windmill/testing/widgets.py 2009-08-18 13:22:07 +0000
3+++ lib/canonical/launchpad/windmill/testing/widgets.py 2009-10-29 14:50:28 +0000
4@@ -86,8 +86,9 @@
5 # And make sure it's actually saved on the server.
6 client.open(url=self.url)
7 client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
8- client.asserts.assertNode(
9- xpath=widget_base + '/span[1]')
10+ client.waits.forElement(
11+ xpath=widget_base + '/span[1]',
12+ timeout=constants.FOR_ELEMENT)
13 client.asserts.assertText(
14 xpath=widget_base + '/span[1]', validator=self.new_value)
15
16
17=== modified file 'lib/lp/code/windmill/tests/test_branch_index.py'
18--- lib/lp/code/windmill/tests/test_branch_index.py 2009-09-30 21:03:52 +0000
19+++ lib/lp/code/windmill/tests/test_branch_index.py 2009-10-29 14:50:28 +0000
20@@ -13,7 +13,7 @@
21 from windmill.authoring import WindmillTestClient
22
23 from canonical.launchpad.windmill.testing.constants import (
24- PAGE_LOAD, SLEEP)
25+ FOR_ELEMENT, PAGE_LOAD, SLEEP)
26 from canonical.launchpad.windmill.testing.lpuser import login_person
27 from lp.code.windmill.testing import CodeWindmillLayer
28 from lp.testing import TestCaseWithFactory
29@@ -33,14 +33,15 @@
30
31 client = WindmillTestClient("Branch status setting")
32
33- login_person(eric, "test", client)
34-
35 start_url = (
36 windmill.settings['TEST_URL'] + branch.unique_name)
37 client.open(url=start_url)
38 client.waits.forPageLoad(timeout=PAGE_LOAD)
39+ login_person(eric, "test", client)
40
41 # Click on the element containing the branch status.
42+ client.waits.forElement(
43+ id=u'branch-details-status-value', timeout=PAGE_LOAD)
44 client.click(id=u'branch-details-status-value')
45 client.waits.forElement(
46 xpath=u'//div[contains(@class, "yui-ichoicelist-content")]')
47@@ -55,6 +56,10 @@
48
49 # Reload the page and make sure the change sticks.
50 client.open(url=start_url)
51+ client.waits.forPageLoad(timeout=PAGE_LOAD)
52+ client.waits.forElement(
53+ xpath=u'//span[@id="branch-details-status-value"]/span',
54+ timeout=FOR_ELEMENT)
55 client.asserts.assertText(
56 xpath=u'//span[@id="branch-details-status-value"]/span',
57 validator=u'Experimental')
58
59=== modified file 'lib/lp/registry/windmill/tests/test_timeline_graph.py'
60--- lib/lp/registry/windmill/tests/test_timeline_graph.py 2009-10-09 22:08:12 +0000
61+++ lib/lp/registry/windmill/tests/test_timeline_graph.py 2009-10-29 14:50:28 +0000
62@@ -27,26 +27,21 @@
63 timeout=u'8000')
64 link_xpath = '//div/a[@href="/firefox/trunk"]'
65
66- # waits.forElement() is called multiple times because there
67- # were sporadic errors where waits.forElement() would succeed but
68- # the following assertNode() would fail 5% of the time.
69- for i in range(5):
70- self.client.waits.forElement(xpath=link_xpath)
71- self.client.asserts.assertNode(xpath=link_xpath)
72+ self.client.waits.forElement(xpath=link_xpath)
73
74 def test_project_timeline_graph(self):
75 """Test that the timeline graph loads on /$project page."""
76
77 self.client.open(url=u'http://launchpad.dev:8085/firefox')
78
79- self.client.waits.forElement(id=u'timeline-loading', timeout=u'20000')
80+ self.client.waits.forElementProperty(
81+ id=u'timeline-loading',
82+ option=u'style.display|none',
83+ timeout=u'20000')
84 self.client.waits.forElementProperty(
85 id=u'timeline-iframe',
86 option=u'style.display|block',
87 timeout=u'8000')
88- self.client.asserts.assertProperty(
89- id=u'timeline-loading',
90- validator=u'style.display|none')
91
92 def test_series_timeline_graph(self):
93 """Test that the timeline graph loads on /$project/$series page."""
94@@ -59,30 +54,23 @@
95 timeout=u'8000')
96 self.client.waits.forElement(id=u'timeline-loading', timeout=u'20000')
97
98- # waits.forElementProperty() is called multiple times because there
99- # were sporadic errors where waits.forElement() would succeed but
100- # the following assertProperty() would fail 5% of the time.
101- for i in range(5):
102- self.client.waits.forElementProperty(
103- id=u'timeline-loading',
104- option=u'style.display|none')
105- self.client.asserts.assertProperty(
106+ self.client.waits.forElementProperty(
107 id=u'timeline-loading',
108- validator=u'style.display|none')
109+ option=u'style.display|none')
110
111 def test_all_series_timeline_graph(self):
112 """Test that the timeline graph loads on /$project/+series page."""
113
114 self.client.open(url=u'http://launchpad.dev:8085/firefox/+series')
115
116- self.client.waits.forElement(id=u'timeline-loading', timeout=u'20000')
117+ self.client.waits.forElement(
118+ id=u'timeline-loading',
119+ option=u'style.display|none',
120+ timeout=u'20000')
121 self.client.waits.forElementProperty(
122 id=u'timeline-iframe',
123 option=u'style.display|block',
124 timeout=u'8000')
125- self.client.asserts.assertProperty(
126- id=u'timeline-loading',
127- validator=u'style.display|none')
128
129 def test_suite():
130 return unittest.TestLoader().loadTestsFromName(__name__)