Make sure packaging tests that register custom commands also clear them

4 files changed, 52 insertions, 20 deletions

diff --git a/Lib/packaging/tests/support.py b/Lib/packaging/tests/support.py
index 66890c9..1479598 100644
--- a/Lib/packaging/tests/support.py
+++ b/Lib/packaging/tests/support.py
@@ -41,6 +41,9 @@ import tempfile
 import sysconfig
 
 from packaging.dist import Distribution
+from packaging.util import resolve_name
+from packaging.command import set_command, _COMMANDS
+
 from packaging.tests import unittest
 from test.support import requires_zlib, unlink
 
@@ -51,7 +54,8 @@ __all__ = [
     # mocks
     'DummyCommand', 'TestDistribution',
     # misc. functions and decorators
-    'fake_dec', 'create_distribution', 'copy_xxmodule_c', 'fixup_build_ext',
+    'fake_dec', 'create_distribution', 'use_command',
+    'copy_xxmodule_c', 'fixup_build_ext',
     # imported from this module for backport purposes
     'unittest', 'requires_zlib', 'skip_2to3_optimize', 'skip_unless_symlink',
 ]
@@ -280,6 +284,15 @@ def create_distribution(configfiles=()):
     return d
 
 
+def use_command(testcase, fullname):
+    """Register command at *fullname* for the duration of a test."""
+    set_command(fullname)
+    # XXX maybe set_command should return the class object
+    name = resolve_name(fullname).get_command_name()
+    # XXX maybe we need a public API to remove commands
+    testcase.addCleanup(_COMMANDS.__delitem__, name)
+
+
 def fake_dec(*args, **kw):
     """Fake decorator"""
     def _wrap(func):
diff --git a/Lib/packaging/tests/test_config.py b/Lib/packaging/tests/test_config.py
index e45fc11..5582f9b 100644
--- a/Lib/packaging/tests/test_config.py
+++ b/Lib/packaging/tests/test_config.py
@@ -183,13 +183,14 @@ class FooBarBazTest:
 
     def __init__(self, dist):
         self.distribution = dist
+        self._record = []
 
     @classmethod
     def get_command_name(cls):
         return 'foo'
 
     def run(self):
-        self.distribution.foo_was_here = True
+        self._record.append('foo has run')
 
     def nothing(self):
         pass
@@ -491,10 +492,12 @@ class ConfigTestCase(support.TempdirManager,
             self.write_file((pkg, '__init__.py'), '#')
 
         # try to run the install command to see if foo is called
+        self.addCleanup(command._COMMANDS.__delitem__, 'foo')
         dist = self.get_dist()
-        self.assertIn('foo', command.get_command_names())
-        self.assertEqual('FooBarBazTest',
-                         dist.get_command_obj('foo').__class__.__name__)
+        dist.run_command('install_dist')
+        cmd = dist.get_command_obj('foo')
+        self.assertEqual(cmd.__class__.__name__, 'FooBarBazTest')
+        self.assertEqual(cmd._record, ['foo has run'])
 
 
 def test_suite():
diff --git a/Lib/packaging/tests/test_dist.py b/Lib/packaging/tests/test_dist.py
index 3089f36..4cac842 100644
--- a/Lib/packaging/tests/test_dist.py
+++ b/Lib/packaging/tests/test_dist.py
@@ -6,30 +6,32 @@ import textwrap
 import packaging.dist
 
 from packaging.dist import Distribution
-from packaging.command import set_command, _COMMANDS
 from packaging.command.cmd import Command
 from packaging.errors import PackagingModuleError, PackagingOptionError
 from packaging.tests import captured_stdout
 from packaging.tests import support, unittest
-from packaging.tests.support import create_distribution
+from packaging.tests.support import create_distribution, use_command
 from test.support import unload
 
 
 class test_dist(Command):
-    """Sample packaging extension command."""
+    """Custom command used for testing."""
 
     user_options = [
-        ("sample-option=", "S", "help text"),
+        ('sample-option=', 'S',
+         "help text"),
         ]
 
     def initialize_options(self):
         self.sample_option = None
+        self._record = []
 
     def finalize_options(self):
-        pass
+        if self.sample_option is None:
+            self.sample_option = 'default value'
 
     def run(self):
-        pass
+        self._record.append('test_dist has run')
 
 
 class DistributionTestCase(support.TempdirManager,
@@ -45,14 +47,10 @@ class DistributionTestCase(support.TempdirManager,
         # (defaulting to sys.argv)
         self.argv = sys.argv, sys.argv[:]
         del sys.argv[1:]
-        self._commands = _COMMANDS.copy()
 
     def tearDown(self):
         sys.argv = self.argv[0]
         sys.argv[:] = self.argv[1]
-        # XXX maybe we need a public API to remove commands
-        _COMMANDS.clear()
-        _COMMANDS.update(self._commands)
         super(DistributionTestCase, self).tearDown()
 
     @unittest.skip('needs to be updated')
@@ -181,7 +179,8 @@ class DistributionTestCase(support.TempdirManager,
         self.write_file((temp_home, "config2.cfg"),
                         '[test_dist]\npre-hook.b = type')
 
-        set_command('packaging.tests.test_dist.test_dist')
+        use_command(self, 'packaging.tests.test_dist.test_dist')
+
         dist = create_distribution(config_files)
         cmd = dist.get_command_obj("test_dist")
         self.assertEqual(cmd.pre_hook, {"a": 'type', "b": 'type'})
@@ -209,7 +208,7 @@ class DistributionTestCase(support.TempdirManager,
                 record.append('post-%s' % cmd.get_command_name())
             '''))
 
-        set_command('packaging.tests.test_dist.test_dist')
+        use_command(self, 'packaging.tests.test_dist.test_dist')
         d = create_distribution([config_file])
         cmd = d.get_command_obj("test_dist")
 
@@ -236,7 +235,7 @@ class DistributionTestCase(support.TempdirManager,
             [test_dist]
             pre-hook.test = nonexistent.dotted.name'''))
 
-        set_command('packaging.tests.test_dist.test_dist')
+        use_command(self, 'packaging.tests.test_dist.test_dist')
         d = create_distribution([config_file])
         cmd = d.get_command_obj("test_dist")
         cmd.ensure_finalized()
@@ -251,7 +250,7 @@ class DistributionTestCase(support.TempdirManager,
             [test_dist]
             pre-hook.test = packaging.tests.test_dist.__doc__'''))
 
-        set_command('packaging.tests.test_dist.test_dist')
+        use_command(self, 'packaging.tests.test_dist.test_dist')
         d = create_distribution([config_file])
         cmd = d.get_command_obj("test_dist")
         cmd.ensure_finalized()
diff --git a/Lib/test/regrtest.py b/Lib/test/regrtest.py
index bbda525..9fe7bba 100755
--- a/Lib/test/regrtest.py
+++ b/Lib/test/regrtest.py
@@ -172,6 +172,7 @@ import io
 import json
 import logging
 import os
+import packaging.command
 import packaging.database
 import platform
 import random
@@ -967,7 +968,7 @@ class saved_test_environment:
                  'sys.warnoptions', 'threading._dangling',
                  'multiprocessing.process._dangling',
                  'sysconfig._CONFIG_VARS', 'sysconfig._SCHEMES',
-                 'packaging.database_caches',
+                 'packaging.command._COMMANDS', 'packaging.database_caches',
                 )
 
     def get_sys_argv(self):
@@ -1055,6 +1056,22 @@ class saved_test_environment:
         # Can't easily revert the logging state
         pass
 
+    def get_packaging_command__COMMANDS(self):
+        # registry mapping command names to full dotted path or to the actual
+        # class (resolved on demand); this check only looks at the names, not
+        # the types of the values (IOW, if a value changes from a string
+        # (dotted path) to a class it's okay but if a key (i.e. command class)
+        # is added we complain)
+        id_ = id(packaging.command._COMMANDS)
+        keys = set(packaging.command._COMMANDS)
+        return id_, keys
+    def restore_packaging_command__COMMANDS(self, saved):
+        # if command._COMMANDS was bound to another dict obhect, we can't
+        # restore the previous object and contents, because the get_ method
+        # above does not return the dict object (to ignore changes in values)
+        for key in packaging.command._COMMANDS.keys() - saved[1]:
+            del packaging.command._COMMANDS[key]
+
     def get_packaging_database_caches(self):
         # caching system used by the PEP 376 implementation
         # we have one boolean and four dictionaries, initially empty