launchpad-reviewers team mailing list archive

[Henning Eggers <henning.eggers@xxxxxxxxxxxxx>]

Henning Eggers has proposed merging lp:~henninge/launchpad/devel-update-testtools-r228 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~henninge/launchpad/devel-update-testtools-r228/+merge/71659

= Summary =

The trunk of testtools has MatchesStructure.byEquality which is of great
use in our code. I could replace most usees of MatchesStructure with
this factory method.

== Pre-implementation notes ==

[15:47] <henninge> jtv: do you know how MatchesStructure works?
[15:47] <jtv> Yes, but attempts to find it back failed for some reason.
[15:48] <henninge> jtv: it's in testtools.matchers
[15:48] <jtv> I thought I'd looked there.
[15:49] <henninge> jtv: it is missing from __all__ ...
[15:49] <jtv> That'd do it.
[15:49] <jtv> I don't see it in help(testtools.matchers).
[15:50] <jml> jtv, henninge: fixed in forthcoming release
[15:50] <jtv> Great, thanks.
[15:50] <henninge> jml: yes, just checked that ;-)
[15:50] <jml> Also, http://testtools.readthedocs.org/en/latest/for-test-authors.html is quite complete
[15:51] <henninge> jml: I am eager for that release, MatchesStructure.byEquality is really missing ... ;)
[15:51] <jml> henninge: nothing to stop LP from using a snapshot of trunk
[15:51] <henninge>  I guess ...
[15:51]  * henninge takes a note to look into that tomorrow
[15:52] <jml> basically, as soon as https://bugs.launchpad.net/testtools/+bug/804127 is fixed and the other critical bug fix lands, I'm going to do a release

== Tests ==

bin/test -vvcm lp.registry.tests.test_distroseriesparent \
         -m lp.soyuz.scripts.tests.test_copypackage \
         -m lp.soyuz.tests.test_archive \
         -m lp.soyuz.tests.test_distroseriesdifferencejob \
         -m lp.soyuz.tests.test_packagecopyjob \
         -m lp.translations.scripts.tests.test_remove_translations

== Demo and Q/A ==

Passing test suite is Q/A enough.

= Launchpad lint =

I will remove most of these before landing but did not want to clutter
up the diff.

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/scripts/tests/test_remove_translations.py
  lib/lp/soyuz/tests/test_archive.py
  versions.cfg
  lib/lp/registry/tests/test_distroseriesparent.py
  doc/buildout.txt
  lib/lp/soyuz/tests/test_packagecopyjob.py
  lib/lp/soyuz/scripts/tests/test_copypackage.py
  lib/lp/soyuz/tests/test_distroseriesdifferencejob.py

./lib/lp/soyuz/tests/test_archive.py
    1369: local variable 'sources_list_str' is assigned to but never used
    1528: local variable 'deparchseries' is assigned to but never used
    1596: E203 whitespace before ','
    1943: E222 multiple spaces after operator
    1968: E222 multiple spaces after operator
    2270: W391 blank line at end of file
    1988: Line exceeds 78 characters.
./versions.cfg
     251: Line exceeds 78 characters.
./doc/buildout.txt
       4: narrative exceeds 78 characters.
       8: narrative exceeds 78 characters.
      10: narrative exceeds 78 characters.
      40: narrative exceeds 78 characters.
      51: narrative exceeds 78 characters.
      59: narrative exceeds 78 characters.
      68: narrative exceeds 78 characters.
      70: narrative exceeds 78 characters.
      74: narrative exceeds 78 characters.
      77: narrative exceeds 78 characters.
     102: narrative exceeds 78 characters.
     105: narrative exceeds 78 characters.
     115: narrative exceeds 78 characters.
     137: narrative exceeds 78 characters.
     140: narrative exceeds 78 characters.
     146: narrative exceeds 78 characters.
     165: narrative exceeds 78 characters.
     192: narrative exceeds 78 characters.
     234: narrative exceeds 78 characters.
     255: narrative exceeds 78 characters.
     259: narrative exceeds 78 characters.
     275: narrative exceeds 78 characters.
     294: narrative exceeds 78 characters.
     297: narrative exceeds 78 characters.
     307: narrative exceeds 78 characters.
     316: narrative exceeds 78 characters.
     331: narrative exceeds 78 characters.
     337: narrative exceeds 78 characters.
     340: narrative exceeds 78 characters.
     344: narrative exceeds 78 characters.
     364: narrative exceeds 78 characters.
     378: narrative exceeds 78 characters.
     388: narrative exceeds 78 characters.
     405: narrative exceeds 78 characters.
     421: narrative exceeds 78 characters.
     425: narrative exceeds 78 characters.
     429: narrative exceeds 78 characters.
     455: narrative exceeds 78 characters.
     481: narrative exceeds 78 characters.
     484: narrative exceeds 78 characters.
     505: narrative exceeds 78 characters.
     533: narrative exceeds 78 characters.
     538: narrative exceeds 78 characters.
     540: narrative exceeds 78 characters.
     541: narrative exceeds 78 characters.
     545: narrative exceeds 78 characters.
     553: narrative exceeds 78 characters.
     572: narrative exceeds 78 characters.
     596: narrative exceeds 78 characters.
./lib/lp/soyuz/scripts/tests/test_copypackage.py
    1446: local variable 'admin' is assigned to but never used
    1669: local variable 'ancestor_bins' is assigned to but never used
    1994: local variable 'noise_source' is assigned to but never used
    2152: local variable 'ppa_binaries' is assigned to but never used
    2193: local variable 'ppa_binaries' is assigned to but never used
    2247: local variable 'ppa_binaries' is assigned to but never used
    2271: local variable 'hoary_amd64' is assigned to but never used
    2829: local variable 'ancestry_binaries' is assigned to but never used
    3033: local variable 'release_source' is assigned to but never used
    3013: local variable 'ppa_source' is assigned to but never used
    2973: local variable 'proposed_source' is assigned to but never used
    2993: local variable 'dev_source' is assigned to but never used
    3059: local variable 'orig_tarball' is assigned to but never used
    3066: local variable 'updates_source' is assigned to but never used
    2942: local variable 'binaries' is assigned to but never used
-- 
https://code.launchpad.net/~henninge/launchpad/devel-update-testtools-r228/+merge/71659
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~henninge/launchpad/devel-update-testtools-r228 into lp:launchpad.

=== modified file 'doc/buildout.txt'
--- doc/buildout.txt	2011-04-05 16:01:41 +0000
+++ doc/buildout.txt	2011-08-16 10:22:40 +0000
@@ -580,6 +580,15 @@
 See the setuptools documentation for more information about `the egg_info
 command`_.
 
+It is also possible that setup.py does not support the egg_info command and
+it is sufficient to just run the sdist command. It adds the current revision
+of the bzr branch to the version number::
+
+    python setup.py sdist
+
+For instance, this might produce a distribution for the ``testtools``
+project with a name like this: ``testtools-0.9.12-r228.tar.gz``.
+
 .. _FeedValidator: http://feedvalidator.org/
 
 .. _`other answers`: http://pypi.python.org/pypi/zc.buildoutsftp

=== modified file 'lib/lp/registry/tests/test_distroseriesparent.py'
--- lib/lp/registry/tests/test_distroseriesparent.py	2011-05-19 15:26:32 +0000
+++ lib/lp/registry/tests/test_distroseriesparent.py	2011-08-16 10:22:40 +0000
@@ -5,10 +5,7 @@
 
 __metaclass__ = type
 
-from testtools.matchers import (
-    Equals,
-    MatchesStructure,
-    )
+from testtools.matchers import MatchesStructure
 from zope.component import getUtility
 from zope.interface.verify import verifyObject
 from zope.security.interfaces import Unauthorized
@@ -48,18 +45,17 @@
         dsp = self.factory.makeDistroSeriesParent(
             derived_series=derived_series,
             parent_series=parent_series,
-            initialized=True
-            )
+            initialized=True)
 
         self.assertThat(
             dsp,
-            MatchesStructure(
-                derived_series=Equals(derived_series),
-                parent_series=Equals(parent_series),
-                initialized=Equals(True),
-                is_overlay=Equals(False),
-                component=Equals(None),
-                pocket=Equals(None),
+            MatchesStructure.byEquality(
+                derived_series=derived_series,
+                parent_series=parent_series,
+                initialized=True,
+                is_overlay=False,
+                component=None,
+                pocket=None,
                 ))
 
     def test_properties_overlay(self):
@@ -78,13 +74,13 @@
 
         self.assertThat(
             dsp,
-            MatchesStructure(
-                derived_series=Equals(derived_series),
-                parent_series=Equals(parent_series),
-                initialized=Equals(True),
-                is_overlay=Equals(True),
-                component=Equals(universe_component),
-                pocket=Equals(PackagePublishingPocket.SECURITY),
+            MatchesStructure.byEquality(
+                derived_series=derived_series,
+                parent_series=parent_series,
+                initialized=True,
+                is_overlay=True,
+                component=universe_component,
+                pocket=PackagePublishingPocket.SECURITY,
                 ))
 
     def test_getByDerivedSeries(self):

=== modified file 'lib/lp/soyuz/scripts/tests/test_copypackage.py'
--- lib/lp/soyuz/scripts/tests/test_copypackage.py	2011-07-30 14:18:20 +0000
+++ lib/lp/soyuz/scripts/tests/test_copypackage.py	2011-08-16 10:22:40 +0000
@@ -1403,9 +1403,9 @@
             [source], target_archive, dsp.derived_series, source.pocket,
             check_permissions=False, overrides=[override])
 
-        matcher = MatchesStructure(
-            component=Equals(override.component),
-            section=Equals(override.section))
+        matcher = MatchesStructure.byEquality(
+            component=override.component,
+            section=override.section)
         self.assertThat(copied_source, matcher)
 
     def test_copy_ppa_generates_notification(self):

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2011-08-02 13:42:46 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2011-08-16 10:22:40 +0000
@@ -12,7 +12,6 @@
 
 from testtools.matchers import (
     DocTestMatches,
-    Equals,
     MatchesRegex,
     MatchesStructure,
     )
@@ -2109,15 +2108,15 @@
         copy_job = job_source.getActiveJobs(target_archive).one()
 
         # Its data should reflect the requested copy.
-        self.assertThat(copy_job, MatchesStructure(
-            package_name=Equals(source_name),
-            package_version=Equals(version),
-            target_archive=Equals(target_archive),
-            source_archive=Equals(source_archive),
-            target_distroseries=Equals(to_series),
-            target_pocket=Equals(to_pocket),
-            include_binaries=Equals(False),
-            copy_policy=Equals(PackageCopyPolicy.INSECURE)))
+        self.assertThat(copy_job, MatchesStructure.byEquality(
+            package_name=source_name,
+            package_version=version,
+            target_archive=target_archive,
+            source_archive=source_archive,
+            target_distroseries=to_series,
+            target_pocket=to_pocket,
+            include_binaries=False,
+            copy_policy=PackageCopyPolicy.INSECURE))
 
     def test_copyPackage_disallows_non_primary_archive_uploaders(self):
         # If copying to a primary archive and you're not an uploader for
@@ -2191,15 +2190,15 @@
         # There should be one copy job.
         job_source = getUtility(IPlainPackageCopyJobSource)
         copy_job = job_source.getActiveJobs(target_archive).one()
-        self.assertThat(copy_job, MatchesStructure(
-            package_name=Equals(source_name),
-            package_version=Equals(version),
-            target_archive=Equals(target_archive),
-            source_archive=Equals(source_archive),
-            target_distroseries=Equals(to_series),
-            target_pocket=Equals(to_pocket),
-            include_binaries=Equals(False),
-            copy_policy=Equals(PackageCopyPolicy.MASS_SYNC)))
+        self.assertThat(copy_job, MatchesStructure.byEquality(
+            package_name=source_name,
+            package_version=version,
+            target_archive=target_archive,
+            source_archive=source_archive,
+            target_distroseries=to_series,
+            target_pocket=to_pocket,
+            include_binaries=False,
+            copy_policy=PackageCopyPolicy.MASS_SYNC))
 
     def test_copyPackages_with_multiple_packages(self):
         # PENDING and PUBLISHED packages should both be copied.

=== modified file 'lib/lp/soyuz/tests/test_distroseriesdifferencejob.py'
--- lib/lp/soyuz/tests/test_distroseriesdifferencejob.py	2011-08-09 10:01:02 +0000
+++ lib/lp/soyuz/tests/test_distroseriesdifferencejob.py	2011-08-16 10:22:40 +0000
@@ -6,10 +6,7 @@
 __metaclass__ = type
 
 from psycopg2 import ProgrammingError
-from testtools.matchers import (
-    Equals,
-    MatchesStructure,
-    )
+from testtools.matchers import MatchesStructure
 import transaction
 from zope.component import getUtility
 from zope.interface.verify import verifyObject
@@ -178,11 +175,11 @@
         expected_metadata = {
             u'sourcepackagename': sourcepackagenameid,
             u'parent_series': dsp.parent_series.id}
-        self.assertThat(job, MatchesStructure(
-            distribution=Equals(dsp.derived_series.distribution),
-            distroseries=Equals(dsp.derived_series),
-            job_type=Equals(DistributionJobType.DISTROSERIESDIFFERENCE),
-            metadata=Equals(expected_metadata)))
+        self.assertThat(job, MatchesStructure.byEquality(
+            distribution=dsp.derived_series.distribution,
+            distroseries=dsp.derived_series,
+            job_type=DistributionJobType.DISTROSERIESDIFFERENCE,
+            metadata=expected_metadata))
 
     def test_create_multiple_jobs_ignore_other_series(self):
         dsp = self.factory.makeDistroSeriesParent()

=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py	2011-07-28 14:07:44 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py	2011-08-16 10:22:40 +0000
@@ -5,10 +5,7 @@
 
 import operator
 from testtools.content import text_content
-from testtools.matchers import (
-    Equals,
-    MatchesStructure,
-    )
+from testtools.matchers import MatchesStructure
 import transaction
 from zope.component import getUtility
 from zope.security.interfaces import (
@@ -871,9 +868,9 @@
             IComponentSet)[pcj.metadata["component_override"]]
         metadata_section = getUtility(
             ISectionSet)[pcj.metadata["section_override"]]
-        matcher = MatchesStructure(
-            component=Equals(metadata_component),
-            section=Equals(metadata_section))
+        matcher = MatchesStructure.byEquality(
+            component=metadata_component,
+            section=metadata_section)
         self.assertThat(override, matcher)
 
     def test_addSourceOverride_accepts_None_component_as_no_change(self):

=== modified file 'lib/lp/translations/scripts/tests/test_remove_translations.py'
--- lib/lp/translations/scripts/tests/test_remove_translations.py	2011-08-10 16:52:46 +0000
+++ lib/lp/translations/scripts/tests/test_remove_translations.py	2011-08-16 10:22:40 +0000
@@ -12,10 +12,7 @@
     OptionParser,
     OptionValueError,
     )
-from testtools.matchers import (
-    Equals,
-    MatchesStructure,
-    )
+from testtools.matchers import MatchesStructure
 from unittest import TestLoader
 
 from storm.store import Store
@@ -193,17 +190,17 @@
             '--origin=1',
             '--force',
             ])
-        self.assertThat(options, MatchesStructure(
-            submitter=Equals(1),
-            reviewer=Equals(2),
-            ids=Equals([3, 4]),
-            potemplate=Equals(5),
-            language=Equals('te'),
-            not_language=Equals(True),
-            is_current_ubuntu=Equals(True),
-            is_current_upstream=Equals(False),
-            origin=Equals(1),
-            force=Equals(True)))
+        self.assertThat(options, MatchesStructure.byEquality(
+            submitter=1,
+            reviewer=2,
+            ids=[3, 4],
+            potemplate=5,
+            language='te',
+            not_language=True,
+            is_current_ubuntu=True,
+            is_current_upstream=False,
+            origin=1,
+            force=True))
 
     def test_WithLookups(self):
         # The script can also look up some items from different
@@ -219,12 +216,12 @@
             '--is-current-upstream=true',
             '--origin=SCM',
             ])
-        self.assertThat(options, MatchesStructure(
-            submitter=Equals(submitter.id),
-            reviewer=Equals(reviewer.id),
-            is_current_ubuntu=Equals(False),
-            is_current_upstream=Equals(True),
-            origin=Equals(RosettaTranslationOrigin.SCM.value)))
+        self.assertThat(options, MatchesStructure.byEquality(
+            submitter=submitter.id,
+            reviewer=reviewer.id,
+            is_current_ubuntu=False,
+            is_current_upstream=True,
+            origin=RosettaTranslationOrigin.SCM.value))
 
     def test_BadBool(self):
         self.assertRaises(Exception, parse_opts, '--is-current-ubuntu=None')

=== modified file 'versions.cfg'
--- versions.cfg	2011-08-15 13:11:05 +0000
+++ versions.cfg	2011-08-16 10:22:40 +0000
@@ -83,7 +83,7 @@
 # lp:~launchpad-committers/storm/with-without-datetime
 storm = 0.18.0.99-lpwithnodatetime-r394
 testresources = 0.2.4-r58
-testtools = 0.9.12-r194
+testtools = 0.9.12-r228
 timeline = 0.0.1
 transaction = 1.0.0
 txamqp = 0.4