From bcaa5ffa42173037030f67e2ae58d10fbe659945 Mon Sep 17 00:00:00 2001 From: Claudio Jolowicz Date: Sun, 6 Jun 2021 14:51:55 +0200 Subject: [PATCH] Prevent sessions from modifying each other's posargs (#439) * test: Do not compare posargs using `is` * test: Do not set posargs to mock sentinel This breaks tests because sentinels cannot be subscripted. * test: Initialize posargs to an empty list, not None * test: Add failing test for bleed-through of posargs between sessions * Give each session its own copy of posargs --- nox/sessions.py | 2 +- tests/test__option_set.py | 6 ++++-- tests/test_manifest.py | 2 +- tests/test_sessions.py | 37 +++++++++++++++++++++++++++---------- tests/test_tasks.py | 20 ++++++++++++++------ 5 files changed, 47 insertions(+), 20 deletions(-) diff --git a/nox/sessions.py b/nox/sessions.py index 31a28e75..cabaae12 100644 --- a/nox/sessions.py +++ b/nox/sessions.py @@ -506,7 +506,7 @@ def __init__( self.global_config = global_config self.manifest = manifest self.venv: Optional[ProcessEnv] = None - self.posargs: List[str] = global_config.posargs + self.posargs: List[str] = global_config.posargs[:] @property def description(self) -> Optional[str]: diff --git a/tests/test__option_set.py b/tests/test__option_set.py index ae238f5e..202473ea 100644 --- a/tests/test__option_set.py +++ b/tests/test__option_set.py @@ -56,7 +56,7 @@ def test_namespace_non_existant_options_with_values(self): optionset.namespace(non_existant_option="meep") def test_session_completer(self): - parsed_args = _options.options.namespace(sessions=(), keywords=()) + parsed_args = _options.options.namespace(sessions=(), keywords=(), posargs=[]) all_nox_sessions = _options._session_completer( prefix=None, parsed_args=parsed_args ) @@ -66,7 +66,9 @@ def test_session_completer(self): assert len(set(some_expected_sessions) - set(all_nox_sessions)) == 0 def test_session_completer_invalid_sessions(self): - parsed_args = _options.options.namespace(sessions=("baz",), keywords=()) + parsed_args = _options.options.namespace( + sessions=("baz",), keywords=(), posargs=[] + ) all_nox_sessions = _options._session_completer( prefix=None, parsed_args=parsed_args ) diff --git a/tests/test_manifest.py b/tests/test_manifest.py index e807619e..4c2cf151 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -373,7 +373,7 @@ def test_notify_with_posargs(): # delete my_session from the queue manifest.filter_by_name(()) - assert session.posargs is cfg.posargs + assert session.posargs == cfg.posargs assert manifest.notify("my_session", posargs=["--an-arg"]) assert session.posargs == ["--an-arg"] diff --git a/tests/test_sessions.py b/tests/test_sessions.py index 3573b45e..89df734e 100644 --- a/tests/test_sessions.py +++ b/tests/test_sessions.py @@ -67,9 +67,7 @@ def make_session_and_runner(self): signatures=["test"], func=func, global_config=_options.options.namespace( - posargs=mock.sentinel.posargs, - error_on_external_run=False, - install_only=False, + posargs=[], error_on_external_run=False, install_only=False ), manifest=mock.create_autospec(nox.manifest.Manifest), ) @@ -101,7 +99,7 @@ def test_properties(self): assert session.name is runner.friendly_name assert session.env is runner.venv.env - assert session.posargs is runner.global_config.posargs + assert session.posargs == runner.global_config.posargs assert session.virtualenv is runner.venv assert session.bin_paths is runner.venv.bin_paths assert session.bin is runner.venv.bin_paths[0] @@ -371,7 +369,7 @@ def test_conda_install(self, auto_offline, offline): name="test", signatures=["test"], func=mock.sentinel.func, - global_config=_options.options.namespace(posargs=mock.sentinel.posargs), + global_config=_options.options.namespace(posargs=[]), manifest=mock.create_autospec(nox.manifest.Manifest), ) runner.venv = mock.create_autospec(nox.virtualenv.CondaEnv) @@ -435,7 +433,7 @@ def test_conda_install_non_default_kwargs(self, version_constraint): name="test", signatures=["test"], func=mock.sentinel.func, - global_config=_options.options.namespace(posargs=mock.sentinel.posargs), + global_config=_options.options.namespace(posargs=[]), manifest=mock.create_autospec(nox.manifest.Manifest), ) runner.venv = mock.create_autospec(nox.virtualenv.CondaEnv) @@ -492,7 +490,7 @@ def test_install(self): name="test", signatures=["test"], func=mock.sentinel.func, - global_config=_options.options.namespace(posargs=mock.sentinel.posargs), + global_config=_options.options.namespace(posargs=[]), manifest=mock.create_autospec(nox.manifest.Manifest), ) runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv) @@ -521,7 +519,7 @@ def test_install_non_default_kwargs(self): name="test", signatures=["test"], func=mock.sentinel.func, - global_config=_options.options.namespace(posargs=mock.sentinel.posargs), + global_config=_options.options.namespace(posargs=[]), manifest=mock.create_autospec(nox.manifest.Manifest), ) runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv) @@ -556,6 +554,25 @@ def test_notify(self): runner.manifest.notify.assert_called_with("other", ["--an-arg"]) + def test_posargs_are_not_shared_between_sessions(self, monkeypatch, tmp_path): + registry = {} + monkeypatch.setattr("nox.registry._REGISTRY", registry) + + @nox.session(venv_backend="none") + def test(session): + session.posargs.extend(["-x"]) + + @nox.session(venv_backend="none") + def lint(session): + if "-x" in session.posargs: + raise RuntimeError("invalid option: -x") + + config = _options.options.namespace(posargs=[]) + manifest = nox.manifest.Manifest(registry, config) + + assert manifest["test"].execute() + assert manifest["lint"].execute() + def test_log(self, caplog): caplog.set_level(logging.INFO) session, _ = self.make_session_and_runner() @@ -628,7 +645,7 @@ def make_runner(self): global_config=_options.options.namespace( noxfile=os.path.join(os.getcwd(), "noxfile.py"), envdir="envdir", - posargs=mock.sentinel.posargs, + posargs=[], reuse_existing_virtualenvs=False, error_on_missing_interpreters=False, ), @@ -644,7 +661,7 @@ def test_properties(self): assert runner.func is not None assert callable(runner.func) assert isinstance(runner.description, str) - assert runner.global_config.posargs == mock.sentinel.posargs + assert runner.global_config.posargs == [] assert isinstance(runner.manifest, nox.manifest.Manifest) def test_str_and_friendly_name(self): diff --git a/tests/test_tasks.py b/tests/test_tasks.py index 967f605e..9827cfc8 100644 --- a/tests/test_tasks.py +++ b/tests/test_tasks.py @@ -140,7 +140,7 @@ def notasession(): mock_module = argparse.Namespace( __name__=foo.__module__, foo=foo, bar=bar, notasession=notasession ) - config = _options.options.namespace(sessions=(), keywords=()) + config = _options.options.namespace(sessions=(), keywords=(), posargs=[]) # Get the manifest and establish that it looks like what we expect. manifest = tasks.discover_manifest(mock_module, config) @@ -150,7 +150,9 @@ def notasession(): def test_filter_manifest(): - config = _options.options.namespace(sessions=(), pythons=(), keywords=()) + config = _options.options.namespace( + sessions=(), pythons=(), keywords=(), posargs=[] + ) manifest = Manifest({"foo": session_func, "bar": session_func}, config) return_value = tasks.filter_manifest(manifest, config) assert return_value is manifest @@ -158,14 +160,18 @@ def test_filter_manifest(): def test_filter_manifest_not_found(): - config = _options.options.namespace(sessions=("baz",), pythons=(), keywords=()) + config = _options.options.namespace( + sessions=("baz",), pythons=(), keywords=(), posargs=[] + ) manifest = Manifest({"foo": session_func, "bar": session_func}, config) return_value = tasks.filter_manifest(manifest, config) assert return_value == 3 def test_filter_manifest_pythons(): - config = _options.options.namespace(sessions=(), pythons=("3.8",), keywords=()) + config = _options.options.namespace( + sessions=(), pythons=("3.8",), keywords=(), posargs=[] + ) manifest = Manifest( {"foo": session_func_with_python, "bar": session_func, "baz": session_func}, config, @@ -176,7 +182,9 @@ def test_filter_manifest_pythons(): def test_filter_manifest_keywords(): - config = _options.options.namespace(sessions=(), pythons=(), keywords="foo or bar") + config = _options.options.namespace( + sessions=(), pythons=(), keywords="foo or bar", posargs=[] + ) manifest = Manifest( {"foo": session_func, "bar": session_func, "baz": session_func}, config ) @@ -231,7 +239,7 @@ def test_verify_manifest_empty(): def test_verify_manifest_nonempty(): - config = _options.options.namespace(sessions=(), keywords=()) + config = _options.options.namespace(sessions=(), keywords=(), posargs=[]) manifest = Manifest({"session": session_func}, config) return_value = tasks.verify_manifest_nonempty(manifest, global_config=config) assert return_value == manifest