launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00355
[Merge] lp:~sinzui/launchpad/stop-shouting-0 into lp:launchpad/devel
Curtis Hovey has proposed merging lp:~sinzui/launchpad/stop-shouting-0 into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
This is my branch to stop the shouting about naked objects in tests.
lp:~sinzui/launchpad/stop-shouting-0
Diff size: 290
Test command: ./bin/test -vv \
-t registry.*test_(distroseries|sourcepackage
-t milestone-views -t productseries-views
Pre-implementation: jml, bigjools
Target release: 10.08
Stop the shouting about naked objects in tests
----------------------------------------------
I cannot work because I keep seeing annoying messages. This branch address
the ones that affect my feature branch.
ADDENDUM: There was a real error in a test that would have been caught if
the test has not removed the proxy. I will explain below.
Rules
-----
* Return a proxied object from the factory.
* Do not unwrap objects, log in to make the factory hack the attribute
for the test.
QA
--
None, this is a test runner.
Lint
----
Linting changed files:
lib/lp/registry/browser/tests/milestone-views.txt
lib/lp/registry/browser/tests/productseries-views.txt
lib/lp/registry/tests/test_distroseries.py
lib/lp/registry/tests/test_sourcepackage.py
lib/lp/testing/factory.py
Test
----
* lib/lp/registry/browser/tests/milestone-views.txt
* Login as the project owner to set the driver.
* lib/lp/registry/browser/tests/productseries-views.txt
* Use a better factory method to get a product series
* Wrap the long line.
* Print the title of the enum ()
* Change the factory to hack the non-writable ProductSeries.datecreated
* lib/lp/registry/tests/test_distroseries.py
* Login as the distro or project owner to set the object field.
* NOTE: product.bugtracker was misspelled in the first test!
The prioritised listing is incrementing the score for *missing*
information. Thus by adding a bugtracker, to cold, we expect that
package to have a lower score...it is missing less information.
This test was always wrong.
* lib/lp/registry/tests/test_sourcepackage.py
* Login as project owner to set the status.
Implementation
--------------
* lib/lp/testing/factory.py
* Hush lint: remove unused or redundant imports, wrap long lines.
* Return proxied milestone, productseries,
SourcePackagePublishingHistory (note that it was created, then
looked up again??).
* DistributionSourcePackage is implicitly wrapped when called via
distribution.getSourcePackage()
--
https://code.launchpad.net/~sinzui/launchpad/stop-shouting-0/+merge/31423
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/stop-shouting-0 into lp:launchpad/devel.
=== modified file 'lib/lp/registry/browser/tests/milestone-views.txt'
--- lib/lp/registry/browser/tests/milestone-views.txt 2010-07-20 17:50:45 +0000
+++ lib/lp/registry/browser/tests/milestone-views.txt 2010-07-30 20:26:07 +0000
@@ -579,13 +579,10 @@
The driver of a series that belongs to an `IDerivativeDistribution` is a
release manager and can create milestones.
- >>> from lp.testing.factory import (
- ... remove_security_proxy_and_shout_at_engineer)
>>> distroseries = factory.makeDistroRelease(name='pumpkin')
>>> driver = factory.makePerson(name='a-driver')
- >>> naked_distroseries = remove_security_proxy_and_shout_at_engineer(
- ... distroseries)
- >>> naked_distroseries.driver = driver
+ >>> login_person(distroseries.distribution.owner)
+ >>> distroseries.driver = driver
>>> login_person(driver)
>>> form = {
=== modified file 'lib/lp/registry/browser/tests/productseries-views.txt'
--- lib/lp/registry/browser/tests/productseries-views.txt 2010-07-20 17:50:45 +0000
+++ lib/lp/registry/browser/tests/productseries-views.txt 2010-07-30 20:26:07 +0000
@@ -12,7 +12,7 @@
>>> from lp.testing.menu import check_menu_links
>>> product = factory.makeProduct(name='app')
- >>> series = factory.makeSeries(name='simple', product=product)
+ >>> series = factory.makeProductSeries(name='simple', product=product)
>>> check_menu_links(ProductSeriesOverviewMenu(series))
True
@@ -80,7 +80,8 @@
>>> script = find_tag_by_id(view.render(), 'milestone-script')
>>> print script
<script id="milestone-script" type="text/javascript">
- YUI().use(... 'lp.registry.milestoneoverlay', 'lp.registry.milestonetable'...
+ YUI().use(... 'lp.registry.milestoneoverlay',
+ 'lp.registry.milestonetable'...
var series_uri = '/app/simple';
var milestone_form_uri = '.../app/simple/+addmilestone/++form++';
var milestone_row_uri =
@@ -130,7 +131,7 @@
>>> from lp.registry.interfaces.series import SeriesStatus
- >>> print series.status
+ >>> print series.status.title
Active Development
>>> view.is_obsolete
False
@@ -255,20 +256,13 @@
>>> from datetime import datetime
>>> from pytz import UTC
- >>> from lp.testing.factory import (
- ... remove_security_proxy_and_shout_at_engineer)
+ >>> test_date = datetime(2009, 05, 01, 19, 34, 24, tzinfo=UTC)
>>> product = factory.makeProduct(name="field", displayname='Field')
>>> productseries = factory.makeProductSeries(
- ... product=product, name='rabbit')
+ ... product=product, name='rabbit', date_created=test_date)
>>> productseries.releasefileglob = 'http://eg.dom/rabbit/*'
- # Hack the creation date for testing purposes.
- >>> test_date = datetime(2009, 05, 01, 19, 34, 24, tzinfo=UTC)
- >>> naked_productseries = remove_security_proxy_and_shout_at_engineer(
- ... productseries)
- >>> naked_productseries.datecreated = test_date
-
Users without edit permission cannot access the view.
>>> from canonical.launchpad.webapp.authorization import check_permission
=== modified file 'lib/lp/registry/tests/test_distroseries.py'
--- lib/lp/registry/tests/test_distroseries.py 2010-07-26 18:23:34 +0000
+++ lib/lp/registry/tests/test_distroseries.py 2010-07-30 20:26:07 +0000
@@ -23,8 +23,7 @@
from lp.soyuz.interfaces.publishing import (
active_publishing_status, PackagePublishingStatus)
from lp.soyuz.model.processor import ProcessorFamilySet
-from lp.testing import TestCase, TestCaseWithFactory
-from lp.testing.factory import remove_security_proxy_and_shout_at_engineer
+from lp.testing import person_logged_in, TestCase, TestCaseWithFactory
from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
from lp.translations.interfaces.translations import (
TranslationsBranchImportMode)
@@ -286,12 +285,11 @@
self.linkPackage('hot')
self.makeSeriesPackage('cold')
product_series = self.linkPackage('cold')
- naked_product_series = remove_security_proxy_and_shout_at_engineer(
- product_series)
- naked_product_series.product.bugtraker = self.factory.makeBugTracker()
+ with person_logged_in(product_series.product.owner):
+ product_series.product.bugtracker = self.factory.makeBugTracker()
packagings = self.series.getPrioritizedPackagings()
names = [packaging.sourcepackagename.name for packaging in packagings]
- expected = [u'hot', u'cold', u'linked']
+ expected = [u'hot', u'linked', u'cold']
self.assertEqual(expected, names)
def test_getPrioritizedPackagings_branch(self):
@@ -299,9 +297,8 @@
self.linkPackage('translatable')
self.makeSeriesPackage('withbranch')
product_series = self.linkPackage('withbranch')
- naked_product_series = remove_security_proxy_and_shout_at_engineer(
- product_series)
- naked_product_series.branch = self.factory.makeBranch()
+ with person_logged_in(product_series.product.owner):
+ product_series.branch = self.factory.makeBranch()
packagings = self.series.getPrioritizedPackagings()
names = [packaging.sourcepackagename.name for packaging in packagings]
expected = [u'translatable', u'linked', u'withbranch']
@@ -313,11 +310,10 @@
self.linkPackage('translatable')
self.makeSeriesPackage('importabletranslatable')
product_series = self.linkPackage('importabletranslatable')
- naked_product_series = remove_security_proxy_and_shout_at_engineer(
- product_series)
- naked_product_series.branch = self.factory.makeBranch()
- naked_product_series.translations_autoimport_mode = (
- TranslationsBranchImportMode.IMPORT_TEMPLATES)
+ with person_logged_in(product_series.product.owner):
+ product_series.branch = self.factory.makeBranch()
+ product_series.translations_autoimport_mode = (
+ TranslationsBranchImportMode.IMPORT_TEMPLATES)
packagings = self.series.getPrioritizedPackagings()
names = [packaging.sourcepackagename.name for packaging in packagings]
expected = [u'translatable', u'linked', u'importabletranslatable']
@@ -350,9 +346,8 @@
new_distroseries = (
self.factory.makeDistroRelease(name=u"sampleseries"))
- naked_new_distroseries = remove_security_proxy_and_shout_at_engineer(
- new_distroseries)
- naked_new_distroseries.hide_all_translations = False
+ with person_logged_in(new_distroseries.distribution.owner):
+ new_distroseries.hide_all_translations = False
transaction.commit()
translatables = self._get_translatables()
self.failUnlessEqual(
@@ -374,7 +369,8 @@
translatables,
self._ref_translatables(u"sampleseries")))
- naked_new_distroseries.hide_all_translations = True
+ with person_logged_in(new_distroseries.distribution.owner):
+ new_distroseries.hide_all_translations = True
transaction.commit()
translatables = self._get_translatables()
self.failUnlessEqual(
=== modified file 'lib/lp/registry/tests/test_sourcepackage.py'
--- lib/lp/registry/tests/test_sourcepackage.py 2010-07-20 17:50:45 +0000
+++ lib/lp/registry/tests/test_sourcepackage.py 2010-07-30 20:26:07 +0000
@@ -21,8 +21,7 @@
from lp.soyuz.interfaces.publishing import PackagePublishingStatus
from lp.code.interfaces.seriessourcepackagebranch import (
IMakeOfficialBranchLinks)
-from lp.testing import TestCaseWithFactory
-from lp.testing.factory import remove_security_proxy_and_shout_at_engineer
+from lp.testing import person_logged_in, TestCaseWithFactory
from lp.testing.views import create_initialized_view
from canonical.testing.layers import DatabaseFunctionalLayer
@@ -253,17 +252,13 @@
self.obsolete_productseries = self.factory.makeProductSeries(
name='obsolete', product=self.product)
- naked_obsolete_productseries = (
- remove_security_proxy_and_shout_at_engineer(
- self.obsolete_productseries))
- naked_obsolete_productseries.status = SeriesStatus.OBSOLETE
+ with person_logged_in(self.product.owner):
+ self.obsolete_productseries.status = SeriesStatus.OBSOLETE
self.dev_productseries = self.factory.makeProductSeries(
name='current', product=self.product)
- naked_dev_productseries = (
- remove_security_proxy_and_shout_at_engineer(
- self.dev_productseries))
- naked_dev_productseries.status = SeriesStatus.DEVELOPMENT
+ with person_logged_in(self.product.owner):
+ self.dev_productseries.status = SeriesStatus.DEVELOPMENT
self.distribution = self.factory.makeDistribution(
name='youbuntu', displayname='Youbuntu', owner=self.owner)
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-07-30 16:30:12 +0000
+++ lib/lp/testing/factory.py 2010-07-30 20:26:07 +0000
@@ -111,8 +111,6 @@
from lp.codehosting.codeimport.worker import CodeImportSourceDetails
from lp.registry.interfaces.distribution import IDistributionSet
-from lp.registry.model.distributionsourcepackage import (
- DistributionSourcePackage)
from lp.registry.interfaces.distroseries import IDistroSeries
from lp.registry.interfaces.gpg import GPGKeyAlgorithm, IGPGKeySet
from lp.registry.interfaces.mailinglist import (
@@ -170,8 +168,6 @@
RosettaImportStatus)
from lp.translations.interfaces.translationgroup import (
ITranslationGroupSet)
-from lp.translations.interfaces.translationimportqueue import (
- RosettaImportStatus)
from lp.translations.interfaces.translationfileformat import (
TranslationFileFormat)
from lp.translations.interfaces.translationsperson import ITranslationsPerson
@@ -636,9 +632,9 @@
product = productseries.product
if name is None:
name = self.getUniqueString()
- return Milestone(product=product, distribution=distribution,
- productseries=productseries,
- name=name)
+ return ProxyFactory(
+ Milestone(product=product, distribution=distribution,
+ productseries=productseries, name=name))
def makeProcessor(self, family=None, name=None, title=None,
description=None):
@@ -749,7 +745,7 @@
return product
def makeProductSeries(self, product=None, name=None, owner=None,
- summary=None):
+ summary=None, date_created=None):
"""Create and return a new ProductSeries."""
if product is None:
product = self.makeProduct()
@@ -762,8 +758,11 @@
# We don't want to login() as the person used to create the product,
# so we remove the security proxy before creating the series.
naked_product = removeSecurityProxy(product)
- return ProxyFactory(
- naked_product.newSeries(owner=owner, name=name, summary=summary))
+ series = naked_product.newSeries(
+ owner=owner, name=name, summary=summary)
+ if date_created is not None:
+ series.datecreated = date_created
+ return ProxyFactory(series)
def makeProject(self, name=None, displayname=None, title=None,
homepageurl=None, summary=None, owner=None,
@@ -2437,7 +2436,7 @@
dsc_binaries=dsc_binaries,
date_uploaded=date_uploaded)
- sspph = SourcePackagePublishingHistory(
+ return ProxyFactory(SourcePackagePublishingHistory(
distroseries=distroseries,
sourcepackagerelease=spr,
component=spr.component,
@@ -2447,11 +2446,7 @@
dateremoved=dateremoved,
scheduleddeletiondate=scheduleddeletiondate,
pocket=pocket,
- archive=archive)
-
- # SPPH and SSPPH IDs are the same, since they are SPPH is a SQLVIEW
- # of SSPPH and other useful attributes.
- return SourcePackagePublishingHistory.get(sspph.id)
+ archive=archive))
def makeBinaryPackagePublishingHistory(self, binarypackagerelease=None,
distroarchseries=None,
@@ -2588,7 +2583,7 @@
if distribution is None:
distribution = self.makeDistribution()
- return DistributionSourcePackage(distribution, sourcepackagename)
+ return distribution.getSourcePackage(sourcepackagename)
def makeEmailMessage(self, body=None, sender=None, to=None,
attachments=None, encode_attachments=False):
@@ -2767,7 +2762,8 @@
# security wrappers for them, as well as for objects created by
# other Python libraries.
unwrapped_types = (
- DSCFile, InstanceType, MergeDirective2, Message, datetime, int, str, unicode)
+ DSCFile, InstanceType, MergeDirective2, Message, datetime,
+ int, str, unicode)
def is_security_proxied_or_harmless(obj):
