Skip to content

Commit

Permalink
Prevent sessions from modifying each other's posargs (#439)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
cjolowicz authored Jun 6, 2021
1 parent 41b9c79 commit bcaa5ff
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 20 deletions.
2 changes: 1 addition & 1 deletion nox/sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down
6 changes: 4 additions & 2 deletions tests/test__option_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand All @@ -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
)
Expand Down
2 changes: 1 addition & 1 deletion tests/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand Down
37 changes: 27 additions & 10 deletions tests/test_sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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,
),
Expand All @@ -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):
Expand Down
20 changes: 14 additions & 6 deletions tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -150,22 +150,28 @@ 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
assert len(manifest) == 2


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,
Expand All @@ -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
)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit bcaa5ff

Please sign in to comment.