Code review comment for lp:~bac/launchpad/bug-753965

We discussed the accidentally-included message argument on line 76 of
the diff in IRC.

I would add failure messages to the asserts inside your custom assert
methods: assertOldLinkMissing, assertOldLinkPresent,
assertNewLinksMissing, assertNewLinksPresent.

Skipping DistroView.setUp on line 345 of the diff and calling
BrowserTestCase.setUp instead confused me. If I replace it with a call
to DistroView.setUp the tests still pass.

Also, you didn't start it, but the manual test suite construction in
test_suite is kinda ugly and brittle (a test could easily be dropped
from the list and not noticed). Here's a change that uses the standard
unittest test loader but ignores test cases from the base class that we
want to avoid (also available at http://pastebin.ubuntu.com/593155/).

=== modified file 'lib/lp/registry/browser/tests/test_subscription_links.py'
--- lib/lp/registry/browser/tests/test_subscription_links.py 2011-04-12 14:49:43 +0000
+++ lib/lp/registry/browser/tests/test_subscription_links.py 2011-04-12 14:49:49 +0000
@@ -855,40 +855,19 @@
         self.assertNewLinksMissing()

+class CustomTestLoader(unittest.TestLoader):
+ """A test loader that avoids running tests from a base class."""
+
+ def getTestCaseNames(self, testCaseClass):
+ # If we're asked about which tests to run for _TestStructSubs, reply
+ # with an empty list.
+ if testCaseClass is _TestStructSubs:
+ return []
+ else:
+ return super(CustomTestLoader, self).getTestCaseNames(
+ testCaseClass)
+
+
 def test_suite():
     """Return the `IStructuralSubscriptionTarget` TestSuite."""
-
- # Manually construct the test suite to avoid having tests from the base
- # class _TestStructSubs run.
- suite = unittest.TestSuite()
- suite.addTest(unittest.makeSuite(ProductView))
- suite.addTest(unittest.makeSuite(ProductBugs))
- suite.addTest(unittest.makeSuite(ProductSeriesView))
- suite.addTest(unittest.makeSuite(ProductSeriesBugs))
- suite.addTest(unittest.makeSuite(ProjectGroupView))
- suite.addTest(unittest.makeSuite(ProjectGroupBugs))
- suite.addTest(unittest.makeSuite(DistributionSourcePackageView))
- suite.addTest(unittest.makeSuite(DistributionSourcePackageBugs))
- suite.addTest(unittest.makeSuite(DistroView))
- suite.addTest(unittest.makeSuite(DistroBugs))
- suite.addTest(unittest.makeSuite(DistroMilestoneView))
- suite.addTest(unittest.makeSuite(ProductMilestoneView))
- suite.addTest(unittest.makeSuite(ProductSeriesMilestoneView))
-
- suite.addTest(unittest.makeSuite(ProductDoesNotUseLPView))
- suite.addTest(unittest.makeSuite(ProductDoesNotUseLPBugs))
- suite.addTest(unittest.makeSuite(ProductSeriesDoesNotUseLPView))
- suite.addTest(unittest.makeSuite(ProductSeriesDoesNotUseLPBugs))
- suite.addTest(unittest.makeSuite(ProjectGroupDoesNotUseLPView))
- suite.addTest(unittest.makeSuite(ProjectGroupDoesNotUseLPBugs))
- suite.addTest(unittest.makeSuite(
- DistributionSourcePackageDoesNotUseLPView))
- suite.addTest(unittest.makeSuite(
- DistributionSourcePackageDoesNotUseLPBugs))
- suite.addTest(unittest.makeSuite(DistroDoesNotUseLPView))
- suite.addTest(unittest.makeSuite(DistroDoesNotUseLPBugs))
- suite.addTest(unittest.makeSuite(
- DistroMilestoneDoesNotUseLPView))
- suite.addTest(unittest.makeSuite(ProductMilestoneDoesNotUseLPView))
-
- return suite
+ return CustomTestLoader().loadTestsFromName(__name__)