launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/moar-factory-cs into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/moar-factory-cs/+merge/92910

Recently I added a factory method, makeCommercialSubscription() that creates a commercial subscription for a given product. I had based it on some code from a product test. However, in the branch that added the factory method, I forgot to go back and switch the product test to using the factory method.

I have also removed a pointless XXX.
-- 
https://code.launchpad.net/~stevenk/launchpad/moar-factory-cs/+merge/92910
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/moar-factory-cs into lp:launchpad.

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2012-02-08 06:00:46 +0000
+++ lib/lp/registry/tests/test_product.py	2012-02-14 04:56:20 +0000
@@ -33,7 +33,6 @@
     License,
     )
 from lp.registry.interfaces.series import SeriesStatus
-from lp.registry.model.commercialsubscription import CommercialSubscription
 from lp.registry.model.product import (
     Product,
     UnDeactivateable,
@@ -266,9 +265,6 @@
 
     def test_adddownloadfile_nonascii_filename(self):
         """Test uploading a file with a non-ascii char in the filename."""
-        # XXX EdwinGrubbs 2008-03-06 bug=69988
-        # Doctests are difficult to use with non-ascii characters, so
-        # I have used a unittest.
         firefox_owner = setupBrowser(auth='Basic mark@xxxxxxxxxxx:test')
         filename = u'foo\xa5.txt'.encode('utf-8')
         firefox_owner.open(
@@ -313,7 +309,7 @@
         self.assertEqual(a_element.contents[0].strip(), u'sig')
 
 
-class ProductAttributeCacheTestCase(TestCase):
+class ProductAttributeCacheTestCase(TestCaseWithFactory):
     """Cached attributes must be cleared at the end of a transaction."""
 
     layer = DatabaseFunctionalLayer
@@ -343,15 +339,7 @@
     def testCommercialSubscriptionCache(self):
         """commercial_subscription cache should not traverse transactions."""
         self.assertEqual(self.product.commercial_subscription, None)
-        now = datetime.datetime.now(pytz.UTC)
-        CommercialSubscription(
-            product=self.product,
-            date_starts=now,
-            date_expires=now,
-            registrant=self.product.owner,
-            purchaser=self.product.owner,
-            sales_system_id='foo',
-            whiteboard='bar')
+        self.factory.makeCommercialSubscription(self.product)
         self.assertEqual(self.product.commercial_subscription, None)
         self.product.redeemSubscriptionVoucher(
             'hello', self.product.owner, self.product.owner, 1)
@@ -363,14 +351,7 @@
 
         # Cache is cleared again.
         transaction.abort()
-        CommercialSubscription(
-            product=self.product,
-            date_starts=now,
-            date_expires=now,
-            registrant=self.product.owner,
-            purchaser=self.product.owner,
-            sales_system_id='new',
-            whiteboard='')
+        self.factory.makeCommercialSubscription(self.product)
         # Cache is cleared and it sees database changes that occur
         # before the cache is populated.
         self.assertEqual(self.product.commercial_subscription.sales_system_id,