Refactor the copying of xxmodule.c in packaging tests (#12141).

I need to copy this file in another test too, so I moved the support
code to distutils.tests.support and improved it to use proper skip
machinery instead of custom print/return/test suite fiddling.

Contrary to my similar change in distutils tests, I did not add support
for finding xxmodule.c when running a test from the tests directory,
because in that case my compiler didn’t find Python.h, so I figured it’s
better to skip than to fail.

diff --git a/Lib/packaging/tests/support.py b/Lib/packaging/tests/support.py
index c6e3f7287938a5b999f8bf0a57aa57dffbafa5d7..a9535ab58455375ce19b0edee190a26bd56c4011 100644 (file)
--- a/Lib/packaging/tests/support.py
+++ b/Lib/packaging/tests/support.py
@@ -32,6 +32,7 @@ import shutil
 import logging
 import weakref
 import tempfile
+import sysconfig
 
 from packaging.dist import Distribution
 from packaging.tests import unittest
@@ -39,7 +40,7 @@ from test.support import requires_zlib, unlink
 
 __all__ = ['LoggingCatcher', 'TempdirManager', 'EnvironRestorer',
            'DummyCommand', 'unittest', 'create_distribution',
-           'skip_unless_symlink', 'requires_zlib']
+           'skip_unless_symlink', 'requires_zlib', 'copy_xxmodule_c']
 
 
 logger = logging.getLogger('packaging')
@@ -271,6 +272,38 @@ def fake_dec(*args, **kw):
     return _wrap
 
 
+def copy_xxmodule_c(directory):
+    """Helper for tests that need the xxmodule.c source file.
+
+    Example use:
+
+        def test_compile(self):
+            copy_xxmodule_c(self.tmpdir)
+            self.assertIn('xxmodule.c', os.listdir(self.tmpdir)
+
+    If the source file can be found, it will be copied to *directory*.  If not,
+    the test will be skipped.  Errors during copy are not caught.
+    """
+    filename = _get_xxmodule_path()
+    if filename is None:
+        raise unittest.SkipTest('cannot find xxmodule.c (test must run in '
+                                'the python build dir)')
+    shutil.copy(filename, directory)
+
+
+def _get_xxmodule_path():
+    srcdir = sysconfig.get_config_var('srcdir')
+    candidates = [
+        # use installed copy if available
+        os.path.join(os.path.dirname(__file__), 'xxmodule.c'),
+        # otherwise try using copy from build directory
+        os.path.join(srcdir, 'Modules', 'xxmodule.c'),
+    ]
+    for path in candidates:
+        if os.path.exists(path):
+            return path
+
+
 try:
     from test.support import skip_unless_symlink
 except ImportError:

diff --git a/Lib/packaging/tests/test_command_build_ext.py b/Lib/packaging/tests/test_command_build_ext.py
index e144fd0738b9ca7b4f72c3abd6a111544c1d3b79..9521adcedfc9759b2219bad62742615f828c151b 100644 (file)
--- a/Lib/packaging/tests/test_command_build_ext.py
+++ b/Lib/packaging/tests/test_command_build_ext.py
@@ -1,7 +1,6 @@
 import os
 import sys
 import site
-import shutil
 import sysconfig
 import textwrap
 from io import StringIO
@@ -12,17 +11,7 @@ from packaging.command.build_ext import build_ext
 from packaging.compiler.extension import Extension
 from test.script_helper import assert_python_ok
 
-from packaging.tests import support, unittest, verbose, unload
-
-
-def _get_source_filename():
-    # use installed copy if available
-    tests_f = os.path.join(os.path.dirname(__file__), 'xxmodule.c')
-    if os.path.exists(tests_f):
-        return tests_f
-    # otherwise try using copy from build directory
-    srcdir = sysconfig.get_config_var('srcdir')
-    return os.path.join(srcdir, 'Modules', 'xxmodule.c')
+from packaging.tests import support, unittest, verbose
 
 
 class BuildExtTestCase(support.TempdirManager,
@@ -33,9 +22,6 @@ class BuildExtTestCase(support.TempdirManager,
         # Note that we're making changes to sys.path
         super(BuildExtTestCase, self).setUp()
         self.tmp_dir = self.mkdtemp()
-        filename = _get_source_filename()
-        if os.path.exists(filename):
-            shutil.copy(filename, self.tmp_dir)
         self.old_user_base = site.USER_BASE
         site.USER_BASE = self.mkdtemp()
         build_ext.USER_BASE = site.USER_BASE
@@ -68,10 +54,8 @@ class BuildExtTestCase(support.TempdirManager,
                 cmd.library_dirs = value.split(os.pathsep)
 
     def test_build_ext(self):
+        support.copy_xxmodule_c(self.tmp_dir)
         xx_c = os.path.join(self.tmp_dir, 'xxmodule.c')
-        if not os.path.exists(xx_c):
-            # skipping if we cannot find it
-            return
         xx_ext = Extension('xx', [xx_c])
         dist = Distribution({'name': 'xx', 'ext_modules': [xx_ext]})
         dist.package_dir = self.tmp_dir
@@ -455,14 +439,7 @@ class BuildExtTestCase(support.TempdirManager,
 
 
 def test_suite():
-    src = _get_source_filename()
-    if not os.path.exists(src):
-        if verbose:
-            print('test_command_build_ext: Cannot find source code (test'
-                  ' must run in python build dir)')
-        return unittest.TestSuite()
-    else:
-        return unittest.makeSuite(BuildExtTestCase)
+    return unittest.makeSuite(BuildExtTestCase)
 
 if __name__ == '__main__':
     unittest.main(defaultTest='test_suite')