Allow parametrization of any testenv

Issue #189 on hold
Daniel Neuhäuser created an issue

As discussed on IRC with @hpk42, I would like the ability to parametrize any testenv and not just the normal [testenv] one. One use case I have is a testenv for testing doctests on different Python interpreters, so basically I would like the following to work:

[tox]
envlist = docs-examples-{py27,py33,py34}

[testenv:docs-examples]
changedir = docs
deps = sphinx
commands = sphinx-build -W -b doctest -d {envtmpdir}/doctrees . {envtmpdir}/doctest

/cc @suor

Comments (27)

  1. Holger Krekel repo owner

    I wonder if we could literally allow this syntax to just work. The expression X in [testenv:X] could be parsed just like a "normal" factor conditional expression.

    In theory, some existing tox.ini files could break but i am not sure if it would be practically a problem. We might introduce it and be prepared for changing it to use special syntax like [testenv::factor-expression], i.e. a double :: to signal it.

  2. Sachi King

    I'm quite interested in getting this feature added to tox.

    I'm very interested in getting this feature added and have been doing some work on the required code for it. I'm fairly certain this can be added safely, no need for ::.

    That said the :: signal could be added quite easily, though it seems a bit ugly to me. tox -e :thing-constant It feels like it would be cleaner to add a config option 'section-factors' or something like that in the [tox] section that defaults to false as a fall-back.

    I will have a pull request for review shortly.

  3. Holger Krekel repo owner

    Also i feel some special syntax makes sense even like [factors:...] . Also we need to be clear about the ordering of how sections are applied. First the general [testenv] then any factors, then most specific envs? Also I'd still like to hear from https://bitbucket.org/suor/ on this.

  4. Alexander Schepanovski

    First, this looks like a good idea, a mentally consistent extension of what we already have.

    Second, if we are ok with slight backward incompatibility then we should go with single colon. People will omit/forget second one and just won't understand what is happening. :: is also inconsistent with current state of affairs, arbitrary and ugly.

    Third, we should decide on semantics first and then implement it. We should aim in a simpler mental model. Current model is:

    • use exactly matched section if exists, use [testenv] and factors overwise,
    • factors order doesn't matter unless for exact section match,
    • missing factors are silently skipped if they are anywhere in tox.ini or command line.

    Pull requests model is:

    • use exactly matched section if exists, overwise use [testenv] and factors if env factors are all either within it or built-in, overwise use partially matched section if non-matched factors are within it or [testenv] or are built-in, overwise use [testenv].
    • factors order matter in exact or partial section match, factors also should go first in env name in partial section match, factor order doesn't matter when using [testenv] unless there are missing factors.
    • missing factors are silently skipped if they are in [testenv] or command line, env names with missing factors are also silently accepted, but only if they exactly match something in envlist (order matter).
  5. Alexander Schepanovski

    The model I propose is:

    • use longest matching section first (by number of matching factors),
    • factor order never matters,
    • missing factors are silently skipped if they are anywhere in tox.ini or command line.

    Some clarifications.

    We can have priority collision for env py27-django16-mysql and testenvs [testenv:py27-django] and [testenv:django-mysql]. Which could be resolved by making this an error either statically or dynamically.

    Making factor order always arbitrary is somewhat backward incompatible, since these are now the same:

    [testenv:django16-mysql]
    ; ...
    
    [testenv:mysql-django16]
    ; ...
    
  6. Sachi King

    Second, if we are ok with slight backward incompatibility then we should go with single colon.

    I'm personally of the opinion this should be omitted, I am of the opinion that if this should default to off, we should add an option to the [tox] section that enables the behaviour. Something like sectionfactors = True to enable it. (default: False) In the current state I believe the chances for breakage is very very low.

    People will omit/forget second one and just won't understand what is happening.

    It would be very easy to miss, it's one character that changes how a single section is processed, I could see some major frustration in debugging why it is not working as expected.

    :: is also inconsistent with current state of affairs, arbitrary and ugly.

    It also makes me worry about future backward compatibility. Forever, a double colon signals doing something different, and additionally would require a fair bit of handling to remove/add the : for the user, which feels like it would be crufty and add code complexity. This can only be used once as well, next time a feature changes something suddenly it's :::, does that do just ::: or does that include ::, or is :::: mean do both. Is the next change ::::: pushing it out to :::::::: to do everything?


    Factor order still doesn't mater with this PR, but the section must be listed first. This functionality is completely new, so I think this is a reasonable assertion as it prevents undefined behaviour from occurring.

    We actually deal with three things.

    • The environment the user tells us to run
      • Current: A section name, or a list of factors
      • PR Method: A section name, a whole list of factors, or a section name followed by factors
    • the sections themselves
    • factors defined in the main [testenv] section.

    As such, the following are not factors, they are section names, and order very much matters.

    [testenv:django16-mysql]
    ; ...
    
    [testenv:mysql-django16]
    ; ...
    

    If we take the user input and start trying to fit section names wherever they fit and deciding the rest are factors we will end up with some nasty undefined behaviours as well as likely to cause more backwards incomparability.

    As a user I would not expect -e foobar-mysql-django16-thing1-thing2 to run mysql-django16, I would expect it to error with an unknown environment error as I was trying to run something like foobar with factors mysql,django16,thing1, and thing2.

    Additionally silently dropping factors that do not exist creates dangerous undefined behaviours. I would suggest exiting with an error when these are specified, as it creates undefined behaviour, however as this is currently supported this functionality is retained in this PR. It will do exactly the same thing, just running testenv with whatever it can glob together.

    If a tox.ini file specifies:

    [tox]
        envlist = py34,django16-mysql-facta
    [testenv]
    [testenv:django16-mysql]
        cmd =
            facta: django16-mysql-facta
            factb: django16-mysql-factb
            django16-mysql
    
    [testenv:mysql-django16]
    

    Here's what a user gets when running currently

    # tox
    Runs: testenv, testenv
    
    # tox -e django16
    Runs: testenv
    
    # tox -e django16-mysql
    Runs: django16-mysql
    
    # tox -e mysql-django16
    Runs: mysql-django16
    
    # tox -e mysql-django16-facta
    Runs: testenv
    
    # tox -e django16-mysql-facta
    Runs: testenv
    
    # tox -e django16-mysql-factb
    Returns: ERROR: unknown environment 'mysql-django16-factb'
    
    # tox -e django16-mysql-factb-facta
    Returns: ERROR: unknown environment 'mysql-django16-factb-facta'
    

    With the change as currently implemented the output would be.

    # tox
    Runs: testenv, django16-mysql-facta
    
    # tox -e django16
    Runs: testenv
    
    # tox -e django16-mysql
    Runs: django16-mysql
    
    # tox -e mysql-django16
    Runs: mysql-django16
    
    # tox -e mysql-django16-facta
    Runs: testenv
    
    # tox -e django16-mysql-facta
    Runs: django16-mysql with the facta command
    
    # tox -e django16-mysql-factb
    Runs: django16-mysql with the factb command
    
    # tox -e django16-mysql-factb-facta
    Runs: django16-mysql with the facta and factb command
    

    Additionally

    [testenv:django16-py27]
    [testenv:django16]
    

    Both current and new PR implementation would run the same.

    # tox -e django16
    runs: [testenv:django16]
    
    # tox -e django16-py27
    runs: [testenv:django16-py27]
    
    • missing factors are silently skipped if they are anywhere in tox.ini or command line.

    On the tox command line they actually throw an environment not found error currently, unless the individual words are somewhere in the envlist.

    Throwing an error seems like the correct behaviour IMHO, however this undefined execution is preserved with the pull-request, and as such will show no behavioural difference to how this currently functions. It will continue to just invoke testenv with whatever factors it can find.

    IMHO, this should be switched to throw an error, because as I see it, it's a usage error, and leads to undefined tests being run. If the user typos a section name, instead of getting an error, it just runs the base test again. Worst case, it passes but the test it should have run broke. Best case, it errors and shouldn't and someone notices the typo.

    For example:

    [tox]
    envlist = foo-py27, bar-py34,baz
    
    [testenv]
    command = thing
    
    [testenv:foo-py27]
    command = foo-py27
    
    [testenv:bar-py34]
    command = bar-py34
    
    # tox
    runs sections: foo-py27, bar-34,testenv
    
    # tox -e foo-py27
    runs section: foo-py27
    
    # tox -e foo-bar
    runs section: testenv
    
    # tox -e baz-thing
    ERROR: unknown environment 'baz-thing'
    

    In the current PR:

    # tox
    runs sections: foo-py27, bar-34,testenv
    
    # tox -e foo-py27
    runs section: foo-py27
    
    # tox -e foo-bar
    runs section: testenv
    
    # tox -e baz-thing
    ERROR: unknown environment 'baz-thing'
    

    We can have priority collision for env py27-django16-mysql and testenvs [testenv:py27-django] and [testenv:django-mysql].

    As with the above, this actually will either error out or run the default testenv, this logic is unchanged with this pull request.

    Current:

    1. Chunk [tox]envlist values up, and add them to a set, add this set to known factors
    2. Check if py27-django16-mysql is a section name. If so make_envconfig.
    3. Split into py27, django16, and mysql, check if all three are in the factor list. If so make_envconfig.
    4. Done

    PR:

    1. Check if py27-django16-mysql is a section name. If so make_envconfig.
    2. Split into py27, django16, and mysql, check if all three are in the factor list. If so make_envconfig. * This is the main difference, we've added factors that exist in test_env, but we don't add the catch-all envlist ones yet.
    3. Now we check 2 times, once for py27-django16 section with factor mysql either in [testenv] or [testenv:py27-django16] and again for py27 section and mysql and django16 factors. If we get a match, we run make_envconfig and stop searching.
    4. Now we add the catch-all factors that come from envlist, test again, and if that matches we make_envconfig.
    5. Done

    However, all that said, I noticed at 4, I'm only taking the catch-all factors from envlist and checking them, not envlist + known_factors. So I'm updating that, because that's wrong and breaks the existing logic.


    I think the first thing to decide on is how to handle section names.

    1. The user must start the environment of section factors with the section name followed by factors
    2. The user can declare the section name anywhere, everything else is considered a factor.
    3. Something else?

    I do believe 1 to be the best choice as it reduces the amount of guessing and makes the matches more defined, so a user can know what to expect.

  7. Alexander Schepanovski

    Making factor order not matter everywhere will end envname as separate concept. This is a new thinking and requires some effort to get used to. But currently we have both in. Your way is to mix envname and factor concepts with some not so easy to explain rules. My way is to make envname a set of factors.

    To illustrate this think of what approach will result in less docs. And less code after all.

    On missing factors. It was also my concern when we introduced current behavior, it was finally selected by @hpk42 on usability standpoints. But if we think in factors, not envnames current behavior is just making an error using factor not in tox.ini, which is most obvious thing. So I was wrong and Holger was right.

    I can see confusion in transition from envnames as strings to envnames as sets of factors. But, first, this is temporary, and, second, we already started this in 1.8 anyway.

  8. Ronny Pfannschmidt

    how about having a explicit declaration of used factors and using the order of that to warn if the order is incorrect it could also be used to show typs when passing errors from cli

    [tox]
    valid_factors = py{27,33,34,35} django1{6,7,8} mysql{4,5,6} postgresql{93,94,95}
    
  9. Alexander Schepanovski

    What's the value? So we have py27-django18 in envlist, but user types django18-py27 on command line and gets an error. Looks more confusing than helpful. And we already have protection from typos in a form of factors should be mentioned in tox.ini somewhere. Adding order for some places (command line) and having no order in others (factor conds) brings nothing but confusion.

    valid_factors could help check if there was a typo somewhere in factor conds, e.g. dango18: ..., which will now be ignored silently since we expect some factors to be available on command line, but not included in envlist. Originally we discussed something similar called extra_factors. It shouldn't include anything already present in envlist though. Now this addition will bring problems for some users as they already use factors only defined in sections. Will we use it or not, I still don't see any value in forcing order.

    Also valid_factors introduce order upon factors globally, which sometimes doesn't have any sense - in situations when we have several parametrized tasks: test, docs, lint. And this is the exact situation we need several parametrized testenvs, the situation we are discussing here.

    The only value I can see is managing some illusion of mental model continuity between versions. My approach is mental model shift, but it's a shift to simplicity. New users will get new model faster than current one and this is a sign it's a step in right direction.

  10. Sachi King

    I'm looking at updating the PR, but I don't understand the expected model anywhere close to enough to start implementing it. Long story short it's "process section names as factors and then figure out what ones to glob" right?
    What I don't understand here is how the resolution logic is going to work.
    Thinking about globing sections from factors I am extremely concerned about both how big a breaking change this will be and creating unexpected behaviour when in use.

    So what becomes the condition of matching the section?
    What happens if we match two, three, or more sections?

    [tox]
    [testenv]
    install_command =
        force: pip install --U --force-reinstall {opts} {packages}
        pip install {opts} {packages}
    commands = 
        testr
    
    [testenv:django16-mysql]
    deps = pymysql
    commands = pymytest
    
    [testenv:django16-pgsql]
    deps =
        factor77: pydep99
        pypgsql
    commands = pypgtest
    
    [testenv:venv]
    commands = {posargs}
    

    What is the expected behaviour when the following are called?

    • django16-mysql-pgsql
    • django16-mysql-venv
    • django16-mysql-pgsql-venv
    • django16-mysql-py27
    • django16-mysql-force
    • django16-venv
    • django16
    • venv-force
    • py27-force
    • force

    Now this addition will bring problems for some users as they already use factors only defined in sections.
    

    AFAICT this is not supported as this is the exact functionality I'm trying to use.
    I'm attempting to call venv-force, it runs the expected install_command, but runs the testr command, not {posargs}

    [tox]
    [testenv]
    install_command =
        force: pip install --U --force-reinstall {opts} {packages}
        pip install {opts} {packages}
    commands =  testr
    
    [testenv:venv]
    commands =
        {posargs}
    

    call tox -e venv-force and inspect vc I can see install_command is pip install --U --force-reinstall {opts} {packages}, however commands is testr.

    Also if I try:

    [tox]
    envlist = venv-facttest
    
    [testenv]
    install_command =
        force: pip install --U --force-reinstall {opts} {packages}
        pip install {opts} {packages}
    commands =  testr
    
    [testenv:venv]
    commands =
        facttest: python -c 'print("hi")'
        {posargs}
    

    and call tox -e venv-facttest and again inspect vc I see the default install_command, and commands is testr.

  11. Alexander Schepanovski

    On section resolution ambiguity. Higher I proposed "use longest matching section first (by number of matching factors)" and clarified that this will leave space to ambiguity: django16-mysql-pgsql-venv in your example could resolve to both [testenv:django16-mysql] and [testenv:django16-pgsql]. This should be an error cause there is no sensible way to resolve that.

    django16-mysql-venv under that model is resolved to [testenv:django16-mysql]. [testenv:venv] is ignored. I see how this could be a confusion, so we can make this an error too.

    I think I see now the source of your questions: you are trying to marry several sections for single envname. This doesn't work now. The only exception is full match and base testenv, which are merged with full match taking priority. The issue is this cannot be generalized for envnames-as-sets model since sets are not linearly ordered. However, testenvs could be ordered linearly for given envname by length of matching starting substring:

    • testenv:django16-mysql-pgsql-venv
    • testenv:django16-mysql-pgsql
    • testenv:django16-mysql
    • testenv:django16
    • testenv

    We can then select the first longest and inherit from shorter ones. This is a generalization of current inheritance model. The issue is such testenvs are not composable - you can't have sections like:

    [testenv:django16-mysql]
    ; ...
    
    [testenv:venv]
    ; ...
    

    and expect both be used for django16-mysql-venv, there is no natural priority here, we will also need to invent that.

    Non-composability of testenvs is what led us to factors originally. Factors are composable by uniting them into sets. Effects of a envname-as-a-set is a union of effects of its factors. Condition holds for envname if it is a subset of its factors. My model is adding testenv names factors to all conditions within it, it builds itself on set mental model.

    This ticket authors use case was having several unrelated but parametrized tasks, and set model will serve it naturally:

    [testenv]
    deps = 
        django16: Django>=1.6<1.7
        ; ...
    command = py.test
    
    [testenv:docs]
    ; ...
    command = sphinx-build ...
    

    P.S. @nakatosa, please post or link your config here so we can see how it fits all the models.

  12. Holger Krekel repo owner

    Not sure i can follow all details from all posts yet but starting from the original issue poster @DasIch i would think that with a new [factors:X-Y-...] section we could make things work nicely and without backward-compat troubles or much changes in the mental model. It would not change anything regarding currently valid tox.ini files. We would still need to think about ordering and i agree to @suor that the longest matching factor-section should have highest priority. So we'd have this order:

    • [testenv:EXACT-ENV-NAME] only matches when the env EXACT-ENV-NAME is used.
    • [factor:...] sections match if they the listed factors are a subset of the envname under test
    • [testenv] as a fallback if nothing else matched

    With this [testenv] and [factor] would be synonyms somewhat but to disambiguate we could disallow a plain [factor] section.

  13. Alexander Schepanovski

    @hpk42, I don't see a point in string matching in the long run. What's the point disambiguating py27-django18 and django18-py27? And if this is only for backward compatibility then there are other ways that will lead to less confusion and clearer API in the end:

    1. Up the version to 3.0 and refuse backward compatability. Rewriting single file config is not that bad of an issue anyway. Or if that's too much ...
    2. Use [factor] and [factor:...] with new semantics, leave [testenv*] work as they are now. Make an error to mix both ways (optionally). Eventually deprecate and remove [testenv*] support (optionally).

    And anyway docs should be significantly updated to start from factors and then explain testenvs/factor sections as their combinations, not the other way around as it is now.

  14. Holger Krekel repo owner

    @suor the point is not so much disambiguating django18-py27 and py27-django18 but rather disambiguating testenv:py27-X and testenv:py27-X-Y. Today only the latter matches when testenv:py27-X-Y is run whereas if we extend testenv syntax both will match which can be surprising. IIRC i have such environment naming in some of my own projects and therefore assume others will have it as well.

    I'll otherwise ponder your suggestion some more -- if we want to converge towards a single syntax we could think about using [env] and [env:...] instead of [factor].

  15. Alexander Schepanovski

    @nakatosa both my and your approach will make docs-constraints work for you, so no difference here. I also think you should use less testenvs and just move them to base [testenv] as factors.

  16. Alexander Schepanovski

    [env*] is short and nice. Still do you want to make it as replacement with new semantics or keep both [env*] and [testenv*] at least for a while? If the latter then I think restricting mixing them is a good idea.

  17. Holger Krekel repo owner

    yes, I guess it makes sense to restrict mixing them.

    Am still thinking a bit what the best approach is. To counteract my own example ("py27-X" versus "py27-X-Y") we could also think about requiring a star as in [testenv:py27-X-*] to make it match any environment that has "py27" and "X" as factors. This way we could introduce it directly to the current testenv syntax. I am open to variations of this syntax idea. With this we wouldn't have big changes in docs or deprecations but a rather smooth transition where those that don't need it can stay ignorant, use their good old tox.ini files.

  18. Alexander Schepanovski

    It's a bit counterintuitive that [testenv:py27-X-*] matches py27-X but not only py27-X-Y alike.

  19. Holger Krekel repo owner

    yes, agreed. Therefore i am open to other syntactic ideas. There is some value in keeping current tox.ini's with their testenv sections valid (with the meaning that it only matches environments which use exactly the stated factors -- i don't think we need to mind the X-Y versus Y-X ambiguity).

  20. Tai Lee

    I like the wildcard suggestion. I want testenv to run python runtests.py by default, I want testenv:flake8 to run flake8 foo for tox -e flake8, and I want factors:django*-cov to run coverage run ... runtests.py for tox -e django17-cov and django18-cov, etc.

    I think adding factors: sections is a bit clearer and less confusing. testenv:name should only match if ALL the factors in the given env name are present in the section name (no matter the order). factors:name should only match if all the factors in the section name are present in the given env name.

    It's basically the inverse, with factor level wildcard support for both testenv: and factors: sections.

    Priority should be:

    1. All factors in given env name are in testenv:name, regardless of order. There can only ever be one match.
    2. All factors given in factors:foo-bar are in env name, regardless of order. There can be multiple matches.
    3. testenv default.

    If there are multiple matching factors: sections, they should be prioritised by priority = N in the factors sections.

    If N is the same (or not defined) in two matching factors: we can further prioritise supersets over subsets (factors:django17-py27 over factors:django17), exact over wildcard matches (factors:django17 over django*), or else it is an error (factors:foo and factors:bar for tox -e foo-bar, when neither define priority = N or N is the same).

  21. Pi Delport

    This feature would also provide a nice solution for at least some of the use cases of #292 (negated factor conditions).

  22. Matthew Schinckel

    I'd like something similar to this: I have base tests (that test a matrix of python versions and django versions), but then I have some example projects, that really should be run against the same matrix.

    Currently, I've used py{...}-django{...}-{base,example1}, but then need to use base: within the [testenv] stanzas: it would be far easier to reason about what is happening in the tests with this concept.

  23. Log in to comment