On Tue, Dec 8, 2009 at 1:37 AM, Bjorn Tillenius <[email protected]> wrote: > On Tue, Dec 08, 2009 at 01:51:16AM -0000, [email protected] wrote: >> Merge authors: >> Edwin Grubbs (edwin-grubbs) >> ------------------------------------------------------------ >> revno: 9986 [merge] >> committer: Launchpad Patch Queue Manager <[email protected]> >> branch nick: launchpad >> timestamp: Tue 2009-12-08 01:48:50 +0000 >> message: >> [r=abentley][ui=none][bug=433809] Fixed ProductSeriesVocabulary >> searches containing a slash. >> added: >> lib/lp/registry/tests/test_productseries_vocabularies.py >> modified: >> lib/lp/registry/vocabularies.py >> >> >> -- >> lp:launchpad/devel >> https://code.launchpad.net/~launchpad-pqm/launchpad/devel >> >> You are subscribed to branch lp:launchpad/devel. >> To unsubscribe from this branch go to >> https://code.launchpad.net/~launchpad-pqm/launchpad/devel/+edit-subscription. > >> === added file 'lib/lp/registry/tests/test_productseries_vocabularies.py' >> --- lib/lp/registry/tests/test_productseries_vocabularies.py 1970-01-01 >> 00:00:00 +0000 >> +++ lib/lp/registry/tests/test_productseries_vocabularies.py 2009-12-07 >> 20:13:51 +0000 >> @@ -0,0 +1,72 @@ >> +# Copyright 2009 Canonical Ltd. This software is licensed under the >> +# GNU Affero General Public License version 3 (see the file LICENSE). >> + >> +"""Test the milestone vocabularies.""" >> + >> +__metaclass__ = type >> + >> +from unittest import TestLoader >> +from operator import attrgetter >> + >> +from lp.registry.vocabularies import ProductSeriesVocabulary >> +from lp.testing import TestCaseWithFactory >> + >> +from canonical.testing import DatabaseFunctionalLayer >> + >> + >> +class TestProductSeriesVocabulary(TestCaseWithFactory): >> + """Test that the ProductSeriesVocabulary behaves as expected.""" >> + layer = DatabaseFunctionalLayer > > Great that you added some better tests; the current vocabulary tests are > lacking quite a bit. > > >> + def setUp(self): >> + super(TestProductSeriesVocabulary, self).setUp() >> + self.vocabulary = ProductSeriesVocabulary() >> + self.product_prefix = 'asdf987-' >> + self.series1_prefix = 'qwerty-' >> + self.product = self.factory.makeProduct( >> + self.product_prefix + 'product1') >> + self.series = self.factory.makeProductSeries( >> + product=self.product, name=self.series1_prefix + "series1") >> + >> + def tearDown(self): >> + super(TestProductSeriesVocabulary, self).tearDown() >> + >> + def test_search(self): >> + series2 = self.factory.makeProductSeries(product=self.product) >> + # Search by product name. >> + result = self.vocabulary.search(self.product.name) >> + self.assertEqual( >> + [self.series, series2].sort(key=attrgetter('id')), >> + list(result).sort(key=attrgetter('id'))) >> + # Search by series name. >> + result = self.vocabulary.search(self.series.name) >> + self.assertEqual([self.series], list(result)) >> + # Search by series2 name. >> + result = self.vocabulary.search(series2.name) >> + self.assertEqual([series2], list(result)) >> + # Search by product & series name substrings. >> + result = self.vocabulary.search( >> + '%s/%s' % (self.product_prefix, self.series1_prefix)) >> + self.assertEqual([self.series], list(result)) > > Two comments. What value do the comments you added add? When commenting > in tests, I think it's better to think more in terms of doctests. What > happens if you remove all the code? Do the comments still make sense, > without the code? > > Also, why is this one single big test, instead of multiple smaller ones? > I had a hard time finding the test that actually tested the issue you > claimed to fix (searches containing a slash), and I think it would have > been easier if there had been a specific test for that. Smaller tests > makes it easier to see what actually is the problem, if a test starts to > fail.
This was just the way I wrote the tests from the beginning, and the benefits of rewriting seemed fairly small. I can definitely change it. >> + >> + def test_toTerm(self): >> + term = self.vocabulary.toTerm(self.series) >> + self.assertEqual( >> + '%s/%s' % (self.product.name, self.series.name), >> + term.token) >> + self.assertEqual(self.series, term.value) >> + >> + def test_getTermByToken(self): >> + token = '%s/%s' % (self.product.name, self.series.name) >> + term = self.vocabulary.getTermByToken(token) >> + self.assertEqual(token, term.token) >> + self.assertEqual(self.series, term.value) >> + >> + def test_getTermByToken_LookupError(self): >> + self.assertRaises( >> + LookupError, >> + self.vocabulary.getTermByToken, 'does/notexist') > > These all lack comments explaining the tests. I can add that. -Edwin _______________________________________________ Mailing list: https://launchpad.net/~launchpad-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-dev More help : https://help.launchpad.net/ListHelp
