Repository: wntrblm/nox
Pull request #921 (2025-01-31 20:19:10)

fix: over validation of lists
Lists were being over-validated when we added validation (see #920), `nox.options.sessions = ("a", "b")` should be allowed. I don't want to allow `nox.options.sessions = "ab"`, though, since that's surprising and would only work for one-letter sessions. ~~I went with just validating both lists and tuples~~, though now that I think of it, maybe sets should be supported too. There's also the option of just validating for Sequence + not(str), but that can't be represented in static types (first commit), as far as I know. Edit: reverted to the original Sequence idea, `set` and `frozenset` (at least) should be supported too.

henryiii: @edgarrmondragon Does this look fine?
edgarrmondragon: > @edgarrmondragon Does this look fine?

Yup, this works!

Patch:
diff --git a/nox/_option_set.py b/nox/_option_set.py
index 23a9331..873a701 100644
--- a/nox/_option_set.py
+++ b/nox/_option_set.py
@@ -30,7 +30,7 @@ import attrs
 import attrs.validators as av
 
 if TYPE_CHECKING:
-    from collections.abc import Callable, Iterable
+    from collections.abc import Callable, Iterable, Sequence
 
 __all__ = [
     "ArgumentError",
@@ -50,7 +50,7 @@ av_opt_str = av.optional(av.instance_of(str))
 av_opt_list_str = av.optional(
     av.deep_iterable(
         member_validator=av.instance_of(str),
-        iterable_validator=attrs.validators.instance_of(list),
+        iterable_validator=av.not_(av.instance_of(str)),
     )
 )
 av_bool = av.instance_of(bool)
@@ -63,16 +63,16 @@ class NoxOptions:
     error_on_external_run: bool = attrs.field(validator=av_bool)
     error_on_missing_interpreters: bool = attrs.field(validator=av_bool)
     force_venv_backend: None | str = attrs.field(validator=av_opt_str)
-    keywords: None | list[str] = attrs.field(validator=av_opt_list_str)
-    pythons: None | list[str] = attrs.field(validator=av_opt_list_str)
+    keywords: None | Sequence[str] = attrs.field(validator=av_opt_list_str)
+    pythons: None | Sequence[str] = attrs.field(validator=av_opt_list_str)
     report: None | str = attrs.field(validator=av_opt_str)
     reuse_existing_virtualenvs: bool = attrs.field(validator=av_bool)
     reuse_venv: None | Literal["no", "yes", "never", "always"] = attrs.field(
         validator=av.optional(av.in_(["no", "yes", "never", "always"]))
     )
-    sessions: None | list[str] = attrs.field(validator=av_opt_list_str)
+    sessions: None | Sequence[str] = attrs.field(validator=av_opt_list_str)
     stop_on_first_error: bool = attrs.field(validator=av_bool)
-    tags: None | list[str] = attrs.field(validator=av_opt_list_str)
+    tags: None | Sequence[str] = attrs.field(validator=av_opt_list_str)
     verbose: bool = attrs.field(validator=av_bool)
 
 


Test patch:
diff --git a/tests/test__option_set.py b/tests/test__option_set.py
index 3eb5b15..3035d84 100644
--- a/tests/test__option_set.py
+++ b/tests/test__option_set.py
@@ -132,3 +132,25 @@ class TestOptionSet:
 
         expected_tags = {f"tag{n}" for n in range(1, 8)}
         assert expected_tags == set(actual_tags_from_file)
+
+    def test_validation_options(self) -> None:
+        options = _option_set.NoxOptions(
+            default_venv_backend=None,
+            envdir=None,
+            error_on_external_run=False,
+            error_on_missing_interpreters=False,
+            force_venv_backend=None,
+            keywords=None,
+            pythons=None,
+            report=None,
+            reuse_existing_virtualenvs=False,
+            reuse_venv=None,
+            sessions=None,
+            stop_on_first_error=False,
+            tags=None,
+            verbose=False,
+        )
+        options.sessions = ["testytest"]
+        options.sessions = ("testytest",)
+        with pytest.raises(ValueError):  # noqa: PT011
+            options.sessions = "testytest"