Merge lp:~gz/bzr/run_test_setup_in_tempdir_140874 into lp:bzr/2.5

[Martin Packman]

Run bt.test_setup in a more isolated environment

The smoketest for setup.py runs with the user's environment and in the current directory, which leads to issues such as the one linked and bug 955314 where running tests makes various build artefacts appear. This branch makes it use a tempdir for both build output and install location.

The downside is this then becomes the test that needs the most space on temp (you can currently get away with just 20MB or so), and I haven't got a neat way of detecting a lack of room and skipping.

[Vincent Ladeuil (vila)]

\o/

57 + self.log('test_build running in %s' % self.source_dir)

running *from* (instead of in) now, no ?

58 + build_dir = os.path.join(self.test_dir, "build")
59 + install_dir = os.path.join(self.test_dir, "install")

That's really nit-picking, but why not osutils.joinpath ?

I won't approve this proposal without a news entry ;-p

review: Needs Fixing

[Martin Packman (gz)]

> 57 + self.log('test_build running in %s' % self.source_dir)
>
> running *from* (instead of in) now, no ?

Good point, changed.

> 58 + build_dir = os.path.join(self.test_dir, "build")
> 59 + install_dir = os.path.join(self.test_dir, "install")
>
> That's really nit-picking, but why not osutils.joinpath ?

Would't serve any useful purpose, adds some segment checking and forces forward slashes on windows. Being consistent would be an argument for, but only if we had a more unified approach to paths in general.

>
> I won't approve this proposal without a news entry ;-p

Added. :)

[Martin Packman (gz)]

sent to pqm by email

[Vincent Ladeuil (vila)]

> Would't serve any useful purpose, adds some segment checking

> and forces forward slashes on windows.

Which shouldn't break right ?

> Being consistent would be an argument for,

Yup, that was mainly my point.

> but
> only if we had a more unified approach to paths in general.

...but using a *single* function to join paths *is* a unification, so using
that as an argument to *not* use osutils seems to aggravate the situation
rather than improving it ;) Not a big deal.

=== modified file 'bzrlib/tests/test_setup.py'
--- bzrlib/tests/test_setup.py	2011-01-12 01:01:53 +0000
+++ bzrlib/tests/test_setup.py	2012-03-29 11:31:22 +0000
@@ -21,19 +21,15 @@
 import subprocess
 
 import bzrlib
-from bzrlib.tests import TestCase, TestSkipped
-import bzrlib.osutils as osutils
+from bzrlib import tests
 
-# XXX: This clobbers the build directory in the real source tree; it'd be nice
-# to avoid that.
-#
 # TODO: Run bzr from the installed copy to see if it works.  Really we need to
 # run something that exercises every module, just starting it may not detect
 # some missing modules.
 #
 # TODO: Check that the version numbers are in sync.  (Or avoid this...)
 
-class TestSetup(TestCase):
+class TestSetup(tests.TestCaseInTempDir):
 
     def test_build_and_install(self):
         """ test cmd `python setup.py build`
@@ -45,30 +41,29 @@
         # are not necessarily invoked from there
         self.source_dir = os.path.dirname(os.path.dirname(bzrlib.__file__))
         if not os.path.isfile(os.path.join(self.source_dir, 'setup.py')):
-            raise TestSkipped(
+            self.skip(
                 'There is no setup.py file adjacent to the bzrlib directory')
         try:
             import distutils.sysconfig
             makefile_path = distutils.sysconfig.get_makefile_filename()
             if not os.path.exists(makefile_path):
-                raise TestSkipped(
+                self.skip(
                     'You must have the python Makefile installed to run this'
                     ' test. Usually this can be found by installing'
                     ' "python-dev"')
         except ImportError:
-            raise TestSkipped(
+            self.skip(
                 'You must have distutils installed to run this test.'
                 ' Usually this can be found by installing "python-dev"')
-        self.log('test_build running in %s' % os.getcwd())
-        root_dir = osutils.mkdtemp()
-        try:
-            self.run_setup(['clean'])
-            # build is implied by install
-            ## self.run_setup(['build'])
-            self.run_setup(['install', '--root', root_dir])
-            self.run_setup(['clean'])
-        finally:
-            osutils.rmtree(root_dir)
+        self.log('test_build running from %s' % self.source_dir)
+        build_dir = os.path.join(self.test_dir, "build")
+        install_dir = os.path.join(self.test_dir, "install")
+        self.run_setup([
+            'build', '-b', build_dir,
+            'install', '--root', install_dir])
+        # Install layout is platform dependant
+        self.assertPathExists(install_dir)
+        self.run_setup(['clean', '-b', build_dir])
 
     def run_setup(self, args):
         args = [sys.executable, './setup.py', ] + args

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2012-03-26 17:01:21 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2012-03-29 11:31:22 +0000
@@ -73,6 +73,9 @@
 * Add support for pyftpdlib >= 0.7.0 and drop support for previous pyftpdlib
   versions. (Vincent Ladeuil, #956027)
 
+* Run smoketest for setup.py isolated in a tempdir. (Martin Packman, #140874)
+
+
 bzr 2.5.0
 #########