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
1=== modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js'
2--- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-12-14 15:49:13 +0000
3+++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-12-22 15:31:25 +0000
4@@ -784,6 +784,12 @@
5 var html = Y.Node.create('<div><a></a></div>');
6 html.addClass(terms.css_name);
7
8+ if (subscription.is_direct_subscription()) {
9+ html.set('id', 'direct-' + terms.css_name);
10+ } else {
11+ html.set('id', 'dupe-' + terms.css_name);
12+ }
13+
14 html.one('a')
15 .set('href', '/~' + terms.name)
16 .set('name', terms.full_name)
17
18=== modified file 'lib/lp/bugs/templates/bug-portlet-subscribers-content.pt'
19--- lib/lp/bugs/templates/bug-portlet-subscribers-content.pt 2009-11-25 13:22:01 +0000
20+++ lib/lp/bugs/templates/bug-portlet-subscribers-content.pt 2009-12-22 15:31:25 +0000
21@@ -14,8 +14,9 @@
22 <div
23 tal:condition="direct_subscriptions"
24 tal:repeat="subscription direct_subscriptions"
25- tal:attributes="class subscription/css_name"
26- >
27+ tal:attributes="
28+ class subscription/css_name;
29+ id string:direct-${subscription/css_name}">
30 <metal:subscriber metal:define-macro="subscriber-row">
31
32 <a
33
34=== modified file 'lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py'
35--- lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py 2009-11-14 20:38:18 +0000
36+++ lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py 2009-12-22 15:31:25 +0000
37@@ -220,13 +220,9 @@
38 # Now back to bug 5. Confirm there are 2 subscriptions.
39 client.open(url=BUG_URL % 5)
40 client.asserts.assertNode(
41- xpath=(u'//div[@id="subscribers-links"]'
42- '/div/a[@name="Sample Person"]'),
43- timeout=FOR_ELEMENT)
44+ id='direct-subscriber-12', timeout=FOR_ELEMENT)
45 client.asserts.assertNode(
46- xpath=(u'//div[@id="subscribers-from-duplicates"]'
47- '/div/a[@name="Sample Person"]'),
48- timeout=FOR_ELEMENT)
49+ id='dupe-subscriber-12', timeout=FOR_ELEMENT)
50 # The first click unsubscribes the direct subscription, leaving
51 # the duplicate subscription.
52 client.click(xpath=SUBSCRIPTION_LINK)