Fix problems with different test ordering (#4043) (#4051)

* Fix problems with different test ordering (#4043)

* fixes #4030

* Properly restore set_by_cli after using it

* mock out post_hook so it isn't stored

* fixes #4044

(cherry picked from commit aaa732d8f3)

* test the 0.10.x branch

.travis.yml

@@ -96,6 +96,7 @@ matrix:
 # turnaround time on review since there is a cap of 5 simultaneous runs.
 branches:
   only:
+    - 0.10.x
     - master
     - /^test-.*$/
 

certbot/tests/cli_test.py

@@ -1,12 +1,11 @@
 """Tests for certbot.cli."""
 import argparse
-import functools
 import unittest
 import os
 import tempfile
 
-import six
 import mock
+import six
 from six.moves import reload_module  # pylint: disable=import-error
 
 from certbot import cli
@@ -14,9 +13,8 @@ from certbot import constants
 from certbot import errors
 from certbot.plugins import disco
 
-def reset_set_by_cli():
-    '''Reset the state of the `set_by_cli` function'''
-    cli.set_by_cli.detector = None
+PLUGINS = disco.PluginsRegistry.find_all()
+
 
 class TestReadFile(unittest.TestCase):
     '''Test cli.read_file'''
@@ -43,13 +41,13 @@ class ParseTest(unittest.TestCase):
 
     _multiprocess_can_split_ = True
 
-    @classmethod
-    def setUpClass(cls):
-        cls.plugins = disco.PluginsRegistry.find_all()
-        cls.parse = functools.partial(cli.prepare_and_parse_args, cls.plugins)
-
     def setUp(self):
-        reset_set_by_cli()
+        reload_module(cli)
+
+    @staticmethod
+    def parse(*args, **kwargs):
+        """Get result of cli.prepare_and_parse_args."""
+        return cli.prepare_and_parse_args(PLUGINS, *args, **kwargs)
 
     def _help_output(self, args):
         "Run a command, and return the ouput string for scrutiny"
@@ -88,7 +86,7 @@ class ParseTest(unittest.TestCase):
         self.assertTrue("{0}" not in out)
 
         out = self._help_output(['-h', 'nginx'])
-        if "nginx" in self.plugins:
+        if "nginx" in PLUGINS:
             # may be false while building distributions without plugins
             self.assertTrue("--nginx-ctl" in out)
         self.assertTrue("--webroot-path" not in out)
@@ -96,7 +94,7 @@ class ParseTest(unittest.TestCase):
 
         out = self._help_output(['-h'])
         self.assertTrue("letsencrypt-auto" not in out)  # test cli.cli_command
-        if "nginx" in self.plugins:
+        if "nginx" in PLUGINS:
             self.assertTrue("Use the Nginx plugin" in out)
         else:
             self.assertTrue("(the certbot nginx plugin is not" in out)

certbot/tests/hook_test.py

@@ -5,16 +5,14 @@ import os
 import unittest
 
 import mock
+from six.moves import reload_module  # pylint: disable=import-error
 
 from certbot import errors
 from certbot import hooks
 
 class HookTest(unittest.TestCase):
     def setUp(self):
-        pass
-
-    def tearDown(self):
-        pass
+        reload_module(hooks)
 
     @mock.patch('certbot.hooks._prog')
     def test_validate_hooks(self, mock_prog):
@@ -47,7 +45,6 @@ class HookTest(unittest.TestCase):
             return mock_logger.warning
 
     def test_pre_hook(self):
-        hooks.pre_hook.already = set()
         config = mock.MagicMock(pre_hook="true")
         self._test_a_hook(config, hooks.pre_hook, 1)
         self._test_a_hook(config, hooks.pre_hook, 0)

certbot/tests/main_test.py

@@ -12,6 +12,7 @@ import datetime
 import pytz
 
 import six
+from six.moves import reload_module  # pylint: disable=import-error
 
 from acme import jose
 
@@ -422,10 +423,9 @@ class MainTest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                               '--logs-dir', self.logs_dir, '--text']
 
     def tearDown(self):
-        shutil.rmtree(self.tmp_dir)
         # Reset globals in cli
-        # pylint: disable=protected-access
-        cli._parser = cli.set_by_cli.detector = None
+        reload_module(cli)
+        shutil.rmtree(self.tmp_dir)
 
     def _call(self, args, stdout=None):
         "Run the cli with output streams and actual client mocked out"
@@ -862,8 +862,9 @@ class MainTest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         test_util.make_lineage(self, 'sample-renewal.conf')
         args = ["renew", "--dry-run", "--post-hook=no-such-command",
                 "--disable-hook-validation"]
-        self._test_renewal_common(True, [], args=args, should_renew=True,
-                                  error_expected=False)
+        with mock.patch("certbot.hooks.post_hook"):
+            self._test_renewal_common(True, [], args=args, should_renew=True,
+                                      error_expected=False)
 
     @mock.patch("certbot.cli.set_by_cli")
     def test_ancient_webroot_renewal_conf(self, mock_set_by_cli):