From 2e3f98a95c1b2641d2f10cb6767e5e2c08de6ec9 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 26 Apr 2023 19:23:17 +0100 Subject: [PATCH] Add unit test to validate logic on calls to `destroy()` Signed-off-by: Pedro Algarvio --- salt/utils/jinja.py | 1 + .../utils/jinja/test_salt_cache_loader.py | 46 ++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/salt/utils/jinja.py b/salt/utils/jinja.py index f788a6e4dd38..a6a8a2796051 100644 --- a/salt/utils/jinja.py +++ b/salt/utils/jinja.py @@ -97,6 +97,7 @@ def file_client(self): self._file_client = salt.fileclient.get_file_client( self.opts, self.pillar_rend ) + self._close_file_client = True return self._file_client def cache_file(self, template): diff --git a/tests/pytests/unit/utils/jinja/test_salt_cache_loader.py b/tests/pytests/unit/utils/jinja/test_salt_cache_loader.py index 0cb95c1748b2..e0f5fa158ff4 100644 --- a/tests/pytests/unit/utils/jinja/test_salt_cache_loader.py +++ b/tests/pytests/unit/utils/jinja/test_salt_cache_loader.py @@ -15,7 +15,7 @@ import salt.utils.stringutils # pylint: disable=unused-import import salt.utils.yaml # pylint: disable=unused-import from salt.utils.jinja import SaltCacheLoader -from tests.support.mock import Mock, patch +from tests.support.mock import Mock, call, patch @pytest.fixture @@ -222,3 +222,47 @@ def test_file_client_kwarg(minion_opts, mock_file_client): mock_file_client.opts = minion_opts loader = SaltCacheLoader(minion_opts, _file_client=mock_file_client) assert loader._file_client is mock_file_client + + +def test_cache_loader_passed_file_client(minion_opts, mock_file_client): + """ + The shudown method can be called without raising an exception when the + file_client does not have a destroy method + """ + # Test SaltCacheLoader creating and destroying the file client created + file_client = Mock() + with patch("salt.fileclient.get_file_client", return_value=file_client): + loader = SaltCacheLoader(minion_opts) + assert loader._file_client is None + with loader: + assert loader._file_client is file_client + assert loader._file_client is None + assert file_client.mock_calls == [call.destroy()] + + # Test SaltCacheLoader reusing the file client passed + file_client = Mock() + file_client.opts = {"file_roots": minion_opts["file_roots"]} + with patch("salt.fileclient.get_file_client", return_value=Mock()): + loader = SaltCacheLoader(minion_opts, _file_client=file_client) + assert loader._file_client is file_client + with loader: + assert loader._file_client is file_client + assert loader._file_client is file_client + assert file_client.mock_calls == [] + + # Test SaltCacheLoader creating a client even though a file client was + # passed because the "file_roots" option is different, and, as such, + # the destroy method on the new file client is called, but not on the + # file client passed in. + file_client = Mock() + file_client.opts = {"file_roots": ""} + new_file_client = Mock() + with patch("salt.fileclient.get_file_client", return_value=new_file_client): + loader = SaltCacheLoader(minion_opts, _file_client=file_client) + assert loader._file_client is file_client + with loader: + assert loader._file_client is not file_client + assert loader._file_client is new_file_client + assert loader._file_client is None + assert file_client.mock_calls == [] + assert new_file_client.mock_calls == [call.destroy()]