launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad-buildd:py3-proc-self-fd into launchpad-buildd:master.

Commit message:
Work around test hang on Python >= 3.3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/+git/launchpad-buildd/+merge/383219

On versions of Python that support file-descriptor-based operations in `os` functions, the test suite hung because TestChroot's overlay fixture for /proc confused the standard library.  Work around this by removing /proc/self/fd from the overlay.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-buildd:py3-proc-self-fd into launchpad-buildd:master.

diff --git a/debian/changelog b/debian/changelog
index 8943f0a..2c12be4 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -3,6 +3,7 @@ launchpad-buildd (190) UNRELEASED; urgency=medium
   * Switch to git; add Vcs-* fields.
   * Always call Resource.putChild with path as bytes.
   * Switch RotatableFileLogObserver from class advice to a class decorator.
+  * Work around /proc/self/fd-related test hang on Python >= 3.3.
 
  -- Colin Watson <cjwatson@xxxxxxxxxx>  Tue, 28 Apr 2020 10:19:27 +0100
 
diff --git a/lpbuildd/target/tests/test_chroot.py b/lpbuildd/target/tests/test_chroot.py
index a245aa5..0361e25 100644
--- a/lpbuildd/target/tests/test_chroot.py
+++ b/lpbuildd/target/tests/test_chroot.py
@@ -15,7 +15,6 @@ from fixtures import (
     )
 import six
 from systemfixtures import (
-    FakeFilesystem,
     FakeProcesses,
     FakeTime,
     )
@@ -25,6 +24,7 @@ from testtools.matchers import DirContains
 from lpbuildd.target.backend import BackendException
 from lpbuildd.target.chroot import Chroot
 from lpbuildd.target.tests.testfixtures import (
+    FakeFilesystem,
     KillFixture,
     SudoUmount,
     )
@@ -317,6 +317,7 @@ class TestChroot(TestCase):
         fs_fixture.add("/expected")
         os.makedirs("/expected/home/build-1/chroot-autobuild")
         fs_fixture.add("/proc")
+        fs_fixture.remove("/proc/self/fd")
         os.mkdir("/proc")
         os.mkdir("/proc/1")
         os.symlink("/", "/proc/1/root")
@@ -345,6 +346,7 @@ class TestChroot(TestCase):
     def _make_initial_proc_mounts(self):
         fs_fixture = self.useFixture(FakeFilesystem())
         fs_fixture.add("/proc")
+        fs_fixture.remove("/proc/self/fd")
         os.mkdir("/proc")
         with open("/proc/mounts", "w") as mounts_file:
             mounts_file.write(dedent("""\
diff --git a/lpbuildd/target/tests/testfixtures.py b/lpbuildd/target/tests/testfixtures.py
index 81d8f4d..41116ff 100644
--- a/lpbuildd/target/tests/testfixtures.py
+++ b/lpbuildd/target/tests/testfixtures.py
@@ -4,9 +4,11 @@
 __metaclass__ = type
 
 import argparse
+import os
 import shutil
 
 from fixtures import MonkeyPatch
+from systemfixtures import FakeFilesystem as _FakeFilesystem
 
 
 class SudoUmount:
@@ -68,3 +70,34 @@ class KillFixture(MonkeyPatch):
     @property
     def kills(self):
         return self.new_value.kills
+
+
+class FakeFilesystem(_FakeFilesystem):
+    """A FakeFilesystem that can exclude subpaths.
+
+    Adding /proc to the overlay filesystem behaves badly on Python 3,
+    because FakeFilesystem uses /proc/self/fd for its own purposes when
+    dealing with file-descriptor-based operations.  Being able to remove
+    /proc/self/fd lets us work around this.
+    """
+
+    def _setUp(self):
+        super(FakeFilesystem, self)._setUp()
+        self._excludes = set()
+
+    def remove(self, path):
+        """Remove a path from the overlay filesystem.
+
+        Any filesystem operation involving this path or any sub-paths of it
+        will not be redirected, even if one of its parent directories is in
+        the overlay filesystem.
+        """
+        if not path.startswith(os.sep):
+            raise ValueError("Non-absolute path '{}'".format(path))
+        self._excludes.add(path.rstrip(os.sep))
+
+    def _is_fake_path(self, path, *args, **kwargs):
+        for prefix in self._excludes:
+            if path.startswith(prefix):
+                return False
+        return super(FakeFilesystem, self)._is_fake_path(path, *args, **kwargs)