Repository: theacodes/nox (python)

Issue:
nox.options.sessions ignored when --pythons is passed
**Describe the bug**

When `nox.options.sessions` is specified in `noxfile.py` and `--pythons` is passed on the command-line, the setting in the `noxfile.py` is ignored.

**How to reproduce**

```python
import nox

nox.options.sessions = ("tests",)
nox.options.pythons = ("3.9",)

@nox.session(python=("3.9", "3.10"))
def tests(session):
    pass

@nox.session(python=("3.9", "3.10"))
def launch_missiles(session):
    pass
```

Run `nox --python=3.10`.

**Actual behavior**

- The tests session is run on Python 3.10.
- The launch_missiles session is run on Python 3.10.

```
nox > Running session tests-3.10
nox > Creating virtual environment (virtualenv) using python3 in .nox/tests-3-10
nox > Session tests-3.10 was successful.
nox > Running session launch_missiles-3.10
nox > Creating virtual environment (virtualenv) using python3 in .nox/launch_missiles-3-10
nox > Session launch_missiles-3.10 was successful.
nox > Ran multiple sessions:
nox > * tests-3.10: success
nox > * launch_missiles-3.10: success
```

**Expected behavior**

- The tests session is run on Python 3.10.
- The launch_missiles session is not run at all.

```
nox > Running session tests-3.10
nox > Creating virtual environment (virtualenv) using python3 in .nox/tests-3-10
nox > Session tests-3.10 was successful.
```

**Discussion**

This is a follow-up to #356, which has an associated pull request in #357.

When `--pythons` was introduced, the merge logic for `--sessions` and `--keywords` was extended to the new `--pythons` option. This means that if any of these options are specified on the command line, the corresponding settings in the `noxfile.py` are ignored. This is achieved by a custom merge function `session_filters_merge_func`, which ensures that `noxfile.py` is ignored for all of these options if any of them are set on the command-line.

In the case of `--sessions` and `--keywords`, this makes sense, because these options are different means to achieve the same goal of specifying which sessions should run. For example, if `noxfile.py` contains `nox.options.keywords = "not launch_missiles"`, we want the user to still be able to trigger the session explicitly using `--session=launch_missiles`; the custom merge function achieves that. Without the custom merge function, the user would be forced to use `--keywords` instead of `--sessions`.

In the case of `--pythons`, I would argue that this behavior is less useful. While it is true that the `--pythons` option, just like the other two options, ultimately results in filtering the session manifest, the new option is particularly useful in combination with the other options, not as a way to override them. Unlike `--sessions` and `--keywords`, it is not possible to use `--pythons` to completely control which sessions are selected. Therefore it makes less sense to me to completely ignore the other options when `--pythons` is passed.

I would also argue that the current behavior violates the principle of least surprise. The example above demonstrates this: While the `noxfile.py` clearly states the intent that `launch_missiles` should only run on explicit user request, it is enough to specify a Python version on the command line to trigger the session, without specifying the session itself.

In my opinion, the merge logic for `--pythons` should be decoupled from that of `--sessions` and `--keywords`. This would change current behavior in two ways:

- The `nox.options.sessions` and `nox.options.keywords` settings would no longer be ignored when `--pythons` is passed on the command-line.
- The `nox.options.pythons` would no longer be ignored when `--sessions` or `--keywords` are passed on the command-line.

(The second point will make it possible to run specific sessions using `-s` or `-k` without having to repeat `nox.options.pythons` on the command-line.)

**Proposed solution**

As a fix, I would propose the following two changes, in addition to the change proposed in #357:

1. Revert `_session_filters_merge_func` to the state before `--pythons` was introduced. This means that it will only check if `--sessions` or `--keywords` were specified on the command-line. As a result, the `nox.options.sessions` and `nox.options.keywords` will no longer be ignored when `--pythons` is passed.
2. Declare the `--python` option without a custom merge function, i.e. omit the `merge_func` parameter. As a result, `nox.options.pythons` will no longer be ignored when `--sessions` or `--keywords` are passed.


Pull request description:
Decouple merging of --python with nox.options from --sessions and --keywords
Closes #358 

This PR is a follow-up to #357, and contains the following additional changes:

**1. Do not ignore nox.options.pythons when --{sessions,keywords} passed**

Declare the `--python` option without a custom merge function, i.e. omit the `merge_func` parameter. As a result, `nox.options.pythons` will no longer be ignored when `--sessions` or `--keywords` are passed.

**2. Do not ignore nox.options.{sessions,keywords} when --pythons passed**

Revert `_session_filters_merge_func` to the state before `--pythons` was introduced. This means that it will only check if `--sessions` or `--keywords` were specified on the command-line. As a result, the `nox.options.sessions` and `nox.options.keywords` settings will no longer be ignored when `--pythons` is passed.

Patch:
diff --git a/nox/_options.py b/nox/_options.py
index 336cff5..dcdcee2 100644
--- a/nox/_options.py
+++ b/nox/_options.py
@@ -41,21 +41,21 @@ options.add_groups(
 )
 
 
-def _session_filters_merge_func(
+def _sessions_and_keywords_merge_func(
     key: str, command_args: argparse.Namespace, noxfile_args: argparse.Namespace
 ) -> List[str]:
-    """Only return the Noxfile value for sessions/pythons/keywords if neither sessions,
-    pythons or keywords are specified on the command-line.
+    """Only return the Noxfile value for sessions/keywords if neither sessions
+    or keywords are specified on the command-line.
 
     Args:
-        key (str): This function is used for the "sessions", "pythons" and "keywords"
+        key (str): This function is used for both the "sessions" and "keywords"
             options, this allows using ``funtools.partial`` to pass the
             same function for both options.
         command_args (_option_set.Namespace): The options specified on the
             command-line.
-        noxfile_args (_option_set.Namespace): The options specified in the
+        noxfile_Args (_option_set.Namespace): The options specified in the
             Noxfile."""
-    if not any((command_args.sessions, command_args.pythons, command_args.keywords)):
+    if not command_args.sessions and not command_args.keywords:
         return getattr(noxfile_args, key)
     return getattr(command_args, key)
 
@@ -221,7 +221,7 @@ options.add_options(
         "--session",
         group=options.groups["primary"],
         noxfile=True,
-        merge_func=functools.partial(_session_filters_merge_func, "sessions"),
+        merge_func=functools.partial(_sessions_and_keywords_merge_func, "sessions"),
         nargs="*",
         default=_sessions_default,
         help="Which sessions to run. By default, all sessions will run.",
@@ -234,7 +234,6 @@ options.add_options(
         "--python",
         group=options.groups["primary"],
         noxfile=True,
-        merge_func=functools.partial(_session_filters_merge_func, "pythons"),
         nargs="*",
         help="Only run sessions that use the given python interpreter versions.",
     ),
@@ -244,7 +243,7 @@ options.add_options(
         "--keywords",
         group=options.groups["primary"],
         noxfile=True,
-        merge_func=functools.partial(_session_filters_merge_func, "keywords"),
+        merge_func=functools.partial(_sessions_and_keywords_merge_func, "keywords"),
         help="Only run sessions that match the given expression.",
     ),
     _option_set.Option(


Test patch:
diff --git a/tests/resources/noxfile_options_pythons.py b/tests/resources/noxfile_options_pythons.py
new file mode 100644
index 0000000..000d468
--- /dev/null
+++ b/tests/resources/noxfile_options_pythons.py
@@ -0,0 +1,28 @@
+# Copyright 2020 Alethea Katherine Flowers
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import nox
+
+nox.options.sessions = ["{default_session}"]
+nox.options.pythons = ["{default_python}"]
+
+
+@nox.session(python=["{default_python}", "{alternate_python}"])
+def test(session):
+    pass
+
+
+@nox.session(python=["{default_python}", "{alternate_python}"])
+def launch_rocket(session):
+    pass
diff --git a/tests/test_main.py b/tests/test_main.py
index c9ef546..0a1322a 100644
--- a/tests/test_main.py
+++ b/tests/test_main.py
@@ -14,6 +14,7 @@
 
 import os
 import sys
+from pathlib import Path
 from unittest import mock
 
 import contexter
@@ -444,6 +445,88 @@ def test_main_noxfile_options_sessions(monkeypatch):
         assert config.sessions == ["test"]
 
 
+@pytest.fixture
+def generate_noxfile_options_pythons(tmp_path):
+    """Generate noxfile.py with test and launch_rocket sessions.
+
+    The sessions are defined for both the default and alternate Python versions.
+    The ``default_session`` and ``default_python`` parameters determine what
+    goes into ``nox.options.sessions`` and ``nox.options.pythons``, respectively.
+    """
+
+    def generate_noxfile(default_session, default_python, alternate_python):
+        path = Path(RESOURCES) / "noxfile_options_pythons.py"
+        text = path.read_text()
+        text = text.format(
+            default_session=default_session,
+            default_python=default_python,
+            alternate_python=alternate_python,
+        )
+        path = tmp_path / "noxfile.py"
+        path.write_text(text)
+        return str(path)
+
+    return generate_noxfile
+
+
+python_current_version = "{}.{}".format(sys.version_info.major, sys.version_info.minor)
+python_next_version = "{}.{}".format(sys.version_info.major, sys.version_info.minor + 1)
+
+
+def test_main_noxfile_options_with_pythons_override(
+    capsys, monkeypatch, generate_noxfile_options_pythons
+):
+    noxfile = generate_noxfile_options_pythons(
+        default_session="test",
+        default_python=python_next_version,
+        alternate_python=python_current_version,
+    )
+
+    monkeypatch.setattr(
+        sys, "argv", ["nox", "--noxfile", noxfile, "--python", python_current_version]
+    )
+
+    with mock.patch("sys.exit") as sys_exit:
+        nox.__main__.main()
+        _, stderr = capsys.readouterr()
+        sys_exit.assert_called_once_with(0)
+
+    for python_version in [python_current_version, python_next_version]:
+        for session in ["test", "launch_rocket"]:
+            line = "Running session {}-{}".format(session, python_version)
+            if session == "test" and python_version == python_current_version:
+                assert line in stderr
+            else:
+                assert line not in stderr
+
+
+def test_main_noxfile_options_with_sessions_override(
+    capsys, monkeypatch, generate_noxfile_options_pythons
+):
+    noxfile = generate_noxfile_options_pythons(
+        default_session="test",
+        default_python=python_current_version,
+        alternate_python=python_next_version,
+    )
+
+    monkeypatch.setattr(
+        sys, "argv", ["nox", "--noxfile", noxfile, "--session", "launch_rocket"]
+    )
+
+    with mock.patch("sys.exit") as sys_exit:
+        nox.__main__.main()
+        _, stderr = capsys.readouterr()
+        sys_exit.assert_called_once_with(0)
+
+    for python_version in [python_current_version, python_next_version]:
+        for session in ["test", "launch_rocket"]:
+            line = "Running session {}-{}".format(session, python_version)
+            if session == "launch_rocket" and python_version == python_current_version:
+                assert line in stderr
+            else:
+                assert line not in stderr
+
+
 @pytest.mark.parametrize(("isatty_value", "expected"), [(True, True), (False, False)])
 def test_main_color_from_isatty(monkeypatch, isatty_value, expected):
     monkeypatch.setattr(sys, "argv", [sys.executable])