Merge lp:~rvb/launchpad/fix-missing-addseries-link into lp:launchpad

Proposed by Raphaël Badin on 2011-03-14
Status: Merged
Approved by: Henning Eggers on 2011-03-15
Approved revision: no longer in the source branch.
Merged at revision: 12597
Proposed branch: lp:~rvb/launchpad/fix-missing-addseries-link
Merge into: lp:launchpad
Diff against target: 214 lines (+93/-89)
2 files modified
lib/lp/registry/browser/tests/test_distribution.py (+90/-0)
lib/lp/registry/tests/test_distribution.py (+3/-89)
To merge this branch: bzr merge lp:~rvb/launchpad/fix-missing-addseries-link
Reviewer Review Type Date Requested Status
Henning Eggers (community) code 2011-03-14 Approve on 2011-03-14
Review via email: mp+53219@code.launchpad.net

Commit message

[incr] [r=henninge][bug=728515,734772] Fix a few tests for distribution (moved to page tests and remove the use of sampledata).

Description of the change

Trivial refactoring of the tests for a distribution:
- removed the use of sampledata
- moved page tests in browser/tests (where they belong)

{{{
./bin/test -cvv test_distribution test_distributionpage_series_list_noadmin
./bin/test -cvv test_distribution test_distributionpage_addseries_link_noadmin
./bin/test -cvv test_distribution test_distributionpage_addseries_link
}}}

To post a comment you must log in.
Henning Eggers (henninge) wrote :

Hi Raphael,
thanks for this nice little clean up, this test really did not belong there ... ;-) Also, I did not know about soupmatchers. I will try them out on the next chance.

While you are moving this test, can you please fix some formatting issues that it already had? They are easy enough to do, so I will approve this branch with the assumption that you will take care of these issues.

- The import section should follow our conventions but luckily you have "utilities/format-imports" to do it for you. Have a look at the docstring of that file to find out more.
- The docstrings on the tests don't follow our conventions either but for these we make life easy by simply not putting docstrings on test methods. Just convert them to comments.
- Also, starting test comments with "Verify that" or "Test that" is stating the obvious and therefore redundant. Please rephrase that, it's actually much simpler. Example:

    def test_distributionpage_addseries_link_noadmin(self):
        # A non-admin does not see the +addseries link nor the series
        # header (since there is no series yet).

Finally, please run "make lint" to see what that has to say about the file. ;-)

Cheers,
Henning

review: Approve (code)
Raphaël Badin (rvb) wrote :

Fixed, thanks for the careful review.

Henning Eggers (henninge) wrote :

Am 14.03.2011 13:29, schrieb Raphaël Victor Badin:
> Fixed, thanks for the careful review.

You are welcome! You forgot to fix the imports, though. ;-) Here is what the
format-imports utility script came up with.

1=== modified file 'lib/lp/registry/browser/tests/test_distribution.py'
2--- lib/lp/registry/browser/tests/test_distribution.py 2011-03-14 12:22:17 +0000
3+++ lib/lp/registry/browser/tests/test_distribution.py 2011-03-14 13:51:27 +0000
4@@ -4,9 +4,14 @@
5
6 __metaclass__ = type
7
8+import soupmatchers
9+from testtools.matchers import (
10+ MatchesAny,
11+ Not,
12+ )
13+
14 from canonical.launchpad.webapp import canonical_url
15 from canonical.testing.layers import DatabaseFunctionalLayer
16-
17 from lp.registry.interfaces.series import SeriesStatus
18 from lp.testing import (
19 login_celebrity,
20@@ -15,11 +20,6 @@
21 )
22 from lp.testing.views import create_initialized_view
23
24-from testtools.matchers import MatchesAny
25-from testtools.matchers import Not
26-
27-import soupmatchers
28-
29
30 class TestDistributionPage(TestCaseWithFactory):
31 """A TestCase for the distribution index page."""
Raphaël Badin (rvb) wrote :

Oops sorry about that, this is done now.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'lib/lp/registry/browser/tests/test_distribution.py'
2--- lib/lp/registry/browser/tests/test_distribution.py 1970-01-01 00:00:00 +0000
3+++ lib/lp/registry/browser/tests/test_distribution.py 2011-03-14 14:32:57 +0000
4@@ -0,0 +1,90 @@
5+# GNU Affero General Public License version 3 (see the file LICENSE).
6+
7+"""Tests for Distribution page."""
8+
9+__metaclass__ = type
10+
11+import soupmatchers
12+from testtools.matchers import (
13+ MatchesAny,
14+ Not,
15+ )
16+
17+from canonical.launchpad.webapp import canonical_url
18+from canonical.testing.layers import DatabaseFunctionalLayer
19+from lp.registry.interfaces.series import SeriesStatus
20+from lp.testing import (
21+ login_celebrity,
22+ login_person,
23+ TestCaseWithFactory,
24+ )
25+from lp.testing.views import create_initialized_view
26+
27+
28+class TestDistributionPage(TestCaseWithFactory):
29+ """A TestCase for the distribution index page."""
30+
31+ layer = DatabaseFunctionalLayer
32+
33+ def setUp(self):
34+ super(TestDistributionPage, self).setUp()
35+ self.distro = self.factory.makeDistribution(
36+ name="distro", displayname=u'distro')
37+ self.simple_user = self.factory.makePerson()
38+
39+ def test_distributionpage_addseries_link(self):
40+ # An admin sees the +addseries link.
41+ self.admin = login_celebrity('admin')
42+ view = create_initialized_view(
43+ self.distro, '+index', principal=self.admin)
44+ series_matches = soupmatchers.HTMLContains(
45+ soupmatchers.Tag(
46+ 'link to add a series', 'a',
47+ attrs={'href':
48+ canonical_url(self.distro, view_name='+addseries')},
49+ text='Add series'),
50+ soupmatchers.Tag(
51+ 'Series and milestones widget', 'h2',
52+ text='Series and milestones'),
53+ )
54+ self.assertThat(view.render(), series_matches)
55+
56+ def test_distributionpage_addseries_link_noadmin(self):
57+ # A non-admin does not see the +addseries link nor the series
58+ # header (since there is no series yet).
59+ login_person(self.simple_user)
60+ view = create_initialized_view(
61+ self.distro, '+index', principal=self.simple_user)
62+ add_series_match = soupmatchers.HTMLContains(
63+ soupmatchers.Tag(
64+ 'link to add a series', 'a',
65+ attrs={'href':
66+ canonical_url(self.distro, view_name='+addseries')},
67+ text='Add series'))
68+ series_header_match = soupmatchers.HTMLContains(
69+ soupmatchers.Tag(
70+ 'Series and milestones widget', 'h2',
71+ text='Series and milestones'))
72+ self.assertThat(
73+ view.render(),
74+ Not(MatchesAny(add_series_match, series_header_match)))
75+
76+ def test_distributionpage_series_list_noadmin(self):
77+ # A non-admin does see the series list when there is a series.
78+ series = self.factory.makeDistroSeries(distribution=self.distro,
79+ status=SeriesStatus.CURRENT)
80+ login_person(self.simple_user)
81+ view = create_initialized_view(
82+ self.distro, '+index', principal=self.simple_user)
83+ add_series_match = soupmatchers.HTMLContains(
84+ soupmatchers.Tag(
85+ 'link to add a series', 'a',
86+ attrs={'href':
87+ canonical_url(self.distro, view_name='+addseries')},
88+ text='Add series'))
89+ series_header_match = soupmatchers.HTMLContains(
90+ soupmatchers.Tag(
91+ 'Series and milestones widget', 'h2',
92+ text='Series and milestones'))
93+ self.assertThat(view.render(), series_header_match)
94+ self.assertThat(view.render(), Not(add_series_match))
95
96=== modified file 'lib/lp/registry/tests/test_distribution.py'
97--- lib/lp/registry/tests/test_distribution.py 2011-03-03 15:45:58 +0000
98+++ lib/lp/registry/tests/test_distribution.py 2011-03-14 14:32:57 +0000
99@@ -5,17 +5,15 @@
100
101 __metaclass__ = type
102
103-from canonical.launchpad.webapp import canonical_url
104+from lazr.lifecycle.snapshot import Snapshot
105+from zope.security.proxy import removeSecurityProxy
106+
107 from canonical.testing.layers import (
108 DatabaseFunctionalLayer,
109 LaunchpadFunctionalLayer,
110 )
111-
112-from lazr.lifecycle.snapshot import Snapshot
113-
114 from lp.registry.errors import NoSuchDistroSeries
115 from lp.registry.interfaces.distribution import IDistribution
116-from lp.registry.interfaces.person import IPersonSet
117 from lp.registry.interfaces.series import SeriesStatus
118 from lp.registry.tests.test_distroseries import (
119 TestDistroSeriesCurrentSourceReleases,
120@@ -25,16 +23,6 @@
121 IDistributionSourcePackageRelease,
122 )
123 from lp.testing import TestCaseWithFactory
124-from lp.testing import login_person
125-from lp.testing.views import create_initialized_view
126-
127-from testtools.matchers import MatchesAny
128-from testtools.matchers import Not
129-
130-import soupmatchers
131-
132-from zope.component import getUtility
133-from zope.security.proxy import removeSecurityProxy
134
135
136 class TestDistribution(TestCaseWithFactory):
137@@ -186,77 +174,3 @@
138 self.assertFalse(
139 hasattr(snapshot, attribute),
140 "Snapshot should not include %s." % attribute)
141-
142-
143-class TestDistributionPage(TestCaseWithFactory):
144- """A TestCase for the distribution page."""
145-
146- layer = DatabaseFunctionalLayer
147-
148- def setUp(self):
149- super(TestDistributionPage, self).setUp('foo.bar@canonical.com')
150- self.distro = self.factory.makeDistribution(
151- name="distro", displayname=u'distro')
152- self.admin = getUtility(IPersonSet).getByEmail(
153- 'admin@canonical.com')
154- self.simple_user = self.factory.makePerson()
155-
156- def test_distributionpage_addseries_link(self):
157- """ Verify that an admin sees the +addseries link."""
158- login_person(self.admin)
159- view = create_initialized_view(
160- self.distro, '+index', principal=self.admin)
161- series_matches = soupmatchers.HTMLContains(
162- soupmatchers.Tag(
163- 'link to add a series', 'a',
164- attrs={'href':
165- canonical_url(self.distro, view_name='+addseries')},
166- text='Add series'),
167- soupmatchers.Tag(
168- 'Series and milestones widget', 'h2',
169- text='Series and milestones'),
170- )
171- self.assertThat(view.render(), series_matches)
172-
173- def test_distributionpage_addseries_link_noadmin(self):
174- """Verify that a non-admin does not see the +addseries link
175- nor the series header (since there is no series yet).
176- """
177- login_person(self.simple_user)
178- view = create_initialized_view(
179- self.distro, '+index', principal=self.simple_user)
180- add_series_match = soupmatchers.HTMLContains(
181- soupmatchers.Tag(
182- 'link to add a series', 'a',
183- attrs={'href':
184- canonical_url(self.distro, view_name='+addseries')},
185- text='Add series'))
186- series_header_match = soupmatchers.HTMLContains(
187- soupmatchers.Tag(
188- 'Series and milestones widget', 'h2',
189- text='Series and milestones'))
190- self.assertThat(
191- view.render(),
192- Not(MatchesAny(add_series_match, series_header_match)))
193-
194- def test_distributionpage_series_list_noadmin(self):
195- """Verify that a non-admin does see the series list
196- when there is a series.
197- """
198- series = self.factory.makeDistroSeries(distribution=self.distro,
199- status=SeriesStatus.CURRENT)
200- login_person(self.simple_user)
201- view = create_initialized_view(
202- self.distro, '+index', principal=self.simple_user)
203- add_series_match = soupmatchers.HTMLContains(
204- soupmatchers.Tag(
205- 'link to add a series', 'a',
206- attrs={'href':
207- canonical_url(self.distro, view_name='+addseries')},
208- text='Add series'))
209- series_header_match = soupmatchers.HTMLContains(
210- soupmatchers.Tag(
211- 'Series and milestones widget', 'h2',
212- text='Series and milestones'))
213- self.assertThat(view.render(), series_header_match)
214- self.assertThat(view.render(), Not(add_series_match))