Remove unnecessary unittest (#9596)

Now that we're using pytest more aggressively, I think we should start transitioning our tests to that style rather than continuing to use unittest. This PR removes some unnecessary uses of unittest I found.

I kept the test classes (while removing the inheritance from unittest.TestCase) where I felt like it added structure or logical grouping of tests.

I verified that pytest still finds all the tests in both this branch and master by running commands like:
```
pytest $(git diff --name-only master | grep -v windows_installer_integration_tests)
```

acme/tests/jose_test.py

@@ -6,10 +6,7 @@ import unittest
 import pytest
 
 
-class JoseTest(unittest.TestCase):
-    """Tests for acme.jose shim."""
-
-    def _test_it(self, submodule, attribute):
+def _test_it(submodule, attribute):
     if submodule:
         acme_jose_path = 'acme.jose.' + submodule
         josepy_path = 'josepy.' + submodule
@@ -32,10 +29,10 @@ class JoseTest(unittest.TestCase):
     assert acme_jose_mod is josepy_mod
     assert getattr(acme_jose_mod, attribute) is getattr(josepy_mod, attribute)
 
-    def test_top_level(self):
-        self._test_it('', 'RS512')
+def test_top_level():
+    _test_it('', 'RS512')
 
-    def test_submodules(self):
+def test_submodules():
     # This test ensures that the modules in josepy that were
     # available at the time it was moved into its own package are
     # available under acme.jose. Backwards compatibility with new
@@ -50,7 +47,7 @@ class JoseTest(unittest.TestCase):
                       ('util', 'ImmutableMap',),]
 
     for mod, attr in mods_and_attrs:
-            self._test_it(mod, attr)
+        _test_it(mod, attr)
 
 
 if __name__ == '__main__':

acme/tests/util_test.py

@@ -5,10 +5,7 @@ import unittest
 import pytest
 
 
-class MapKeysTest(unittest.TestCase):
-    """Tests for acme.util.map_keys."""
-
-    def test_it(self):
+def test_it():
     from acme.util import map_keys
     assert {'a': 'b', 'c': 'd'} == \
                      map_keys({'a': 'b', 'c': 'd'}, lambda key: key)

certbot-apache/tests/entrypoint_test.py

@@ -1,6 +1,5 @@
 """Test for certbot_apache._internal.entrypoint for override class resolution"""
 import sys
-import unittest
 from unittest import mock
 
 import pytest
@@ -9,11 +8,7 @@ from certbot_apache._internal import configurator
 from certbot_apache._internal import entrypoint
 
 
-class EntryPointTest(unittest.TestCase):
-    """Entrypoint tests"""
-
-    def test_get_configurator(self):
-
+def test_get_configurator():
     with mock.patch("certbot.util.get_os_info") as mock_info:
         for distro in entrypoint.OVERRIDE_CLASSES:
             return_value = (distro, "whatever")
@@ -25,7 +20,7 @@ class EntryPointTest(unittest.TestCase):
             assert entrypoint.get_configurator() == \
                              entrypoint.OVERRIDE_CLASSES[distro]
 
-    def test_nonexistent_like(self):
+def test_nonexistent_like():
     with mock.patch("certbot.util.get_os_info") as mock_info:
         mock_info.return_value = ("nonexistent", "irrelevant")
         with mock.patch("certbot.util.get_systemd_os_like") as mock_like:
@@ -34,7 +29,7 @@ class EntryPointTest(unittest.TestCase):
                 assert entrypoint.get_configurator() == \
                                  entrypoint.OVERRIDE_CLASSES[like]
 
-    def test_nonexistent_generic(self):
+def test_nonexistent_generic():
     with mock.patch("certbot.util.get_os_info") as mock_info:
         mock_info.return_value = ("nonexistent", "irrelevant")
         with mock.patch("certbot.util.get_systemd_os_like") as mock_like:

certbot-apache/tests/parsernode_test.py

@@ -1,7 +1,6 @@
 """ Tests for ParserNode interface """
 
 import sys
-import unittest
 
 import pytest
 
@@ -104,10 +103,8 @@ interfaces.CommentNode.register(DummyCommentNode)
 interfaces.DirectiveNode.register(DummyDirectiveNode)
 interfaces.BlockNode.register(DummyBlockNode)
 
-class ParserNodeTest(unittest.TestCase):
+def test_dummy():
     """Dummy placeholder test case for ParserNode interfaces"""
-
-    def test_dummy(self):
     dummyblock = DummyBlockNode(
         name="None",
         parameters=(),

certbot-apache/tests/parsernode_util_test.py

@@ -1,16 +1,12 @@
 """ Tests for ParserNode utils """
 import sys
-import unittest
 
 import pytest
 
 from certbot_apache._internal import parsernode_util as util
 
 
-class ParserNodeUtilTest(unittest.TestCase):
-    """Tests for ParserNode utils"""
-
-    def _setup_parsernode(self):
+def _setup_parsernode():
     """ Sets up kwargs dict for ParserNode """
     return {
         "ancestor": None,
@@ -18,41 +14,41 @@ class ParserNodeUtilTest(unittest.TestCase):
         "filepath": "/tmp",
     }
 
-    def _setup_commentnode(self):
+def _setup_commentnode():
     """ Sets up kwargs dict for CommentNode """
 
-        pn = self._setup_parsernode()
+    pn = _setup_parsernode()
     pn["comment"] = "x"
     return pn
 
-    def _setup_directivenode(self):
+def _setup_directivenode():
     """ Sets up kwargs dict for DirectiveNode """
 
-        pn = self._setup_parsernode()
+    pn = _setup_parsernode()
     pn["name"] = "Name"
     pn["parameters"] = ("first",)
     pn["enabled"] = True
     return pn
 
-    def test_unknown_parameter(self):
-        params = self._setup_parsernode()
+def test_unknown_parameter():
+    params = _setup_parsernode()
     params["unknown"] = "unknown"
     with pytest.raises(TypeError):
         util.parsernode_kwargs(params)
 
-        params = self._setup_commentnode()
+    params = _setup_commentnode()
     params["unknown"] = "unknown"
     with pytest.raises(TypeError):
         util.commentnode_kwargs(params)
 
-        params = self._setup_directivenode()
+    params = _setup_directivenode()
     params["unknown"] = "unknown"
     with pytest.raises(TypeError):
         util.directivenode_kwargs(params)
 
-    def test_parsernode(self):
-        params = self._setup_parsernode()
-        ctrl = self._setup_parsernode()
+def test_parsernode():
+    params = _setup_parsernode()
+    ctrl = _setup_parsernode()
 
     ancestor, dirty, filepath, metadata = util.parsernode_kwargs(params)
     assert ancestor == ctrl["ancestor"]
@@ -60,8 +56,8 @@ class ParserNodeUtilTest(unittest.TestCase):
     assert filepath == ctrl["filepath"]
     assert metadata == {}
 
-    def test_parsernode_from_metadata(self):
-        params = self._setup_parsernode()
+def test_parsernode_from_metadata():
+    params = _setup_parsernode()
     params.pop("filepath")
     md = {"some": "value"}
     params["metadata"] = md
@@ -70,32 +66,32 @@ class ParserNodeUtilTest(unittest.TestCase):
     _, _, _, metadata = util.parsernode_kwargs(params)
     assert metadata == md
 
-    def test_commentnode(self):
-        params = self._setup_commentnode()
-        ctrl = self._setup_commentnode()
+def test_commentnode():
+    params = _setup_commentnode()
+    ctrl = _setup_commentnode()
 
     comment, _ = util.commentnode_kwargs(params)
     assert comment == ctrl["comment"]
 
-    def test_commentnode_from_metadata(self):
-        params = self._setup_commentnode()
+def test_commentnode_from_metadata():
+    params = _setup_commentnode()
     params.pop("comment")
     params["metadata"] = {}
 
     # Just testing that error from missing required parameters is not raised
     util.commentnode_kwargs(params)
 
-    def test_directivenode(self):
-        params = self._setup_directivenode()
-        ctrl = self._setup_directivenode()
+def test_directivenode():
+    params = _setup_directivenode()
+    ctrl = _setup_directivenode()
 
     name, parameters, enabled, _ = util.directivenode_kwargs(params)
     assert name == ctrl["name"]
     assert parameters == ctrl["parameters"]
     assert enabled == ctrl["enabled"]
 
-    def test_directivenode_from_metadata(self):
-        params = self._setup_directivenode()
+def test_directivenode_from_metadata():
+    params = _setup_directivenode()
     params.pop("filepath")
     params.pop("name")
     params["metadata"] = {"irrelevant": "value"}
@@ -103,18 +99,18 @@ class ParserNodeUtilTest(unittest.TestCase):
     # Just testing that error from missing required parameters is not raised
     util.directivenode_kwargs(params)
 
-    def test_missing_required(self):
-        c_params = self._setup_commentnode()
+def test_missing_required():
+    c_params = _setup_commentnode()
     c_params.pop("comment")
     with pytest.raises(TypeError):
         util.commentnode_kwargs(c_params)
 
-        d_params = self._setup_directivenode()
+    d_params = _setup_directivenode()
     d_params.pop("ancestor")
     with pytest.raises(TypeError):
         util.directivenode_kwargs(d_params)
 
-        p_params = self._setup_parsernode()
+    p_params = _setup_parsernode()
     p_params.pop("filepath")
     with pytest.raises(TypeError):
         util.parsernode_kwargs(p_params)

certbot-ci/windows_installer_integration_tests/test_main.py

@@ -4,12 +4,11 @@ import re
 import subprocess
 import time
 from typing import Any
-import unittest
 
 import pytest
 
 
-@unittest.skipIf(os.name != 'nt', reason='Windows installer tests must be run on Windows.')
+@pytest.mark.skipif(os.name != 'nt', reason='Windows installer tests must be run on Windows.')
 def test_it(request: pytest.FixtureRequest) -> None:
     try:
         subprocess.check_call(['certbot', '--version'])

certbot/tests/compat/misc_test.py

@@ -1,6 +1,5 @@
 """Tests for certbot.compat.misc"""
 import sys
-import unittest
 from unittest import mock
 
 import pytest
@@ -8,7 +7,7 @@ import pytest
 from certbot.compat import os
 
 
-class ExecuteStatusTest(unittest.TestCase):
+class ExecuteStatusTest:
     """Tests for certbot.compat.misc.execute_command_status."""
 
     @classmethod

certbot/tests/compat/os_test.py

@@ -1,15 +1,12 @@
 """Unit test for os module."""
 import sys
-import unittest
 
 import pytest
 
 from certbot.compat import os
 
 
-class OsTest(unittest.TestCase):
-    """Unit tests for os module."""
-    def test_forbidden_methods(self):
+def test_forbidden_methods():
     # Checks for os module
     for method in ['chmod', 'chown', 'open', 'mkdir', 'makedirs', 'rename',
                    'replace', 'access', 'stat', 'fstat']:

certbot/tests/display/util_test.py

@@ -3,7 +3,6 @@ import io
 import socket
 import sys
 import tempfile
-import unittest
 from unittest import mock
 
 import pytest
@@ -12,11 +11,8 @@ from certbot import errors
 import certbot.tests.util as test_util
 
 
-class NotifyTest(unittest.TestCase):
-    """Tests for certbot.display.util.notify"""
-
 @test_util.patch_display_util()
-    def test_notify(self, mock_util):
+def test_notify(mock_util):
     from certbot.display.util import notify
     notify("Hello World")
     mock_util().notification.assert_called_with(
@@ -24,11 +20,8 @@ class NotifyTest(unittest.TestCase):
     )
 
 
-class NotificationTest(unittest.TestCase):
-    """Tests for certbot.display.util.notification"""
-
 @test_util.patch_display_util()
-    def test_notification(self, mock_util):
+def test_notification(mock_util):
     from certbot.display.util import notification
     notification("Hello World")
     mock_util().notification.assert_called_with(
@@ -36,11 +29,8 @@ class NotificationTest(unittest.TestCase):
     )
 
 
-class MenuTest(unittest.TestCase):
-    """Tests for certbot.display.util.menu"""
-
 @test_util.patch_display_util()
-    def test_menu(self, mock_util):
+def test_menu(mock_util):
     from certbot.display.util import menu
     menu("Hello World", ["one", "two"], default=0)
     mock_util().menu.assert_called_with(
@@ -48,11 +38,8 @@ class MenuTest(unittest.TestCase):
     )
 
 
-class InputTextTest(unittest.TestCase):
-    """Tests for certbot.display.util.input_text"""
-
 @test_util.patch_display_util()
-    def test_input_text(self, mock_util):
+def test_input_text(mock_util):
     from certbot.display.util import input_text
     input_text("Hello World", default="something")
     mock_util().input.assert_called_with(
@@ -60,11 +47,8 @@ class InputTextTest(unittest.TestCase):
     )
 
 
-class YesNoTest(unittest.TestCase):
-    """Tests for certbot.display.util.yesno"""
-
 @test_util.patch_display_util()
-    def test_yesno(self, mock_util):
+def test_yesno(mock_util):
     from certbot.display.util import yesno
     yesno("Hello World", default=True)
     mock_util().yesno.assert_called_with(
@@ -73,11 +57,8 @@ class YesNoTest(unittest.TestCase):
     )
 
 
-class ChecklistTest(unittest.TestCase):
-    """Tests for certbot.display.util.checklist"""
-
 @test_util.patch_display_util()
-    def test_checklist(self, mock_util):
+def test_checklist(mock_util):
     from certbot.display.util import checklist
     checklist("Hello World", ["one", "two"], default="one")
     mock_util().checklist.assert_called_with(
@@ -85,11 +66,8 @@ class ChecklistTest(unittest.TestCase):
     )
 
 
-class DirectorySelectTest(unittest.TestCase):
-    """Tests for certbot.display.util.directory_select"""
-
 @test_util.patch_display_util()
-    def test_directory_select(self, mock_util):
+def test_directory_select(mock_util):
     from certbot.display.util import directory_select
     directory_select("Hello World", default="something")
     mock_util().directory_select.assert_called_with(

certbot/tests/helpful_test.py

@@ -1,6 +1,5 @@
 """Tests for certbot.helpful_parser"""
 import sys
-import unittest
 from unittest import mock
 
 import pytest
@@ -11,7 +10,7 @@ from certbot._internal.cli import _DomainsAction
 from certbot._internal.cli import HelpfulArgumentParser
 
 
-class TestScanningFlags(unittest.TestCase):
+class TestScanningFlags:
     '''Test the prescan_for_flag method of HelpfulArgumentParser'''
     def test_prescan_no_help_flag(self):
         arg_parser = HelpfulArgumentParser(['run'], {})
@@ -40,7 +39,7 @@ class TestScanningFlags(unittest.TestCase):
                                                     arg_parser.help_topics)
         assert detected_flag is False
 
-class TestDetermineVerbs(unittest.TestCase):
+class TestDetermineVerbs:
     '''Tests for determine_verb methods of HelpfulArgumentParser'''
     def test_determine_verb_wrong_verb(self):
         arg_parser = HelpfulArgumentParser(['potato'], {})
@@ -71,7 +70,7 @@ class TestDetermineVerbs(unittest.TestCase):
         assert arg_parser.args == []
 
 
-class TestAdd(unittest.TestCase):
+class TestAdd:
     '''Tests for add method in HelpfulArgumentParser'''
     def test_add_trivial_argument(self):
         arg_parser = HelpfulArgumentParser(['run'], {})
@@ -93,7 +92,7 @@ class TestAdd(unittest.TestCase):
         assert hasattr(parsed_args, 'eab_kid')
 
 
-class TestAddGroup(unittest.TestCase):
+class TestAddGroup:
     '''Test add_group method of HelpfulArgumentParser'''
     def test_add_group_no_input(self):
         arg_parser = HelpfulArgumentParser(['run'], {})
@@ -118,7 +117,7 @@ class TestAddGroup(unittest.TestCase):
             assert arg_parser.groups["certonly"] is False
 
 
-class TestParseArgsErrors(unittest.TestCase):
+class TestParseArgsErrors:
     '''Tests for errors that should be met for some cases in parse_args method
     in HelpfulArgumentParser'''
     def test_parse_args_renew_force_interactive(self):
@@ -194,7 +193,7 @@ class TestParseArgsErrors(unittest.TestCase):
             arg_parser.parse_args()
 
 
-class TestAddDeprecatedArgument(unittest.TestCase):
+class TestAddDeprecatedArgument:
     """Tests for add_deprecated_argument method of HelpfulArgumentParser"""
 
     @mock.patch.object(HelpfulArgumentParser, "modify_kwargs_for_default_detection")

certbot/tests/plugins/util_test.py

@@ -1,6 +1,5 @@
 """Tests for certbot.plugins.util."""
 import sys
-import unittest
 from unittest import mock
 
 import pytest
@@ -8,9 +7,7 @@ import pytest
 from certbot.compat import os
 
 
-class GetPrefixTest(unittest.TestCase):
-    """Tests for certbot.plugins.get_prefixes."""
-    def test_get_prefix(self):
+def test_get_prefix():
     from certbot.plugins.util import get_prefixes
     assert get_prefixes('/a/b/c') == \
         [os.path.normpath(path) for path in ['/a/b/c', '/a/b', '/a', '/']]
@@ -18,11 +15,8 @@ class GetPrefixTest(unittest.TestCase):
     assert get_prefixes('a') == ['a']
 
 
-class PathSurgeryTest(unittest.TestCase):
-    """Tests for certbot.plugins.path_surgery."""
-
 @mock.patch("certbot.plugins.util.logger.debug")
-    def test_path_surgery(self, mock_debug):
+def test_path_surgery(mock_debug):
     from certbot.plugins.util import path_surgery
     all_path = {"PATH": "/usr/local/bin:/bin/:/usr/sbin/:/usr/local/sbin/"}
     with mock.patch.dict('os.environ', all_path):