From 5db8c53527245f61b7094ff9c35f04670b37a15f Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Sat, 29 Jul 2023 20:31:22 +0100 Subject: [PATCH] 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 --- changelog/157.trivial.rst | 1 + src/saltfactories/utils/functional.py | 100 ++++++------------ .../utils/saltext/engines/pytest_engine.py | 8 +- src/saltfactories/utils/virtualenv.py | 2 +- .../daemons/test_container_factory.py | 2 +- tests/functional/loader/test_loader.py | 2 +- tests/functional/test_cli.py | 4 +- .../factories/daemons/container/conftest.py | 2 +- .../daemons/container/test_minion.py | 2 +- .../minion/test_event_forwarder_engine.py | 2 +- .../daemons/proxy/test_proxy_minion.py | 2 +- tests/unit/factories/cli/test_salt.py | 2 +- 12 files changed, 45 insertions(+), 84 deletions(-) create mode 100644 changelog/157.trivial.rst diff --git a/changelog/157.trivial.rst b/changelog/157.trivial.rst new file mode 100644 index 00000000..743d8ebf --- /dev/null +++ b/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 diff --git a/src/saltfactories/utils/functional.py b/src/saltfactories/utils/functional.py index 504593b9..418e71c7 100644 --- a/src/saltfactories/utils/functional.py +++ b/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): diff --git a/src/saltfactories/utils/saltext/engines/pytest_engine.py b/src/saltfactories/utils/saltext/engines/pytest_engine.py index 420522ab..bd8eb42d 100644 --- a/src/saltfactories/utils/saltext/engines/pytest_engine.py +++ b/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] diff --git a/src/saltfactories/utils/virtualenv.py b/src/saltfactories/utils/virtualenv.py index 4504569f..24d7db9c 100644 --- a/src/saltfactories/utils/virtualenv.py +++ b/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 diff --git a/tests/functional/factories/daemons/test_container_factory.py b/tests/functional/factories/daemons/test_container_factory.py index 1fadc2c0..a299d5cd 100644 --- a/tests/functional/factories/daemons/test_container_factory.py +++ b/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}") diff --git a/tests/functional/loader/test_loader.py b/tests/functional/loader/test_loader.py index fb3eda9a..c34430ca 100644 --- a/tests/functional/loader/test_loader.py +++ b/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 diff --git a/tests/functional/test_cli.py b/tests/functional/test_cli.py index 45aa24d9..2712c92f 100644 --- a/tests/functional/test_cli.py +++ b/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, diff --git a/tests/integration/factories/daemons/container/conftest.py b/tests/integration/factories/daemons/container/conftest.py index 1e95ebc4..f295f21b 100644 --- a/tests/integration/factories/daemons/container/conftest.py +++ b/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 diff --git a/tests/integration/factories/daemons/container/test_minion.py b/tests/integration/factories/daemons/container/test_minion.py index 0cf4cdfc..df7f547e 100644 --- a/tests/integration/factories/daemons/container/test_minion.py +++ b/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 diff --git a/tests/integration/factories/daemons/minion/test_event_forwarder_engine.py b/tests/integration/factories/daemons/minion/test_event_forwarder_engine.py index c6620e76..e21bab3e 100644 --- a/tests/integration/factories/daemons/minion/test_event_forwarder_engine.py +++ b/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: diff --git a/tests/integration/factories/daemons/proxy/test_proxy_minion.py b/tests/integration/factories/daemons/proxy/test_proxy_minion.py index 52ff0cea..f9a2f672 100644 --- a/tests/integration/factories/daemons/proxy/test_proxy_minion.py +++ b/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." diff --git a/tests/unit/factories/cli/test_salt.py b/tests/unit/factories/cli/test_salt.py index 1e86f476..076f0069 100644 --- a/tests/unit/factories/cli/test_salt.py +++ b/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" )