launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/remove-pyvirtualdisplay into lp:maas.

Commit message:
Remove the development dependency on PyVirtualDisplay.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1052852 in MAAS: "PyVirtualDisplay indirectly depends on an argparse version not in the standard library"
  https://bugs.launchpad.net/maas/+bug/1052852

For more details, see:
https://code.launchpad.net/~allenap/maas/remove-pyvirtualdisplay/+merge/135652

This is an old branch that can now be landed because sst 0.2.2 has been released. See the bug for rationale.

The tiny part of PyVirtualDisplay that we used has been replaced by a slightly beefed-up DisplayFixture, which is really way cooler than the one in PyVirtualDisplay ;)

-- 
https://code.launchpad.net/~allenap/maas/remove-pyvirtualdisplay/+merge/135652
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/remove-pyvirtualdisplay into lp:maas.

=== modified file 'src/maastesting/fixtures.py'
--- src/maastesting/fixtures.py	2012-10-18 08:16:31 +0000
+++ src/maastesting/fixtures.py	2012-11-22 11:54:08 +0000
@@ -19,13 +19,17 @@
 
 import logging
 import os
+from subprocess import (
+    CalledProcessError,
+    PIPE,
+    Popen,
+    )
 
 from fixtures import (
     EnvironmentVariableFixture,
     Fixture,
     TempDir,
     )
-from pyvirtualdisplay import Display
 from sst.actions import (
     start,
     stop,
@@ -53,22 +57,58 @@
 
 
 class DisplayFixture(Fixture):
-    """Fixture to create a virtual display with pyvirtualdisplay.Display."""
-
-    logger_names = ['easyprocess', 'pyvirtualdisplay']
-
-    def __init__(self, visible=False, size=(1280, 1024)):
+    """Fixture to create a virtual display with `xvfb-run`.
+
+    This will set the ``DISPLAY`` environment variable once it's up and
+    running (and reset it when it shuts down).
+    """
+
+    def __init__(self, size=(1280, 1024), depth=24):
         super(DisplayFixture, self).__init__()
-        self.visible = visible
-        self.size = size
+        self.width, self.height = size
+        self.depth = depth
+
+    @property
+    def command(self):
+        """The command this fixture will start.
+
+        ``xvfb-run`` is the executable used, to which the following arguments
+        are passed:
+
+          ``--server-args=``
+            ``-ac`` disables host-based access control mechanisms. See
+              Xserver(1).
+            ``-screen`` forces a screen configuration. At the time of writing
+               there is some disagreement between xvfb-run(1) and Xvfb(1)
+               about what the default is.
+
+          ``--auto-servernum``
+            Try to get a free server number, starting at 99. See xvfb-run(1).
+
+        ``xvfb-run`` is asked to chain to ``bash``, which echos the
+        ``DISPLAY`` environment variable and execs ``cat``. This lets us shut
+        down the framebuffer simply by closing the process's stdin.
+        """
+        spec = "{self.width}x{self.height}x{self.depth}".format(self=self)
+        args = "-ac -screen 0 %s" % spec
+        return (
+            "xvfb-run", "--server-args", args, "--auto-servernum", "--",
+            "bash", "-c", "echo $DISPLAY && exec cat",
+            )
 
     def setUp(self):
         super(DisplayFixture, self).setUp()
-        self.useFixture(LoggerSilencerFixture(self.logger_names))
-        self.display = Display(
-            visible=self.visible, size=self.size)
-        self.display.start()
-        self.addCleanup(self.display.stop)
+        self.process = Popen(self.command, stdin=PIPE, stdout=PIPE)
+        self.display = self.process.stdout.readline().strip()
+        if not self.display or self.process.poll() is not None:
+            raise CalledProcessError(self.process.returncode, self.command)
+        self.useFixture(EnvironmentVariableFixture("DISPLAY", self.display))
+        self.addCleanup(self.shutdown)
+
+    def shutdown(self):
+        self.process.stdin.close()
+        if self.process.wait() != 0:
+            raise CalledProcessError(self.process.returncode, self.command)
 
 
 class SSTFixture(Fixture):

=== modified file 'versions.cfg'
--- versions.cfg	2012-10-15 09:44:53 +0000
+++ versions.cfg	2012-11-22 11:54:08 +0000
@@ -31,6 +31,7 @@
 nose-subunit = 0.2
 python-subunit = 0.0.7
 rabbitfixture = 0.3.2
+sst = 0.2.2
 testresources = 0.2.5
 testscenarios = 0.3
 testtools = 0.9.14
@@ -44,30 +45,6 @@
 # collective.recipe.sphinxbuilder==0.7.0
 zc.recipe.egg = 1.3.2
 
-# Added by Buildout Versions at 2012-02-24 16:56:06.100791
-PyVirtualDisplay = 0.0.9
-sst = 0.2.1
-
-# Required by:
-# PyVirtualDisplay==0.0.9
-EasyProcess = 0.1.3
-
-# Required by:
-# entrypoint2==0.0.4
-argparse = 1.2.1
-
-# Required by:
-# entrypoint2==0.0.4
-decorator = 3.3.2
-
-# Required by:
-# PyVirtualDisplay==0.0.9
-entrypoint2 = 0.0.4
-
-# Required by:
-# PyVirtualDisplay==0.0.9
-path.py = 2.2.2
-
 # Required by:
 # oops==0.0.10
 iso8601 = 0.1.4
@@ -128,14 +105,14 @@
 # Added by Buildout Versions at 2012-04-24 11:28:06.120704
 
 # Required by:
-# sst==0.2.1
+# sst==0.2.2
 junitxml = 0.6
 
 # Required by:
-# sst==0.2.1
+# sst==0.2.2
 # saucelabsfixture=0.1
 selenium = 2.25
 
 # Required by:
-# sst==0.2.1
+# sst==0.2.2
 unittest2 = 0.5.1