tox-gh-actions icon indicating copy to clipboard operation
tox-gh-actions copied to clipboard

fail when envlist is empty

Open fschulze opened this issue 4 years ago • 8 comments

It took me a while to realize I had the same issue as #44.

I think the run should fail if the resulting envlist is empty, that way one doesn't accidentally think the action succeeded when nothing was actually done.

fschulze avatar May 17 '21 09:05 fschulze

Agreed, it was surprising than tox running no testenv resulted in success.

merwok avatar Nov 02 '21 22:11 merwok

This is especially important since it is not very well documented that, in order to be eligible for selection by this plugin, the env must also be present in envlist. This may make sense to those who understand how Tox plugins work, but is not intuitive to mere users. A failure on an empty generated envlist would help users detect and diagnose this.

posita avatar Jul 18 '22 22:07 posita

Hello everyone, thanks for the feedback. At this point, I'd like to avoid actually failing the tox when it couldn't find no environments as it can be technically a breaking change for exiting users.

At the same time, I understand this issue can be a bit difficult to catch for users. I'd like to propose showing a warning message when this problem happens as a mitigation: https://github.com/ymyzk/tox-gh-actions/pull/123

This is especially important since it is not very well documented that, in order to be eligible for selection by this plugin, the env must also be present in envlist.

As the first example in README only specifies envlist and gh-actions, I was assuming it's clear enough for users but if you have ideas to improve the README further, feel free to open a PR.

[tox]
envlist = py37, py38, py39, py310, mypy

[gh-actions]
python =
    3.7: py37
    3.8: py38
    3.9: py39
    3.10: py310, mypy

[testenv]
...

ymyzk avatar Jul 26 '22 14:07 ymyzk

What about an option --fail-on-no-environment? That wouldn't be a breaking change.

fschulze avatar Jul 26 '22 15:07 fschulze

Just having an example of how to do it right does not inform users about nuanced behaviors when they deviate from the example.¹ Even this would be better:

[tox]
# gh-actions only has access to environments explicitly listed in envlist
envlist = py38, py39, py310, mypy

[gh-actions]
python =
    # note that py37 and py38-legacy will silently never be run because they are missing from envlist
    3.7: py37
    3.8: py38, py38-legacy
    3.9: py39
    3.10: py310, mypy

[testenv]
...

I like the --fail-on-no-environment option, but I like the --fail-on-missing-environment better, which would error if there was an environment listed in the [gh-actions] section that was unreachable or not also present in envlist. Either way, there should likely be warnings for any unreachable environments listed in [gh-actions].


¹ Assuming otherwise is typical by people already familiar with the behaviors in question, but often flawed. Those people are not the audience of documentation. The mere existence of this issue is evidence that your assumptions deserve revisiting. Further, if you can solve your user's difficulty with poor ergonomics by improving ergonomics, that's almost always a better solution than relying on secondary sources like documentation.

posita avatar Jul 26 '22 15:07 posita

Thanks for sharing additional feedback. Adding an option to fail should be possible but let me think about it a bit deeper before actually implementing it as tox-gh-actions hasn't provided any CLI options so far.

ymyzk avatar Jul 27 '22 15:07 ymyzk

If it is easier, then maybe some environment variable could be used?

Czaki avatar Dec 13 '22 09:12 Czaki

Sorry for a slow response but I just published tox-gh-actions v2.12.0 which adds an option to fail when no environments are matched. Please check details in README and give it a try: https://github.com/ymyzk/tox-gh-actions/blob/v2.12.0/README.md#fail-when-no-environments-are-matched

Thank you all for your patience!

ymyzk avatar Dec 13 '22 14:12 ymyzk