Improve code coverage

By either removing code not getting used anymore or marking sections of the code which are not expected to be covered.

Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>

changelog/157.trivial.rst

@@ -0,0 +1 @@
+Improve code coverage by either removing code not getting used anymore or marking sections of the code which are not expected to be covered

src/saltfactories/utils/functional.py

@@ -136,16 +136,11 @@ def grains(self):
         import salt.loader  # pylint: disable=import-outside-toplevel
 
         if self._grains is None:
-            try:
-                self._grains = salt.loader.grains(  # pylint: disable=unexpected-keyword-arg
-                    self.opts,
-                    context=self.context,
-                    loaded_base_name=self.loaded_base_name,
-                )
-            except TypeError:
-                # Salt < 3005
-                with mock.patch(PATCH_TARGET, self.loaded_base_name):
-                    self._grains = salt.loader.grains(self.opts, context=self.context)
+            self._grains = salt.loader.grains(
+                self.opts,
+                context=self.context,
+                loaded_base_name=self.loaded_base_name,
+            )
         return self._grains
 
     @property
@@ -158,16 +153,11 @@ def utils(self):
         import salt.loader  # pylint: disable=import-outside-toplevel
 
         if self._utils is None:
-            try:
-                self._utils = salt.loader.utils(  # pylint: disable=unexpected-keyword-arg
-                    self.opts,
-                    context=self.context,
-                    loaded_base_name=self.loaded_base_name,
-                )
-            except TypeError:
-                # Salt < 3005
-                with mock.patch(PATCH_TARGET, self.loaded_base_name):
-                    self._utils = salt.loader.utils(self.opts, context=self.context)
+            self._utils = salt.loader.utils(
+                self.opts,
+                context=self.context,
+                loaded_base_name=self.loaded_base_name,
+            )
         return self._utils
 
     @property
@@ -238,17 +228,10 @@ def serializers(self):
         import salt.loader  # pylint: disable=import-outside-toplevel
 
         if self._serializers is None:
-            try:
-                self._serializers = (
-                    salt.loader.serializers(  # pylint: disable=unexpected-keyword-arg
-                        self.opts,
-                        loaded_base_name=self.loaded_base_name,
-                    )
-                )
-            except TypeError:
-                # Salt < 3005
-                with mock.patch(PATCH_TARGET, self.loaded_base_name):
-                    self._serializers = salt.loader.serializers(self.opts)
+            self._serializers = salt.loader.serializers(
+                self.opts,
+                loaded_base_name=self.loaded_base_name,
+            )
         return self._serializers
 
     @property
@@ -261,25 +244,14 @@ def states(self):
         import salt.loader  # pylint: disable=import-outside-toplevel
 
         if self._states is None:
-            try:
-                _states = salt.loader.states(  # pylint: disable=unexpected-keyword-arg
-                    self.opts,
-                    functions=self.modules,
-                    utils=self.utils,
-                    serializers=self.serializers,
-                    context=self.context,
-                    loaded_base_name=self.loaded_base_name,
-                )
-            except TypeError:
-                # Salt < 3005
-                with mock.patch(PATCH_TARGET, self.loaded_base_name):
-                    _states = salt.loader.states(
-                        self.opts,
-                        functions=self.modules,
-                        utils=self.utils,
-                        serializers=self.serializers,
-                        context=self.context,
-                    )
+            _states = salt.loader.states(
+                self.opts,
+                functions=self.modules,
+                utils=self.utils,
+                serializers=self.serializers,
+                context=self.context,
+                loaded_base_name=self.loaded_base_name,
+            )
             # For state execution modules, because we'd have to almost copy/paste what salt.modules.state.single
             # does, we actually "proxy" the call through salt.modules.state.single instead of calling the state
             # execution modules directly. This was also how the non pytest test suite worked
@@ -327,26 +299,14 @@ def pillar(self):
         # onedir build in salt's repo checkout.
         import salt.pillar  # pylint: disable=import-outside-toplevel
 
-        if self._pillar is None:
-            try:
-                self._pillar = salt.pillar.get_pillar(  # pylint: disable=unexpected-keyword-arg
-                    self.opts,
-                    self.grains,
-                    self.opts["id"],
-                    saltenv=self.opts["saltenv"],
-                    pillarenv=self.opts.get("pillarenv"),
-                    loaded_base_name=self.loaded_base_name,
-                ).compile_pillar()
-            except TypeError:
-                # Salt < 3005
-                with mock.patch(PATCH_TARGET, self.loaded_base_name):
-                    self._pillar = salt.pillar.get_pillar(
-                        self.opts,
-                        self.grains,
-                        self.opts["id"],
-                        saltenv=self.opts["saltenv"],
-                        pillarenv=self.opts.get("pillarenv"),
-                    ).compile_pillar()
+        with mock.patch(PATCH_TARGET, self.loaded_base_name):
+            self._pillar = salt.pillar.get_pillar(
+                self.opts,
+                self.grains,
+                self.opts["id"],
+                saltenv=self.opts["saltenv"],
+                pillarenv=self.opts.get("pillarenv"),
+            ).compile_pillar()
         return self._pillar
 
     def refresh_pillar(self):

src/saltfactories/utils/saltext/engines/pytest_engine.py

@@ -18,7 +18,7 @@
 
 try:
     from salt.utils.data import CaseInsensitiveDict
-except ImportError:
+except ImportError:  # pragma: no cover
     CaseInsensitiveDict = None
 
 try:
@@ -34,13 +34,13 @@
 
 
 def __virtual__():
-    if HAS_MSGPACK is False:
+    if HAS_MSGPACK is False:  # pragma: no cover
         return False, "msgpack was not importable. Please install msgpack."
-    if "__role" not in __opts__:
+    if "__role" not in __opts__:  # pragma: no cover
         return False, "The required '__role' key could not be found in the options dictionary"
     role = __opts__["__role"]
     pytest_key = "pytest-{}".format(role)
-    if pytest_key not in __opts__:
+    if pytest_key not in __opts__:  # pragma: no cover
         return False, "No '{}' key in opts dictionary".format(pytest_key)
 
     pytest_config = __opts__[pytest_key]

src/saltfactories/utils/virtualenv.py

@@ -81,7 +81,7 @@ def __enter__(self):
         """
         try:
             self._create_virtualenv()
-        except subprocess.CalledProcessError as exc:
+        except subprocess.CalledProcessError as exc:  # pragma: no cover
             msg = "Failed to create virtualenv"
             raise AssertionError(msg) from exc
         return self

tests/functional/factories/daemons/test_container_factory.py

@@ -12,7 +12,7 @@ def _connectable_docker_client():
     try:
         client = docker.from_env()
         connectable = Container.client_connectable(client)
-        if not connectable:
+        if not connectable:  # pragma: no cover
             pytest.skip(connectable)
     except docker.errors.DockerException as exc:
         pytest.skip(f"Failed to instantiate a docker client: {exc}")

tests/functional/loader/test_loader.py

@@ -4,7 +4,7 @@
 
 try:
     saltfactories.__salt__  # pylint: disable=pointless-statement
-    HAS_SALT_DUNDER = True
+    HAS_SALT_DUNDER = True  # pragma: no cover
 except AttributeError:
     HAS_SALT_DUNDER = False
 

tests/functional/test_cli.py

@@ -22,7 +22,7 @@ def cmdline_ids(value):
     ids=cmdline_ids,
 )
 def test_salt_factories_cli(cmdline):
-    if not shutil.which(cmdline[0]):
+    if not shutil.which(cmdline[0]):  # pragma: no cover
         pytest.skip(f"binary {cmdline[0]} not found")
     ret = subprocess.run(
         cmdline,
@@ -44,7 +44,7 @@ def test_salt_factories_cli(cmdline):
     ids=cmdline_ids,
 )
 def test_salt_factories_cli_show_help(cmdline):
-    if not shutil.which(cmdline[0]):
+    if not shutil.which(cmdline[0]):  # pragma: no cover
         pytest.skip(f"binary {cmdline[0]} not found")
     ret = subprocess.run(
         cmdline,

tests/integration/factories/daemons/container/conftest.py

@@ -10,7 +10,7 @@
 def docker_client():
     try:
         client = docker.from_env()
-    except DockerException:
+    except DockerException:  # pragma: no cover
         pytest.skip("Failed to get a connection to docker running on the system")
     connectable = Container.client_connectable(client)
     if connectable is not True:  # pragma: no cover

tests/integration/factories/daemons/container/test_minion.py

@@ -24,7 +24,7 @@
 
 @pytest.fixture(scope="session")
 def docker_client(salt_factories, docker_client):
-    if salt_factories.system_service:
+    if salt_factories.system_service:  # pragma: no cover
         msg = "Test should not run against system install of Salt"
         raise pytest.skip.Exception(msg, _use_item_location=True)
     return docker_client

tests/integration/factories/daemons/minion/test_event_forwarder_engine.py

@@ -45,7 +45,7 @@ def test_event_listener_engine(minion, salt_call_cli, event_listener):
     expected_tag = salt.defaults.events.MINION_PILLAR_REFRESH_COMPLETE
     master_event_pattern = (minion.id, expected_tag)
     while True:
-        if time.time() > stop_time:
+        if time.time() > stop_time:  # pragma: no cover
             pytest.fail("Failed to receive the refresh pillar event.")
 
         if not master_event:

tests/integration/factories/daemons/proxy/test_proxy_minion.py

@@ -51,7 +51,7 @@ def test_no_match(proxy_minion, salt_cli):
 
 
 def test_show_jid(proxy_minion, salt_cli):
-    if platform.is_darwin() and sys.version_info[:2] == (3, 7):
+    if platform.is_darwin() and sys.version_info[:2] == (3, 7):  # pragma: no cover
         pytest.skip(
             "This test passes on Darwin under Py3.7, it has the expected output "
             "and yet, it times out. Will investigate later."

tests/unit/factories/cli/test_salt.py

@@ -66,7 +66,7 @@ def test_missing_minion_id_does_not_raise_exception(
     proc = Salt(script_name=cli_script_name, config=config)
     try:
         proc.cmdline(*args)
-    except RuntimeError:
+    except RuntimeError:  # pragma: no cover
         pytest.fail(
             f"The Salt class raised RuntimeError when the CLI flag '{flag}' was present in args"
         )