From 5f31177076ada1367a66a28c65af4658636b244d Mon Sep 17 00:00:00 2001 From: Tom Fleet Date: Fri, 8 Oct 2021 14:45:48 +0100 Subject: [PATCH 1/4] Replace to be deprecated load_module --- nox/tasks.py | 12 +++++++++--- tests/test_tasks.py | 8 ++------ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/nox/tasks.py b/nox/tasks.py index 918f3f18..83702f76 100644 --- a/nox/tasks.py +++ b/nox/tasks.py @@ -13,10 +13,11 @@ # limitations under the License. import ast -import importlib.machinery +import importlib.util import io import json import os +import sys import types from argparse import Namespace from typing import List, Union @@ -64,9 +65,14 @@ def load_nox_module(global_config: Namespace) -> Union[types.ModuleType, int]: # guess. The original working directory (the directory that Nox was # invoked from) gets stored by the .invoke_from "option" in _options. os.chdir(noxfile_parent_dir) - return importlib.machinery.SourceFileLoader( + + spec = importlib.util.spec_from_file_location( "user_nox_module", global_config.noxfile - ).load_module() + ) + module = importlib.util.module_from_spec(spec) + sys.modules["user_nox_module"] = module + spec.loader.exec_module(module) + return module except (VersionCheckFailed, InvalidVersionSpecifier) as error: logger.error(str(error)) diff --git a/tests/test_tasks.py b/tests/test_tasks.py index 6ce4e503..6c1889d3 100644 --- a/tests/test_tasks.py +++ b/tests/test_tasks.py @@ -95,9 +95,7 @@ def test_load_nox_module_IOError(caplog): our_noxfile = Path(__file__).parent.parent.joinpath("noxfile.py") config = _options.options.namespace(noxfile=str(our_noxfile)) - with mock.patch( - "nox.tasks.importlib.machinery.SourceFileLoader.load_module" - ) as mock_load: + with mock.patch("nox.tasks.importlib.util.module_from_spec") as mock_load: mock_load.side_effect = IOError assert tasks.load_nox_module(config) == 2 @@ -112,9 +110,7 @@ def test_load_nox_module_OSError(caplog): our_noxfile = Path(__file__).parent.parent.joinpath("noxfile.py") config = _options.options.namespace(noxfile=str(our_noxfile)) - with mock.patch( - "nox.tasks.importlib.machinery.SourceFileLoader.load_module" - ) as mock_load: + with mock.patch("nox.tasks.importlib.util.module_from_spec") as mock_load: mock_load.side_effect = OSError assert tasks.load_nox_module(config) == 2 From 25d6565b1a966e0698f81c118a7305fef6b9f427 Mon Sep 17 00:00:00 2001 From: Tom Fleet Date: Fri, 8 Oct 2021 14:54:43 +0100 Subject: [PATCH 2/4] Satisfy mypy (sort of) --- nox/tasks.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/nox/tasks.py b/nox/tasks.py index 83702f76..2b32fa4b 100644 --- a/nox/tasks.py +++ b/nox/tasks.py @@ -69,9 +69,21 @@ def load_nox_module(global_config: Namespace) -> Union[types.ModuleType, int]: spec = importlib.util.spec_from_file_location( "user_nox_module", global_config.noxfile ) + if not spec: + raise IOError(f"Could not get module spec from {global_config.noxfile}") + module = importlib.util.module_from_spec(spec) + if not module: + raise IOError( + f"Noxfile {global_config.noxfile} is not a valid python module." + ) sys.modules["user_nox_module"] = module - spec.loader.exec_module(module) + loader = spec.loader + if not loader: + raise IOError(f"Could not get module loader for {global_config.noxfile}") + # See https://docs.python.org/3/library/importlib.html#importing-a-source-file-directly + # unsure why mypy doesn't like this + loader.exec_module(module) # type: ignore return module except (VersionCheckFailed, InvalidVersionSpecifier) as error: From 54cc335cccf0b61ebf7f91d91ea8d85ff946b656 Mon Sep 17 00:00:00 2001 From: Tom Fleet Date: Fri, 8 Oct 2021 15:16:00 +0100 Subject: [PATCH 3/4] Add tests to cover new statements --- tests/test_tasks.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/test_tasks.py b/tests/test_tasks.py index 6c1889d3..94262751 100644 --- a/tests/test_tasks.py +++ b/tests/test_tasks.py @@ -117,6 +117,39 @@ def test_load_nox_module_OSError(caplog): assert "Failed to load Noxfile" in caplog.text +def test_load_nox_module_invalid_spec(caplog, tmp_path): + bad_noxfile = tmp_path / "badspec.py" + config = _options.options.namespace(noxfile=str(bad_noxfile)) + + with mock.patch("nox.tasks.importlib.util.spec_from_file_location") as mock_spec: + mock_spec.return_value = None + + assert tasks.load_nox_module(config) == 2 + assert "Failed to load Noxfile" in caplog.text + + +def test_load_nox_module_invalid_module(caplog, tmp_path): + bad_noxfile = tmp_path / "badspec.py" + config = _options.options.namespace(noxfile=str(bad_noxfile)) + + with mock.patch("nox.tasks.importlib.util.module_from_spec") as mock_spec: + mock_spec.return_value = None + + assert tasks.load_nox_module(config) == 2 + assert "Failed to load Noxfile" in caplog.text + + +def test_load_nox_module_invalid_module_loader(caplog, tmp_path): + bad_noxfile = tmp_path / "badspec.py" + config = _options.options.namespace(noxfile=str(bad_noxfile)) + + with mock.patch("nox.tasks.importlib.machinery.ModuleSpec") as mock_spec: + mock_spec.loader = None + + assert tasks.load_nox_module(config) == 2 + assert "Failed to load Noxfile" in caplog.text + + @pytest.fixture def reset_needs_version(): """Do not leak ``nox.needs_version`` between tests.""" From 5ba08a02b773021070d542506abf3103bd27d652 Mon Sep 17 00:00:00 2001 From: Tom Fleet Date: Fri, 8 Oct 2021 15:53:57 +0100 Subject: [PATCH 4/4] Factor out the loader function and test seperately --- nox/tasks.py | 56 ++++++++++++++++++++++++++++++--------------- tests/test_tasks.py | 31 ++++++++----------------- 2 files changed, 47 insertions(+), 40 deletions(-) diff --git a/nox/tasks.py b/nox/tasks.py index 2b32fa4b..e237e4d3 100644 --- a/nox/tasks.py +++ b/nox/tasks.py @@ -32,6 +32,42 @@ from nox.sessions import Result +def _load_and_exec_nox_module(global_config: Namespace) -> types.ModuleType: + """ + Loads, executes, then returns the global_config nox module. + + Args: + global_config (Namespace): The global config. + + Raises: + IOError: If the nox module cannot be loaded. This + exception is chosen such that it will be caught + by load_nox_module and logged appropriately. + + Returns: + types.ModuleType: The initialised nox module. + """ + spec = importlib.util.spec_from_file_location( + "user_nox_module", global_config.noxfile + ) + if not spec: + raise IOError(f"Could not get module spec from {global_config.noxfile}") + + module = importlib.util.module_from_spec(spec) + if not module: + raise IOError(f"Noxfile {global_config.noxfile} is not a valid python module.") + + sys.modules["user_nox_module"] = module + + loader = spec.loader + if not loader: # pragma: no cover + raise IOError(f"Could not get module loader for {global_config.noxfile}") + # See https://docs.python.org/3/library/importlib.html#importing-a-source-file-directly + # unsure why mypy doesn't like this + loader.exec_module(module) # type: ignore + return module + + def load_nox_module(global_config: Namespace) -> Union[types.ModuleType, int]: """Load the user's noxfile and return the module object for it. @@ -66,25 +102,7 @@ def load_nox_module(global_config: Namespace) -> Union[types.ModuleType, int]: # invoked from) gets stored by the .invoke_from "option" in _options. os.chdir(noxfile_parent_dir) - spec = importlib.util.spec_from_file_location( - "user_nox_module", global_config.noxfile - ) - if not spec: - raise IOError(f"Could not get module spec from {global_config.noxfile}") - - module = importlib.util.module_from_spec(spec) - if not module: - raise IOError( - f"Noxfile {global_config.noxfile} is not a valid python module." - ) - sys.modules["user_nox_module"] = module - loader = spec.loader - if not loader: - raise IOError(f"Could not get module loader for {global_config.noxfile}") - # See https://docs.python.org/3/library/importlib.html#importing-a-source-file-directly - # unsure why mypy doesn't like this - loader.exec_module(module) # type: ignore - return module + return _load_and_exec_nox_module(global_config) except (VersionCheckFailed, InvalidVersionSpecifier) as error: logger.error(str(error)) diff --git a/tests/test_tasks.py b/tests/test_tasks.py index 94262751..2df4cfd5 100644 --- a/tests/test_tasks.py +++ b/tests/test_tasks.py @@ -117,37 +117,26 @@ def test_load_nox_module_OSError(caplog): assert "Failed to load Noxfile" in caplog.text -def test_load_nox_module_invalid_spec(caplog, tmp_path): - bad_noxfile = tmp_path / "badspec.py" - config = _options.options.namespace(noxfile=str(bad_noxfile)) +def test_load_nox_module_invalid_spec(): + our_noxfile = Path(__file__).parent.parent.joinpath("noxfile.py") + config = _options.options.namespace(noxfile=str(our_noxfile)) with mock.patch("nox.tasks.importlib.util.spec_from_file_location") as mock_spec: mock_spec.return_value = None - assert tasks.load_nox_module(config) == 2 - assert "Failed to load Noxfile" in caplog.text + with pytest.raises(IOError): + tasks._load_and_exec_nox_module(config) -def test_load_nox_module_invalid_module(caplog, tmp_path): - bad_noxfile = tmp_path / "badspec.py" - config = _options.options.namespace(noxfile=str(bad_noxfile)) +def test_load_nox_module_invalid_module(): + our_noxfile = Path(__file__).parent.parent.joinpath("noxfile.py") + config = _options.options.namespace(noxfile=str(our_noxfile)) with mock.patch("nox.tasks.importlib.util.module_from_spec") as mock_spec: mock_spec.return_value = None - assert tasks.load_nox_module(config) == 2 - assert "Failed to load Noxfile" in caplog.text - - -def test_load_nox_module_invalid_module_loader(caplog, tmp_path): - bad_noxfile = tmp_path / "badspec.py" - config = _options.options.namespace(noxfile=str(bad_noxfile)) - - with mock.patch("nox.tasks.importlib.machinery.ModuleSpec") as mock_spec: - mock_spec.loader = None - - assert tasks.load_nox_module(config) == 2 - assert "Failed to load Noxfile" in caplog.text + with pytest.raises(IOError): + tasks._load_and_exec_nox_module(config) @pytest.fixture