Launchpad itself

Merge lp:~deryck/launchpad/fix-subscriber-windmill-fragility-497112 into lp:launchpad

  1. fix-subscriber-windmill-fragility-497112
  2. Merge into devel
Proposed by Deryck Hodge
Status: Merged
Approved by: Deryck Hodge
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~deryck/launchpad/fix-subscriber-windmill-fragility-497112
Merge into: lp:launchpad
Diff against target: 52 lines (+11/-8)
3 files modified
lib/canonical/launchpad/javascript/bugs/bugtask-index.js (+6/-0)
lib/lp/bugs/templates/bug-portlet-subscribers-content.pt (+3/-2)
lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py (+2/-6)
To merge this branch: bzr merge lp:~deryck/launchpad/fix-subscriber-windmill-fragility-497112
Reviewer Review Type Date Requested Status
Björn Tillenius (community) code Approve
Review via email: mp+16431@code.launchpad.net

Commit message

Make the inline subscribing Windmill test less fragile by using IDs instead of xpath.

To post a comment you must log in.
Revision history for this message
Deryck Hodge (deryck) wrote :

This branch attempts to fix the bug subscribing Windmill test failure
reported in bug 497112. I say *attempts* because the error only happens
when running the entire Windmill suite and hasn't happened since I
recorded the bug. I can't reproduce this failure.

However, this section of the test could be made less fragile by relying
on IDs rather than xpath to find elements.

So this branch creates an id for direct subscribers and makes the test
use IDs for the direct and dupe subscriber part of the subscribe someone
else section of the test (which was the part that failed).

To test:

./bin/test --layer=BugsWindmillLayer -vvt test_bug_inline_subscriber

A couple of additional notes:

1) I focused only on the reported failure area, and I am working on a
branch to clean up the test generally to use these IDs.

2) the official-bug-tags js script is reporting lint errors, which is an
unrelated bit of the code. I have fixed this in a separate drive-by
branch and will put that up for review shortly.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/bugs/templates/bug-portlet-subscribers-content.pt
  lib/canonical/launchpad/javascript/bugs/bugtask-index.js
  lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py

Revision history for this message
Björn Tillenius (bjornt) wrote :

What does css_name look like? Is it unique for each subscription? I suspect that it is, so I'll approve this branch on condition that it is. Relying on ids is better than using xpath.

review: Approve (code)
Revision history for this message
Deryck Hodge (deryck) wrote :

Yes, css_name is unique. As mentioned on IRC, it's of the form "subscriber-XXX" where XXX is the DB id of the subscribed user.

Cheers,
deryck

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js'
--- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-12-14 15:49:13 +0000
+++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-12-22 15:31:25 +0000
@@ -784,6 +784,12 @@
784 var html = Y.Node.create('<div><a></a></div>');784 var html = Y.Node.create('<div><a></a></div>');
785 html.addClass(terms.css_name);785 html.addClass(terms.css_name);
786786
787 if (subscription.is_direct_subscription()) {
788 html.set('id', 'direct-' + terms.css_name);
789 } else {
790 html.set('id', 'dupe-' + terms.css_name);
791 }
792
787 html.one('a')793 html.one('a')
788 .set('href', '/~' + terms.name)794 .set('href', '/~' + terms.name)
789 .set('name', terms.full_name)795 .set('name', terms.full_name)
790796
=== modified file 'lib/lp/bugs/templates/bug-portlet-subscribers-content.pt'
--- lib/lp/bugs/templates/bug-portlet-subscribers-content.pt 2009-11-25 13:22:01 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscribers-content.pt 2009-12-22 15:31:25 +0000
@@ -14,8 +14,9 @@
14 <div14 <div
15 tal:condition="direct_subscriptions"15 tal:condition="direct_subscriptions"
16 tal:repeat="subscription direct_subscriptions"16 tal:repeat="subscription direct_subscriptions"
17 tal:attributes="class subscription/css_name"17 tal:attributes="
18 >18 class subscription/css_name;
19 id string:direct-${subscription/css_name}">
19 <metal:subscriber metal:define-macro="subscriber-row">20 <metal:subscriber metal:define-macro="subscriber-row">
2021
21 <a22 <a
2223
=== modified file 'lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py'
--- lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py 2009-11-14 20:38:18 +0000
+++ lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py 2009-12-22 15:31:25 +0000
@@ -220,13 +220,9 @@
220 # Now back to bug 5. Confirm there are 2 subscriptions.220 # Now back to bug 5. Confirm there are 2 subscriptions.
221 client.open(url=BUG_URL % 5)221 client.open(url=BUG_URL % 5)
222 client.asserts.assertNode(222 client.asserts.assertNode(
223 xpath=(u'//div[@id="subscribers-links"]'223 id='direct-subscriber-12', timeout=FOR_ELEMENT)
224 '/div/a[@name="Sample Person"]'),
225 timeout=FOR_ELEMENT)
226 client.asserts.assertNode(224 client.asserts.assertNode(
227 xpath=(u'//div[@id="subscribers-from-duplicates"]'225 id='dupe-subscriber-12', timeout=FOR_ELEMENT)
228 '/div/a[@name="Sample Person"]'),
229 timeout=FOR_ELEMENT)
230 # The first click unsubscribes the direct subscription, leaving226 # The first click unsubscribes the direct subscription, leaving
231 # the duplicate subscription.227 # the duplicate subscription.
232 client.click(xpath=SUBSCRIPTION_LINK)228 client.click(xpath=SUBSCRIPTION_LINK)