Repository: theacodes/nox
Pull request #229 (2019-08-05 03:50:31)

immediately exit if unknown arguments are passed
This PR adds changes to raise an error if an unknown argument is passed.

## Test plan

### before (all tests would run):
```
nox lint 
> nox 
nox > Running session tests-3.5
nox > Session tests-3.5 skipped: Python interpreter 3.5 not found.
nox > Running session tests-3.6
...
```
### after (error is raised):
```
>> nox lint
usage: nox [-h] [--version] [-l] [-s [SESSIONS [SESSIONS ...]]] [-k KEYWORDS]
           [-r] [--no-reuse-existing-virtualenvs] [-f NOXFILE]
           [--envdir ENVDIR] [-x] [--no-stop-on-first-error]
           [--error-on-missing-interpreters]
           [--no-error-on-missing-interpreters] [--error-on-external-run]
           [--no-error-on-external-run] [--install-only] [--report REPORT]
           [--non-interactive] [--nocolor] [--forcecolor]
           ...
nox: error: Unrecognized argument(s): 'lint'

>> nox -s lint
nox > Running session lint
nox > Creating virtualenv using python3.7 in .nox/lint
...
```

fixes https://github.com/theacodes/nox/issues/226
cc @theacodes 

theacodes: > This looks great - just one last question before merging. :)

Of course, we'll need CI to pass. Let me know if you run into any trouble there.
cs01: > > This looks great - just one last question before merging. :)
> 
> Of course, we'll need CI to pass. Let me know if you run into any trouble there.

I'm having trouble getting the unit test to pass. It did start failing, which is a good thing. It means the new stricter check is working! I just can't figure out how to assert the proper error was raised. Any help would be appreciated.
cs01: Tests are now passing. I also updated the flake8 rule to comply with black's formatting (and removed the `fmt: off`.

Patch:
diff --git a/.flake8 b/.flake8
index 4c0072e..aa15c81 100644
--- a/.flake8
+++ b/.flake8
@@ -1,6 +1,6 @@
 [flake8]
 # Ignore black styles.
-ignore = E501, W503
+ignore = E501, W503, E203
 # Imports
 import-order-style = google
 application-import-names = nox,tests
diff --git a/nox/_options.py b/nox/_options.py
index c2da68c..2d663da 100644
--- a/nox/_options.py
+++ b/nox/_options.py
@@ -98,6 +98,30 @@ def _color_finalizer(value, args):
     return sys.stdin.isatty()
 
 
+def _posargs_finalizer(value, args):
+    """Removes the leading "--"s in the posargs array (if any) and asserts that
+    remaining arguments came after a "--".
+    """
+    posargs = value
+    if not posargs:
+        return []
+
+    if "--" not in posargs:
+        unexpected_posargs = posargs
+        raise _option_set.ArgumentError(
+            None, "Unknown argument(s) '{}'.".format(" ".join(unexpected_posargs))
+        )
+
+    dash_index = posargs.index("--")
+    if dash_index != 0:
+        unexpected_posargs = posargs[0:dash_index]
+        raise _option_set.ArgumentError(
+            None, "Unknown argument(s) '{}'.".format(" ".join(unexpected_posargs))
+        )
+
+    return posargs[dash_index + 1 :]
+
+
 def _session_completer(prefix, parsed_args, **kwargs):
     global_config = parsed_args
     module = load_nox_module(global_config)
@@ -108,13 +132,6 @@ def _session_completer(prefix, parsed_args, **kwargs):
     ]
 
 
-def _posargs_finalizer(value, unused_args):
-    """Removes any leading "--"s in the posargs array."""
-    if value and value[0] == "--":
-        value.pop(0)
-    return value
-
-
 options.add_options(
     _option_set.Option(
         "help",
diff --git a/nox/virtualenv.py b/nox/virtualenv.py
index 836b642..9ddabeb 100644
--- a/nox/virtualenv.py
+++ b/nox/virtualenv.py
@@ -91,6 +91,39 @@ def locate_via_py(version):
             return None
 
 
+def locate_using_path_and_version(version):
+    """Check the PATH's python interpreter and return it if the version
+    matches.
+
+    On systems without version-named interpreters and with missing
+    launcher (which is on all Windows Anaconda installations),
+    we search the PATH for a plain "python" interpreter and accept it
+    if its --version matches the specified interpreter version.
+
+    Args:
+        version (str): The desired Python version. Of the form ``X.Y``.
+
+    Returns:
+        Optional[str]: The full executable path for the Python ``version``,
+        if it is found.
+    """
+    if not version:
+        return None
+
+    script = "import platform; print(platform.python_version())"
+    path_python = py.path.local.sysfind("python")
+    if path_python:
+        try:
+            prefix = "{}.".format(version)
+            version_string = path_python.sysexec("-c", script).strip()
+            if version_string.startswith(prefix):
+                return str(path_python)
+        except py.process.cmdexec.Error:
+            return None
+
+    return None
+
+
 def _clean_location(self):
     """Deletes any existing path-based environment"""
     if os.path.exists(self.location):
@@ -257,11 +290,15 @@ class VirtualEnv(ProcessEnv):
             xy_version = cleaned_interpreter
 
         path_from_launcher = locate_via_py(xy_version)
-
         if path_from_launcher:
             self._resolved = path_from_launcher
             return self._resolved
 
+        path_from_version_param = locate_using_path_and_version(xy_version)
+        if path_from_version_param:
+            self._resolved = path_from_version_param
+            return self._resolved
+
         # If we got this far, then we were unable to resolve the interpreter
         # to an actual executable; raise an exception.
         self._resolved = InterpreterNotFound(self.interpreter)


Test patch:
diff --git a/tests/test_main.py b/tests/test_main.py
index 07486ec..a48ee7f 100644
--- a/tests/test_main.py
+++ b/tests/test_main.py
@@ -166,20 +166,27 @@ def test_main_session_from_nox_env_var(monkeypatch, env, sessions):
             assert session in config.sessions
 
 
-def test_main_positional_args(monkeypatch):
-    monkeypatch.setattr(sys, "argv", [sys.executable, "1", "2", "3"])
-    with mock.patch("nox.workflow.execute") as execute:
-        execute.return_value = 0
+def test_main_positional_args(capsys, monkeypatch):
+    fake_exit = mock.Mock(side_effect=ValueError("asdf!"))
 
-        # Call the main function.
-        with mock.patch.object(sys, "exit") as exit:
-            nox.__main__.main()
-            exit.assert_called_once_with(0)
-        assert execute.called
-
-        # Verify that the positional args are listed in the config.
-        config = execute.call_args[1]["global_config"]
-        assert config.posargs == ["1", "2", "3"]
+    monkeypatch.setattr(sys, "argv", [sys.executable, "1", "2", "3"])
+    with mock.patch.object(sys, "exit", fake_exit), pytest.raises(
+        ValueError, match="asdf!"
+    ):
+        nox.__main__.main()
+    _, stderr = capsys.readouterr()
+    assert "Unknown argument(s) '1 2 3'" in stderr
+    fake_exit.assert_called_once_with(2)
+
+    fake_exit.reset_mock()
+    monkeypatch.setattr(sys, "argv", [sys.executable, "1", "2", "3", "--"])
+    with mock.patch.object(sys, "exit", fake_exit), pytest.raises(
+        ValueError, match="asdf!"
+    ):
+        nox.__main__.main()
+    _, stderr = capsys.readouterr()
+    assert "Unknown argument(s) '1 2 3'" in stderr
+    fake_exit.assert_called_once_with(2)
 
 
 def test_main_positional_with_double_hyphen(monkeypatch):
diff --git a/tests/test_virtualenv.py b/tests/test_virtualenv.py
index 61b246d..517dd6b 100644
--- a/tests/test_virtualenv.py
+++ b/tests/test_virtualenv.py
@@ -25,6 +25,7 @@ import nox.virtualenv
 
 IS_WINDOWS = nox.virtualenv._SYSTEM == "Windows"
 HAS_CONDA = shutil.which("conda") is not None
+RAISE_ERROR = "RAISE_ERROR"
 
 
 @pytest.fixture
@@ -47,6 +48,78 @@ def make_conda(tmpdir):
     return factory
 
 
+@pytest.fixture
+def make_mocked_interpreter_path():
+    """Provides a factory to create a mocked ``path`` object pointing
+    to a python interpreter.
+
+    This mocked ``path`` provides
+        - a ``__str__`` which is equal to the factory's ``path`` parameter
+        - a ``sysexec`` method which returns the value of the
+          factory's ``sysexec_result`` parameter.
+          (the ``sysexec_result`` parameter can be a version string
+          or ``RAISE_ERROR``).
+    """
+
+    def factory(path, sysexec_result):
+        def mock_sysexec(*_):
+            if sysexec_result == RAISE_ERROR:
+                raise py.process.cmdexec.Error(1, 1, "", "", "")
+            else:
+                return sysexec_result
+
+        attrs = {
+            "sysexec.side_effect": mock_sysexec,
+            "__str__": mock.Mock(return_value=path),
+        }
+        mock_python = mock.Mock()
+        mock_python.configure_mock(**attrs)
+
+        return mock_python
+
+    return factory
+
+
+@pytest.fixture
+def patch_sysfind(make_mocked_interpreter_path):
+    """Provides a function to patch ``sysfind`` with parameters for tests related
+    to locating a Python interpreter in the system ``PATH``.
+    """
+
+    def patcher(sysfind, only_find, sysfind_result, sysexec_result):
+        """Returns an extended ``sysfind`` patch object for tests related to locating a
+        Python interpreter in the system ``PATH``.
+
+        Args:
+            sysfind: The original sysfind patch object
+            only_find (Tuple[str]): The strings for which ``sysfind`` should be successful,
+                e.g. ``("python", "python.exe")``
+            sysfind_result (Optional[str]): The ``path`` string to create the returned
+                mocked ``path`` object with which will represent the found Python interpreter,
+                or ``None``.
+                This parameter is passed on to ``make_mocked_interpreter_path``.
+            sysexec_result (str): A string that should be returned when executing the
+                mocked ``path`` object. Usually a Python version string.
+                Use the global ``RAISE_ERROR`` to have ``sysexec`` fail.
+                This parameter is passed on to ``make_mocked_interpreter_path``.
+        """
+        mock_python = make_mocked_interpreter_path(sysfind_result, sysexec_result)
+
+        def mock_sysfind(arg):
+            if sysfind_result is None:
+                return None
+            elif arg.lower() in only_find:
+                return mock_python
+            else:
+                return None
+
+        sysfind.side_effect = mock_sysfind
+
+        return sysfind
+
+    return patcher
+
+
 def test_process_env_constructor():
     penv = nox.virtualenv.ProcessEnv()
     assert not penv.bin
@@ -321,7 +394,7 @@ def test__resolved_interpreter_windows_pyexe(sysfind, make_one, input_, expected
     attrs = {"sysexec.return_value": expected}
     mock_py = mock.Mock()
     mock_py.configure_mock(**attrs)
-    sysfind.side_effect = lambda arg: mock_py if arg == "py" else False
+    sysfind.side_effect = lambda arg: mock_py if arg == "py" else None
 
     # Okay now run the test.
     assert venv._resolved_interpreter == expected
@@ -344,7 +417,7 @@ def test__resolved_interpreter_windows_pyexe_fails(sysfind, make_one):
     attrs = {"sysexec.side_effect": py.process.cmdexec.Error(1, 1, "", "", "")}
     mock_py = mock.Mock()
     mock_py.configure_mock(**attrs)
-    sysfind.side_effect = lambda arg: mock_py if arg == "py" else False
+    sysfind.side_effect = lambda arg: mock_py if arg == "py" else None
 
     # Okay now run the test.
     with pytest.raises(nox.virtualenv.InterpreterNotFound):
@@ -354,6 +427,55 @@ def test__resolved_interpreter_windows_pyexe_fails(sysfind, make_one):
     sysfind.assert_any_call("py")
 
 
+@mock.patch("nox.virtualenv._SYSTEM", new="Windows")
+@mock.patch.object(py.path.local, "sysfind")
+def test__resolved_interpreter_windows_path_and_version(
+    sysfind, make_one, patch_sysfind
+):
+    # Establish that if we get a standard pythonX.Y path, we look it
+    # up via the path on Windows.
+    venv, _ = make_one(interpreter="3.7")
+
+    # Trick the system into thinking that it cannot find
+    # pythonX.Y up until the python-in-path check at the end.
+    # Also, we don't give it a mock py launcher.
+    # But we give it a mock python interpreter to find
+    # in the system path.
+    correct_path = r"c:\python37-x64\python.exe"
+    patch_sysfind(
+        sysfind,
+        only_find=("python", "python.exe"),
+        sysfind_result=correct_path,
+        sysexec_result="3.7.3\\n",
+    )
+
+    # Okay, now run the test.
+    assert venv._resolved_interpreter == correct_path
+
+
+@pytest.mark.parametrize("input_", ["2.7", "python3.7", "goofy"])
+@pytest.mark.parametrize("sysfind_result", [r"c:\python37-x64\python.exe", None])
+@pytest.mark.parametrize("sysexec_result", ["3.7.3\\n", RAISE_ERROR])
+@mock.patch("nox.virtualenv._SYSTEM", new="Windows")
+@mock.patch.object(py.path.local, "sysfind")
+def test__resolved_interpreter_windows_path_and_version_fails(
+    sysfind, input_, sysfind_result, sysexec_result, make_one, patch_sysfind
+):
+    # Establish that if we get a standard pythonX.Y path, we look it
+    # up via the path on Windows.
+    venv, _ = make_one(interpreter=input_)
+
+    # Trick the system into thinking that it cannot find
+    # pythonX.Y up until the python-in-path check at the end.
+    # Also, we don't give it a mock py launcher.
+    # But we give it a mock python interpreter to find
+    # in the system path.
+    patch_sysfind(sysfind, ("python", "python.exe"), sysfind_result, sysexec_result)
+
+    with pytest.raises(nox.virtualenv.InterpreterNotFound):
+        venv._resolved_interpreter
+
+
 @mock.patch("nox.virtualenv._SYSTEM", new="Windows")
 @mock.patch.object(py._path.local.LocalPath, "check")
 @mock.patch.object(py.path.local, "sysfind")