Partial match on testenv results in use of base testenv

stephenfin commented 8 months ago

Issue

Consider the following toy tox.ini:

[testenv]
skip_install = true
allowlist_externals =
  echo
commands =
  echo "base testenv"

[testenv:functional{-py310}]
commands =
  echo "functional testenv"

This should result in a single testenv functional-py310 (there's only one factor) plus the default pyNN testenvs. Attempts to use a testenv with the functional factor but a different pyNN factor (e.g. tox -e functional-py312) or without a pyNN factor (e.g. tox -e functional) should fail since it doesn't match an env making it no different to e.g. tox -e doesnotexist.

In tox 3, this was the case:

❯ virtualenv .venv --python=python3.8
❯ source .venv/bin/activate
❯ pip install 'tox<3'

❯ tox -e functional-py310 -q
functional testenv
__________________________________________________________________ summary __________________________________________________________________
  functional-py310: commands succeeded
  congratulations :)

❯ tox -e functional-py312 -q
ERROR: unknown environment 'functional-py312

❯ tox -e functional -q
ERROR: unknown environment 'functional'

❯ tox -e doesnotexist -q
ERROR: unknown environment 'doesnotexist'

Once we switch to tox 4, this is no longer the case:

❯ tox -e functional-py310 -q
functional testenv
  functional-py310: OK (0.22 seconds)
  congratulations :) (0.29 seconds)

❯ tox -e functional-py312 -q
base testenv
  functional-py312: OK (0.13 seconds)
  congratulations :) (0.18 seconds)

❯ tox -e functional -q
base testenv
  functional: OK (0.14 seconds)
  congratulations :) (0.20 seconds)

Totally undefined envs fail as expected (though with an uglier error than previously):

❯ tox -e doesnotexist -q
ROOT: HandledError| provided environments not found in configuration file:
doesnotexist

This can result in us running a wholly different set of tests/commands if e.g. a user uses a pyNN factor that is not defined inline or does not use a factor (with the idea being you'd use the Python version tox is installed under).

Environment

Using latest tox 3 and 4 available from pip (currently 3.28.0 and 4.12.1, respectively).

Minimal example

See above.

stephenfin commented 8 months ago

I'd like to know what is the expected behaviours in the below:

[testenv]
skip_install = true
allowlist_externals =
  echo
commands =
  foo: echo "foo"
  bar: echo "bar"
  baz: echo "baz"
  fizz-buzz: echo "fizz-buzz"

[testenv:foo-bar]
commands =
  echo "foo-bar"

[testenv:fizz-buzz]
commands = 
  echo "basic sparkle"

[testenv:fizz-buzz-bang]
commands = 
  echo "extra sparkle"

tox -e foo
tox -e bar
tox -e foo-bar
tox -e bar-foo
tox -e foo-bar-baz
tox -e fizz
tox -e fizz-buzz
tox -e fizz-buzz-bang
tox -e fizz-bang-buzz

gaborbernat commented 8 months ago

tox -e foo-bar
foo-bar

tox -e foo
foo

tox -e bar
bar

tox -e bar-foo
# undefined, but probably bar

tox -e fizz
# empty output, matches nothing

tox -e fizz-buzz
basic sparkle

tox -e fizz-buzz-bang
extra sparkle

tox -e fizz-bang-buzz
# undefined, I'd expect empty output, matches nothing, but can see a case for fizz-buz

stephenfin commented 8 months ago

Okay, that broadly aligns with what I'd expect. So I think the issue here is that we're equating a factor to be both a thing defined inside e.g. [testenv] commands or as any piece of an testenv, when in fact factors should really be only the former plus the dynamic pyNM stuff. In the above tox.ini, I think we have the following static testenvs:

foo-bar
fizz-buzz
fizz-buzz-bang

We then have the following factors:

foo
bar
baz
fizz-buzz (question: should this be illegal? In my experience, factors must usually consist of alphnumeric characters and periods)
the dynamic Python factors like pyN, pyNM, pyN.M, pyN.M-alpha etc.

This results in a large matrix of dynamic testenvs:

foo
foo-bar
baz-foo-bar
py310-foo
py3.10-rc1-foo-bar-baz
...

So the crucial bit here that we probably don't consider bang to be a factor since it's only defined as part of a static testenv (the fizz-buzz-bang testenv). We also don't consider fizz or buzz to be factors since fizz-buzz is the factor. Correct? If so, we (read: I, in a future patch, time permitting) probably want to start disambiguating between factors and testenvs, allowing the latter to be used for generating dynamic testenvs but not the former (either in whole or in part by splitting on -)?

stephenfin commented 8 months ago

So the crucial bit here that we probably don't consider bang to be a factor since it's only defined as part of a static testenv (the fizz-buzz-bang testenv). We also don't consider fizz or buzz to be factors since fizz-buzz is the factor. Correct? If so, we (read: I, in a future patch, time permitting) probably want to start disambiguating between factors and testenvs, allowing the latter to be used for generating dynamic testenvs but not the former (either in whole or in part by splitting on -)?

To be clear, this is only for determining whether a provided testenv in valid or not. Obviously a user could do the following:

[testenv:fizz-buzz{,-bang}]
commands = 
  !bang: echo "basic sparkle"
  bang: echo "extra sparkle"

So bang is a factor in that context. It's just not a factor that can be considered for generating dynamic testenvs. A user still shouldn't be able to run tox -e bang: they could only run tox -e fizz-buzz-bang (or something else entirely).

tox-dev / tox