>
>
Inactive bug trackers
>
> + trackers view/inactive_trackers">
>
>
>
Can I just say that this template is a massive hack? I don't think
you've made it any worse though.
> === modified file 'lib/lp/bugs/tests/test_bugtracker.py'
> --- lib/lp/bugs/tests/test_bugtracker.py 2010-08-26 20:08:43 +0000
> +++ lib/lp/bugs/tests/test_bugtracker.py 2010-09-19 22:21:05 +0000
> @@ -28,7 +28,10 @@
>
> from canonical.launchpad.ftests import login_person
> from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
> -from canonical.testing import LaunchpadFunctionalLayer
> +from canonical.testing import (
> + DatabaseFunctionalLayer,
> + LaunchpadFunctionalLayer,
> + )
> from lp.bugs.externalbugtracker import (
> BugTrackerConnectError,
> Mantis,
> @@ -38,12 +41,36 @@
> BugTrackerType,
> IBugTracker,
> )
> +from lp.bugs.model.bugtracker import BugTrackerSet
> from lp.bugs.tests.externalbugtracker import Urlib2TransportTestHandler
> from lp.registry.interfaces.person import IPersonSet
> from lp.testing import login, login_person, TestCaseWithFactory
> from lp.testing.sampledata import ADMIN_EMAIL
>
>
> +class TestBugTrackerSet(TestCaseWithFactory):
> +
> + layer = DatabaseFunctionalLayer
> +
> + def test_trackers(self):
> + tracker = self.factory.makeBugTracker()
> + trackers = BugTrackerSet()
> + # Active trackers are in all trackers,
> + self.assertTrue(tracker in trackers.trackers())
> + # and active,
> + self.assertTrue(tracker in trackers.trackers(active=True))
> + # But not inactive.
> + self.assertFalse(tracker in trackers.trackers(active=False))
> + login(ADMIN_EMAIL)
> + tracker.active = False
> + # Inactive trackers are in all trackers
> + self.assertTrue(tracker in trackers.trackers())
> + # and inactive,
> + self.assertTrue(tracker in trackers.trackers(active=False))
> + # but not in active.
> + self.assertFalse(tracker in trackers.trackers(active=True))
I kind of hate tests with lots of assertions like this, and it's a
shame that makeBugTracker doesn't take an active= argument, but I
don't think I'm going to insist you change anything here...
> class BugTrackerTestCase(TestCaseWithFactory):
> """Unit tests for the `BugTracker` class."""
>
> === modified file 'lib/lp/testing/matchers.py'
> --- lib/lp/testing/matchers.py 2010-08-25 20:04:40 +0000
> +++ lib/lp/testing/matchers.py 2010-09-19 22:21:05 +0000
> @@ -17,9 +17,12 @@
> from testtools.content import Content
> from testtools.content_type import UTF8_TEXT
> from testtools.matchers import (
> + Equals,
> Matcher,
> Mismatch,
> + MismatchesAll,
> )
> +from testtools import matchers
> from zope.interface.exceptions import (
> BrokenImplementation,
> BrokenMethodImplementation,
> @@ -31,6 +34,8 @@
> Proxy,
> )
>
> +from canonical.launchpad.webapp.batching import BatchNavigator
> +
>
> class DoesNotProvide(Mismatch):
> """An object does not provide an interface."""
> @@ -220,3 +225,44 @@
> if not matchee.startswith(self.expected):
> return DoesNotStartWith(matchee, self.expected)
> return None
> +
> +
> +class IsConfiguredBatchNavigator(Matcher):
> + """Check that an object is a batch navigator."""
> +
> + def __init__(self, singular, plural, batch_size=None):
> + """Create a ConfiguredBatchNavigator.
> +
> + :param singular: The singular header the batch should be using.
> + :param plural: The plural header the batch should be using.
Can you document the batch_size parameter?
> + """
> + self._single = Equals(singular)
> + self._plural = Equals(plural)
> + self._batch = None
> + if batch_size:
> + self._batch = Equals(batch_size)
> + self.matchers = dict(
> + _singular_heading=self._single, _plural_heading=self._plural)
> + if self._batch:
> + self.matchers['default_size'] = self._batch
> +
> + def __str__(self):
> + if self._batch:
> + batch = ", %r" % self._batch.expected
> + else:
> + batch = ''
> + return "ConfiguredBatchNavigator(%r, %r%s)" % (
> + self._single.expected, self._plural.expected, batch)
> +
> + def match(self, matchee):
> + if not isinstance(matchee, BatchNavigator):
> + # Testtools doesn't have an IsInstanceMismatch yet.
> + return matchers._BinaryMismatch(
> + BatchNavigator, 'isinstance', matchee)
> + mismatches = []
> + for attrname, matcher in self.matchers.items():
> + mismatches.append(matcher.match(getattr(matchee, attrname)))
> + if not mismatches[-1]:
> + mismatches.pop(-1)
On the theory that positive conditions are easier to understand, can
you please rewrite this as:
for attrname, matcher in self.matchers.items():
mismatch = matcher.match(getattr(matchee, attrname))
if mismatch is not None:
mismatches.append(mismatch)
> + if mismatches:
> + return MismatchesAll(mismatches)
This is a nice matcher though, the tests that you changed to use it
have become much nicer :-)