From bd850c079851d5458e4af82921739666545b6170 Mon Sep 17 00:00:00 2001 From: "Pradyun S. Gedam" Date: Mon, 15 May 2017 22:08:02 +0530 Subject: [PATCH] Improve tests --- pip/configuration.py | 2 +- tests/functional/test_configuration.py | 4 +- tests/lib/configuration_helpers.py | 65 ++++++--------- tests/unit/test_configuration.py | 111 +++++++++++++++++-------- 4 files changed, 103 insertions(+), 79 deletions(-) diff --git a/pip/configuration.py b/pip/configuration.py index fbefc52ac..90892391c 100644 --- a/pip/configuration.py +++ b/pip/configuration.py @@ -222,7 +222,6 @@ class Configuration(object): # Keeping track of the parsers used self._parsers[variant].append((file_, parser)) - # XXX: This is patched in the tests. def _load_file(self, variant, file_): logger.debug("For variant '%s', will try loading '%s'", variant, file_) parser = self._construct_parser(file_) @@ -269,6 +268,7 @@ class Configuration(object): if key.startswith("PIP_"): yield key[4:].lower(), val + # XXX: This is patched in the tests. def _get_config_files(self): """Yields variant and configuration files associated with it. diff --git a/tests/functional/test_configuration.py b/tests/functional/test_configuration.py index 35c118ae8..9eea5b02e 100644 --- a/tests/functional/test_configuration.py +++ b/tests/functional/test_configuration.py @@ -5,7 +5,7 @@ import textwrap import pytest from pip.status_codes import ERROR -from tests.lib.configuration_helpers import kinds, ConfigurationFileIOMixin +from tests.lib.configuration_helpers import kinds, ConfigurationMixin def test_no_options_passed_should_error(script): @@ -13,7 +13,7 @@ def test_no_options_passed_should_error(script): assert result.returncode == ERROR -class TestBasicLoading(ConfigurationFileIOMixin): +class TestBasicLoading(ConfigurationMixin): @pytest.mark.skip("Can't modify underlying file for any mode") def test_reads_file_appropriately(self, script): diff --git a/tests/lib/configuration_helpers.py b/tests/lib/configuration_helpers.py index 464ea6a26..2ec482185 100644 --- a/tests/lib/configuration_helpers.py +++ b/tests/lib/configuration_helpers.py @@ -6,7 +6,6 @@ import functools import os import tempfile import textwrap -from mock import patch import pip.configuration from pip.utils import ensure_dir @@ -15,41 +14,45 @@ from pip.utils import ensure_dir kinds = pip.configuration.kinds -class ConfigurationFileIOMixin(object): +class ConfigurationMixin(object): def setup(self): - self._patches_to_clear = [] + self.configuration = pip.configuration.Configuration(isolated=False) + self._files_to_clear = [] + + self._old_environ = os.environ.copy() def teardown(self): - for patch_obj in self._patches_to_clear: - patch_obj.stop() + for file_ in self._files_to_clear: + file_.stop() + + os.environ = self._old_environ + + def patch_configuration(self, variant, di): + old = self.configuration._load_config_files + + @functools.wraps(old) + def overidden(): + # Manual Overload + self.configuration._config[variant].update(di) + self.configuration._parsers[variant].append((None, None)) + return old() + + self.configuration._load_config_files = overidden @contextlib.contextmanager - def patched_file(self, kind, contents): + def tmpfile(self, contents): # Create a temporary file _, path = tempfile.mkstemp( - prefix="pip_", suffix="_" + kind + "config.ini", text=True + prefix="pip_", suffix="_config.ini", text=True ) - contents = textwrap.dedent(contents) + contents = textwrap.dedent(contents).lstrip() ensure_dir(os.path.dirname(path)) with open(path, "w") as f: f.write(contents) - # Convert kind to attribute - kind_to_attribute_mapping = { - kinds.VENV: "pip.configuration.venv_config_file", - kinds.USER: "pip.configuration.new_config_file", - kinds.GLOBAL: "pip.configuration.site_config_files", - } - - # Patch the attribute - # FIXME: This won't work. There's subprocesses! - patch_to_make = patch(kind_to_attribute_mapping[kind], path) - patch_to_make.start() - self._patches_to_clear.append(patch_to_make) - - yield + yield path os.remove(path) @@ -57,21 +60,3 @@ class ConfigurationFileIOMixin(object): def get_file_contents(path): with open(path) as f: return f.read() - - -class ConfigurationPatchingMixin(object): - - def patch_configuration(self, variant, di): - old = self.configuration._load_file - - @functools.wraps(old) - def overidden(v, file_): - if variant == v: - self.configuration._config[v].update(di) - return object() - else: - return old(v, file_) - self.configuration._load_file = overidden - - def setup(self): - self.configuration = pip.configuration.Configuration(isolated=False) diff --git a/tests/unit/test_configuration.py b/tests/unit/test_configuration.py index 366a5e621..23e92c59f 100644 --- a/tests/unit/test_configuration.py +++ b/tests/unit/test_configuration.py @@ -2,99 +2,138 @@ """ import os -import tempfile -import textwrap from mock import MagicMock from pip.locations import venv_config_file, new_config_file, site_config_files from pip.exceptions import ConfigurationError -from tests.lib.configuration_helpers import kinds, ConfigurationPatchingMixin +from tests.lib.configuration_helpers import kinds, ConfigurationMixin -class TestConfigurationLoading(ConfigurationPatchingMixin): +class TestConfigurationLoading(ConfigurationMixin): def test_global_loading(self): self.patch_configuration(kinds.GLOBAL, {"test.hello": "1"}) + self.configuration.load() assert self.configuration.get_value("test.hello") == "1" def test_user_loading(self): self.patch_configuration(kinds.USER, {"test.hello": "2"}) + self.configuration.load() assert self.configuration.get_value("test.hello") == "2" def test_venv_loading(self): self.patch_configuration(kinds.VENV, {"test.hello": "3"}) + self.configuration.load() assert self.configuration.get_value("test.hello") == "3" def test_environment_config_loading(self): - contents = textwrap.dedent("""\ + contents = """ [test] hello = 4 - """) + """ - _, config_file = tempfile.mkstemp('-pip.cfg', 'test-') - os.environ["PIP_CONFIG_FILE"] = config_file - with open(config_file, "w") as f: - f.write(contents) + with self.tmpfile(contents) as config_file: + os.environ["PIP_CONFIG_FILE"] = config_file + + self.configuration.load() + assert self.configuration.get_value("test.hello") == "4", \ + self.configuration._config + + def test_environment_var_loading(self): + os.environ["PIP_HELLO"] = "5" self.configuration.load() - assert self.configuration.get_value("test.hello") == "4" + assert self.configuration.get_value(":env:.hello") == "5" -class TestConfigurationPrecedence(ConfigurationPatchingMixin): +class TestConfigurationPrecedence(ConfigurationMixin): # Tests for methods to that determine the order of precedence of # configuration options - def test_global_overriden_by_user(self): - self.patch_configuration(kinds.GLOBAL, {"test.hello": "1"}) + def test_env_overides_venv(self): + self.patch_configuration(kinds.VENV, {"test.hello": "1"}) + self.patch_configuration(kinds.ENV, {"test.hello": "0"}) + self.configuration.load() + + assert self.configuration.get_value("test.hello") == "0" + + def test_env_overides_user(self): self.patch_configuration(kinds.USER, {"test.hello": "2"}) + self.patch_configuration(kinds.ENV, {"test.hello": "0"}) self.configuration.load() - assert self.configuration.get_value("test.hello") == "2" + assert self.configuration.get_value("test.hello") == "0" - def test_global_overriden_by_venv(self): - self.patch_configuration(kinds.GLOBAL, {"test.hello": "1"}) - self.patch_configuration(kinds.VENV, {"test.hello": "3"}) + def test_env_overides_global(self): + self.patch_configuration(kinds.GLOBAL, {"test.hello": "3"}) + self.patch_configuration(kinds.ENV, {"test.hello": "0"}) self.configuration.load() - assert self.configuration.get_value("test.hello") == "3" + assert self.configuration.get_value("test.hello") == "0" - def test_user_overriden_by_venv(self): + def test_venv_overides_user(self): self.patch_configuration(kinds.USER, {"test.hello": "2"}) - self.patch_configuration(kinds.VENV, {"test.hello": "3"}) - self.configuration.load() - - assert self.configuration.get_value("test.hello") == "3" - - def test_global_not_overriden_by_environment_var(self): - self.patch_configuration(kinds.GLOBAL, {"test.hello": "1"}) - os.environ["PIP_HELLO"] = "4" + self.patch_configuration(kinds.VENV, {"test.hello": "1"}) self.configuration.load() assert self.configuration.get_value("test.hello") == "1" - assert self.configuration.get_value(":env:.hello") == "4" - def test_user_not_overriden_by_environment_var(self): + def test_venv_overides_global(self): + self.patch_configuration(kinds.GLOBAL, {"test.hello": "3"}) + self.patch_configuration(kinds.VENV, {"test.hello": "1"}) + self.configuration.load() + + assert self.configuration.get_value("test.hello") == "1" + + def test_user_overides_global(self): + self.patch_configuration(kinds.GLOBAL, {"test.hello": "3"}) self.patch_configuration(kinds.USER, {"test.hello": "2"}) - os.environ["PIP_HELLO"] = "4" self.configuration.load() assert self.configuration.get_value("test.hello") == "2" - assert self.configuration.get_value(":env:.hello") == "4" + + def test_env_not_overriden_by_environment_var(self): + self.patch_configuration(kinds.ENV, {"test.hello": "1"}) + os.environ["PIP_HELLO"] = "5" + + self.configuration.load() + + assert self.configuration.get_value("test.hello") == "1" + assert self.configuration.get_value(":env:.hello") == "5" def test_venv_not_overriden_by_environment_var(self): - self.patch_configuration(kinds.VENV, {"test.hello": "3"}) - os.environ["PIP_HELLO"] = "4" + self.patch_configuration(kinds.VENV, {"test.hello": "2"}) + os.environ["PIP_HELLO"] = "5" + + self.configuration.load() + + assert self.configuration.get_value("test.hello") == "2" + assert self.configuration.get_value(":env:.hello") == "5" + + def test_user_not_overriden_by_environment_var(self): + self.patch_configuration(kinds.USER, {"test.hello": "3"}) + os.environ["PIP_HELLO"] = "5" + self.configuration.load() assert self.configuration.get_value("test.hello") == "3" - assert self.configuration.get_value(":env:.hello") == "4" + assert self.configuration.get_value(":env:.hello") == "5" + + def test_global_not_overriden_by_environment_var(self): + self.patch_configuration(kinds.GLOBAL, {"test.hello": "4"}) + os.environ["PIP_HELLO"] = "5" + + self.configuration.load() + + assert self.configuration.get_value("test.hello") == "4" + assert self.configuration.get_value(":env:.hello") == "5" -class TestConfigurationModification(ConfigurationPatchingMixin): +class TestConfigurationModification(ConfigurationMixin): # Tests for methods to that modify the state of a Configuration def test_no_specific_given_modification(self):