launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/refactor-fakeslave-waitingfiles into lp:launchpad-buildd with lp:~cjwatson/launchpad-buildd/scan-for-processes-python as a prerequisite.

Commit message:
Refactor FakeSlave.addWaitingFile tests to be friendlier to upcoming multiple-backend work.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/refactor-fakeslave-waitingfiles/+merge/328593
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/refactor-fakeslave-waitingfiles into lp:launchpad-buildd.

=== modified file 'lpbuildd/tests/fakeslave.py'
--- lpbuildd/tests/fakeslave.py	2017-04-26 12:24:32 +0000
+++ lpbuildd/tests/fakeslave.py	2017-08-04 17:33:03 +0000
@@ -7,7 +7,9 @@
     'FakeSlave',
     ]
 
+import hashlib
 import os
+import shutil
 
 
 class FakeMethod:
@@ -76,7 +78,7 @@
         self._config = FakeConfig()
         self.waitingfiles = {}
         for fake_method in (
-            "storeFile", "addWaitingFile", "emptyLog", "log",
+            "emptyLog", "log",
             "chrootFail", "buildFail", "builderFail", "depFail", "buildOK",
             "buildComplete",
             ):
@@ -85,6 +87,13 @@
     def cachePath(self, file):
         return os.path.join(self._cachepath, file)
 
+    def addWaitingFile(self, path):
+        with open(path, "rb") as f:
+            contents = f.read()
+        sha1sum = hashlib.sha1(contents).hexdigest()
+        shutil.copy(path, self.cachePath(sha1sum))
+        self.waitingfiles[os.path.basename(path)] = sha1sum
+
     def anyMethod(self, *args, **kwargs):
         pass
 

=== added file 'lpbuildd/tests/matchers.py'
--- lpbuildd/tests/matchers.py	1970-01-01 00:00:00 +0000
+++ lpbuildd/tests/matchers.py	2017-08-04 17:33:03 +0000
@@ -0,0 +1,29 @@
+# Copyright 2017 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from testtools.matchers import (
+    Equals,
+    Matcher,
+    MatchesDict,
+    )
+
+
+class HasWaitingFiles(Matcher):
+    """Match files that have been added using `slave.addWaitingFile`."""
+
+    def __init__(self, files):
+        self.files = files
+
+    @classmethod
+    def byEquality(cls, files):
+        return cls(
+            {name: Equals(contents) for name, contents in files.items()})
+
+    def match(self, slave):
+        waiting_file_contents = {}
+        for name in slave.waitingfiles:
+            with open(slave.cachePath(slave.waitingfiles[name]), "rb") as f:
+                waiting_file_contents[name] = f.read()
+        return MatchesDict(self.files).match(waiting_file_contents)

=== modified file 'lpbuildd/tests/test_binarypackage.py'
--- lpbuildd/tests/test_binarypackage.py	2017-08-04 17:33:03 +0000
+++ lpbuildd/tests/test_binarypackage.py	2017-08-04 17:33:03 +0000
@@ -32,6 +32,7 @@
     FakeMethod,
     FakeSlave,
     )
+from lpbuildd.tests.matchers import HasWaitingFiles
 
 
 class MockTransport:
@@ -178,8 +179,9 @@
         # After building the package, reap processes.
         self.assertScansSanely(SBuildExitCodes.OK)
         self.assertFalse(self.slave.wasCalled('buildFail'))
-        self.assertEqual(
-            [((changes_path,), {})], self.slave.addWaitingFile.calls)
+        self.assertThat(self.slave, HasWaitingFiles.byEquality({
+            'foo_1_i386.changes': b'I am a changes file.',
+            }))
 
         # Control returns to the DebianBuildManager in the UMOUNT state.
         self.assertUnmountsSanely()
@@ -341,9 +343,7 @@
         # After building the package, reap processes.
         self.assertScansSanely(SBuildExitCodes.OK)
         self.assertTrue(self.slave.wasCalled('buildFail'))
-        self.assertEqual(
-            [((os.path.join(build_dir, 'foo_1_i386.changes'),), {})],
-            self.slave.addWaitingFile.calls)
+        self.assertThat(self.slave, HasWaitingFiles({}))
 
         # Control returns to the DebianBuildManager in the UMOUNT state.
         self.assertUnmountsSanely()

=== modified file 'lpbuildd/tests/test_livefs.py'
--- lpbuildd/tests/test_livefs.py	2017-08-04 17:33:03 +0000
+++ lpbuildd/tests/test_livefs.py	2017-08-04 17:33:03 +0000
@@ -14,6 +14,7 @@
     LiveFilesystemBuildState,
     )
 from lpbuildd.tests.fakeslave import FakeSlave
+from lpbuildd.tests.matchers import HasWaitingFiles
 
 
 class MockBuildManager(LiveFilesystemBuildManager):
@@ -86,15 +87,13 @@
         self.startBuild()
 
         log_path = os.path.join(self.buildmanager._cachepath, "buildlog")
-        log = open(log_path, "w")
-        log.write("I am a build log.")
-        log.close()
+        with open(log_path, "w") as log:
+            log.write("I am a build log.")
 
         os.makedirs(self.build_dir)
         manifest_path = os.path.join(self.build_dir, "livecd.ubuntu.manifest")
-        manifest = open(manifest_path, "w")
-        manifest.write("I am a manifest file.")
-        manifest.close()
+        with open(manifest_path, "w") as manifest:
+            manifest.write("I am a manifest file.")
 
         # After building the package, reap processes.
         self.buildmanager.iterate(0)
@@ -109,8 +108,9 @@
         self.assertNotEqual(
             self.buildmanager.iterate, self.buildmanager.iterators[-1])
         self.assertFalse(self.slave.wasCalled("buildFail"))
-        self.assertEqual(
-            [((manifest_path,), {})], self.slave.addWaitingFile.calls)
+        self.assertThat(self.slave, HasWaitingFiles.byEquality({
+            "livecd.ubuntu.manifest": b"I am a manifest file.",
+            }))
 
         # Control returns to the DebianBuildManager in the UMOUNT state.
         self.buildmanager.iterateReap(self.getState(), 0)
@@ -130,19 +130,18 @@
         self.startBuild()
 
         log_path = os.path.join(self.buildmanager._cachepath, "buildlog")
-        log = open(log_path, "w")
-        log.write("I am a build log.")
-        log.close()
+        with open(log_path, "w") as log:
+            log.write("I am a build log.")
 
         os.makedirs(self.build_dir)
         target_path = os.path.join(
             self.build_dir, "livecd.ubuntu.kernel-generic")
-        target = open(target_path, "w")
-        target.write("I am a kernel.")
-        target.close()
+        with open(target_path, "w") as target:
+            target.write("I am a kernel.")
         link_path = os.path.join(self.build_dir, "livecd.ubuntu.kernel")
         os.symlink("livecd.ubuntu.kernel-generic", link_path)
 
         self.buildmanager.iterate(0)
-        self.assertEqual(
-            [((target_path,), {})], self.slave.addWaitingFile.calls)
+        self.assertThat(self.slave, HasWaitingFiles.byEquality({
+            "livecd.ubuntu.kernel-generic": b"I am a kernel.",
+            }))

=== modified file 'lpbuildd/tests/test_snap.py'
--- lpbuildd/tests/test_snap.py	2017-08-04 17:33:03 +0000
+++ lpbuildd/tests/test_snap.py	2017-08-04 17:33:03 +0000
@@ -14,6 +14,7 @@
     SnapBuildState,
     )
 from lpbuildd.tests.fakeslave import FakeSlave
+from lpbuildd.tests.matchers import HasWaitingFiles
 
 
 class MockBuildManager(SnapBuildManager):
@@ -98,9 +99,8 @@
         self.startBuild()
 
         log_path = os.path.join(self.buildmanager._cachepath, "buildlog")
-        log = open(log_path, "w")
-        log.write("I am a build log.")
-        log.close()
+        with open(log_path, "w") as log:
+            log.write("I am a build log.")
 
         output_dir = os.path.join(self.build_dir, "test-snap")
         os.makedirs(output_dir)
@@ -120,7 +120,9 @@
         self.assertNotEqual(
             self.buildmanager.iterate, self.buildmanager.iterators[-1])
         self.assertFalse(self.slave.wasCalled("buildFail"))
-        self.assertEqual([((snap_path,), {})], self.slave.addWaitingFile.calls)
+        self.assertThat(self.slave, HasWaitingFiles.byEquality({
+            "test-snap_0_all.snap": b"I am a snap package.",
+            }))
 
         # Control returns to the DebianBuildManager in the UMOUNT state.
         self.buildmanager.iterateReap(self.getState(), 0)
@@ -141,9 +143,8 @@
         self.startBuild()
 
         log_path = os.path.join(self.buildmanager._cachepath, "buildlog")
-        log = open(log_path, "w")
-        log.write("I am a build log.")
-        log.close()
+        with open(log_path, "w") as log:
+            log.write("I am a build log.")
 
         output_dir = os.path.join(self.build_dir, "test-snap")
         os.makedirs(output_dir)
@@ -166,9 +167,10 @@
         self.assertNotEqual(
             self.buildmanager.iterate, self.buildmanager.iterators[-1])
         self.assertFalse(self.slave.wasCalled("buildFail"))
-        self.assertEqual(
-            [((manifest_path,), {}), ((snap_path,), {})],
-            self.slave.addWaitingFile.calls)
+        self.assertThat(self.slave, HasWaitingFiles.byEquality({
+            "test-snap_0_all.manifest": b"I am a manifest.",
+            "test-snap_0_all.snap": b"I am a snap package.",
+            }))
 
         # Control returns to the DebianBuildManager in the UMOUNT state.
         self.buildmanager.iterateReap(self.getState(), 0)

=== modified file 'lpbuildd/tests/test_sourcepackagerecipe.py'
--- lpbuildd/tests/test_sourcepackagerecipe.py	2017-08-04 17:33:03 +0000
+++ lpbuildd/tests/test_sourcepackagerecipe.py	2017-08-04 17:33:03 +0000
@@ -10,12 +10,13 @@
 
 from testtools import TestCase
 
-from lpbuildd.tests.fakeslave import FakeSlave
 from lpbuildd.sourcepackagerecipe import (
     RETCODE_FAILURE_INSTALL_BUILD_DEPS,
     SourcePackageRecipeBuildManager,
     SourcePackageRecipeBuildState,
     )
+from lpbuildd.tests.fakeslave import FakeSlave
+from lpbuildd.tests.matchers import HasWaitingFiles
 
 
 class MockBuildManager(SourcePackageRecipeBuildManager):
@@ -105,22 +106,19 @@
         self.startBuild()
 
         log_path = os.path.join(self.buildmanager._cachepath, 'buildlog')
-        log = open(log_path, 'w')
-        log.write("I am a build log.")
-        log.close()
+        with open(log_path, 'w') as log:
+            log.write("I am a build log.")
 
         changes_path = os.path.join(
             self.buildmanager.home, 'build-%s' % self.buildid,
             'foo_1_source.changes')
-        changes = open(changes_path, 'w')
-        changes.write("I am a changes file.")
-        changes.close()
+        with open(changes_path, 'w') as changes:
+            changes.write("I am a changes file.")
 
         manifest_path = os.path.join(
             self.buildmanager.home, 'build-%s' % self.buildid, 'manifest')
-        manifest = open(manifest_path, 'w')
-        manifest.write("I am a manifest file.")
-        manifest.close()
+        with open(manifest_path, 'w') as manifest:
+            manifest.write("I am a manifest file.")
 
         # After building the package, reap processes.
         self.buildmanager.iterate(0)
@@ -135,9 +133,10 @@
         self.assertNotEqual(
             self.buildmanager.iterate, self.buildmanager.iterators[-1])
         self.assertFalse(self.slave.wasCalled('buildFail'))
-        self.assertEqual(
-            [((changes_path,), {}), ((manifest_path,), {})],
-            self.slave.addWaitingFile.calls)
+        self.assertThat(self.slave, HasWaitingFiles.byEquality({
+            'foo_1_source.changes': b'I am a changes file.',
+            'manifest': b'I am a manifest file.',
+            }))
 
         # Control returns to the DebianBuildManager in the UMOUNT state.
         self.buildmanager.iterateReap(self.getState(), 0)
@@ -157,12 +156,12 @@
         self.startBuild()
 
         log_path = os.path.join(self.buildmanager._cachepath, 'buildlog')
-        log = open(log_path, 'w')
-        log.write(
-            "The following packages have unmet dependencies:\n"
-            " pbuilder-satisfydepends-dummy : Depends: base-files (>= 1000)"
-            " but it is not going to be installed.\n")
-        log.close()
+        with open(log_path, 'w') as log:
+            log.write(
+                "The following packages have unmet dependencies:\n"
+                " pbuilder-satisfydepends-dummy :"
+                " Depends: base-files (>= 1000)"
+                " but it is not going to be installed.\n")
 
         # The buildmanager calls depFail correctly and reaps processes.
         self.buildmanager.iterate(RETCODE_FAILURE_INSTALL_BUILD_DEPS)
@@ -199,9 +198,8 @@
         self.startBuild()
 
         log_path = os.path.join(self.buildmanager._cachepath, 'buildlog')
-        log = open(log_path, 'w')
-        log.write("I am a failing build log.")
-        log.close()
+        with open(log_path, 'w') as log:
+            log.write("I am a failing build log.")
 
         # The buildmanager calls buildFail correctly and reaps processes.
         self.buildmanager.iterate(RETCODE_FAILURE_INSTALL_BUILD_DEPS)

=== modified file 'lpbuildd/tests/test_translationtemplatesbuildmanager.py'
--- lpbuildd/tests/test_translationtemplatesbuildmanager.py	2017-08-04 17:33:03 +0000
+++ lpbuildd/tests/test_translationtemplatesbuildmanager.py	2017-08-04 17:33:03 +0000
@@ -10,6 +10,7 @@
 from testtools import TestCase
 
 from lpbuildd.tests.fakeslave import FakeSlave
+from lpbuildd.tests.matchers import HasWaitingFiles
 from lpbuildd.translationtemplates import (
     TranslationTemplatesBuildManager,
     TranslationTemplatesBuildState,
@@ -97,9 +98,8 @@
             self.buildmanager._resultname)
         os.makedirs(os.path.dirname(outfile_path))
 
-        outfile = open(outfile_path, 'w')
-        outfile.write("I am a template tarball. Seriously.")
-        outfile.close()
+        with open(outfile_path, 'w') as outfile:
+            outfile.write("I am a template tarball. Seriously.")
 
         # After generating templates, reap processes.
         self.buildmanager.iterate(0)
@@ -114,8 +114,10 @@
         self.assertNotEqual(
             self.buildmanager.iterate, self.buildmanager.iterators[-1])
         self.assertFalse(self.slave.wasCalled('buildFail'))
-        self.assertEqual(
-            [((outfile_path,), {})], self.slave.addWaitingFile.calls)
+        self.assertThat(self.slave, HasWaitingFiles.byEquality({
+            self.buildmanager._resultname: (
+                b'I am a template tarball. Seriously.'),
+            }))
 
         # The control returns to the DebianBuildManager in the UMOUNT state.
         self.buildmanager.iterateReap(self.getState(), 0)