Skip to content

Fix factor selection via TOX_FACTORS environment variable#3725

Merged
gaborbernat merged 2 commits intotox-dev:mainfrom
worksbyfriday:fix-factor-env-var
Feb 17, 2026
Merged

Fix factor selection via TOX_FACTORS environment variable#3725
gaborbernat merged 2 commits intotox-dev:mainfrom
worksbyfriday:fix-factor-env-var

Conversation

@worksbyfriday
Copy link
Contributor

Summary

  • Fix TOX_FACTORS env var producing wrong results (each character treated as a separate factor instead of each factor string)
  • Root cause: get_type() inferred list[str] for _AppendAction with nargs="+", but the actual runtime type is list[list[str]]
  • The existing Convert infrastructure already handles nested generics, so fixing the type inference is sufficient

Closes #3557

Test plan

  • Added parametrized test test_factor_select_via_env_var covering single factor, AND (comma-separated), and OR (semicolon-separated)
  • All 65 env_select tests pass
  • All 6355 config tests pass
  • All 6 CLI env var tests pass

🤖 Generated with Claude Code

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the PR draft until the CI passes.

@worksbyfriday
Copy link
Contributor Author

Rebased on main to pick up latest changes. Will mark ready for review once CI passes.

@worksbyfriday worksbyfriday marked this pull request as ready for review February 17, 2026 16:47
@worksbyfriday
Copy link
Contributor Author

Rebased on main, all tests pass. The only failures are type/type-min which also fail on main. Ready for review.

@worksbyfriday worksbyfriday marked this pull request as draft February 17, 2026 16:50
@worksbyfriday
Copy link
Contributor Author

Understood — converted back to draft. All test jobs pass (3.10-3.14 on Ubuntu/macOS/Windows), but the type/type-min jobs are failing. Those same jobs also fail on main — is there something I need to do to fix them, or are they a known issue on the main branch?

@worksbyfriday
Copy link
Contributor Author

Note: the type/type-min CI failures are caused by argcomplete (optional dependency) not being installed in the ty check environment. I've submitted #3729 to fix this — once merged, rebasing this PR should make all CI pass.

worksbyfriday and others added 2 commits February 17, 2026 17:28
The argparse action type for `-f`/`--factors` is `_AppendAction` with
`nargs="+"`, which produces `list[list[str]]` at runtime. But `get_type()`
only inferred `list[str]`, so `get_env_var()` produced a flat list of strings.
When `_parse_factors()` iterated over this, it treated each character as a
separate factor instead of each string.

Fix `get_type()` to detect `nargs` on `_AppendAction` and return the correct
nested type `list[list[str]]`. The existing `Convert` infrastructure already
handles nested generics — semicolons separate OR groups, commas separate AND
factors within a group.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@worksbyfriday worksbyfriday marked this pull request as ready for review February 17, 2026 17:33
@worksbyfriday
Copy link
Contributor Author

Rebased on main (includes #3728). All CI checks passing — type/type-min now green. Marking ready for review.

@gaborbernat gaborbernat merged commit e7a0800 into tox-dev:main Feb 17, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Factor parsing is broken when using env vars

2 participants