-
-
Notifications
You must be signed in to change notification settings - Fork 524
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate extras option in tox.ini #1113
Comments
To solve this we need to define what is obviously wrong: Standard defined at https://packaging.python.org/specifications/core-metadata/#provides-extra-multiple-use:
That being said I believe it's pip that should fail, as the one using it, not tox itself. |
Well, the current behaviour is that pip just prints a warning that there's no such extra in a dist and ignores it. That's be a breaking change.. |
I don't think we should behave differently from the tool that we're wrapping (in this case pip), so if the tool doesn't fail, tox shouldn't either. This is also for the sake of making reasoning about what should be happening easier, because I can always try out what happens, if I use the wrapped tool directly with what I am trying to configure in tox. |
I understand your position, but tox is a testing tool. It creates a wrong impression that everything went well. It is important that it'd alert the user that something is wrong exactly because it's especially wrong to be misleading during the testing process. |
In this concrete example where I mistype the name of the extras to be installed, I would expect the process to fail later anyway because needed packages are not installed and then I'll have to find out why. That's a pain and it would be nicer if it would have failed earlier, but this is something that should be decided in the packaging tool - in this case: pip. I had a look in the pip issues and can't find an open issues about this yet. If you open one (or find the one I didn't find), please let us know here. Also: I'd rather say it passes the wrong impression truthfully along :) On a more general note: The thought that tox abstracts the problems and weaknesses of the underlying tools away and even enhances on their actual behaviour is very tempting and a reoccuring theme in quite a few issues, but I am growing more opposed to this general idea as tempting as it might be for a number of reasons that I won't go into detail about, but the main reason is: all abstractions are leakyAll is nice if things work as they should, but if things do not work they need to be debugged on the level where the problem occurs. So in our case, if I want to install a set of extras and this does not happen, then I need to find out why pip (or any other packaging tool I might be using) doesn't install it. For this I need knowledge about how this tool behaves. In this case we would need to run tox with higher verbosity to see all output of the underlying tools and then we stumble over the warning emitted by pip. The burden of this can't be taken away by tox and the more diverse the packaging scene in Python will get over the next years the more of a step back tox needs to take there. This is why I would propose to close this (and similar) requests as wontfix. P.S. (off topic, but need to add my 2 cents :))
To me tox is a generic automation tool. Orchestrating tests is just one of the many things it can do. P.P.S If I mistype the name of a key (e.g. |
I myself would agree it's a pip gap, so let's implemented it there (if we can't we can add it here). As for #1121 that option I don't think it's doable either without breaking a lot of current code, so overall, not worth it. In general, if people miss-type configuration the runtime will usually fail. In virtualenvs case it did not fail only because pip was already provided in a different way, would it be another package would have failed the environment at runtime, reporting it does not have the missing dependencies. |
This is exactly what happened to me wrt #1097: because of a series of unrelated errors it has lead me into the wrong direction and caused a lot of misleading assumptions resulting in a lot of time being wasted.
Sometimes it tests have this thing "test x if y is available" and we don't know that something doesn't happen for a while and your whole "investigate this" flow stops there, it just never starts. You can say that one would notice that coverage reduced but that's not always the case. Especially when you inherit a project from someone else who missed it... P.S. Yes, tox is more than just testing tool. But in the context of this issue, I'd like to treat it like it. |
Ah, I understand. That's annoying indeed :( - this is why in general I am also a big proponent of the idea that a process should crash at the earliest possible time, but in our case here it is sadly not that easy to determine.
Would you have a suggestion how this could be done in a reasonably sane way? |
Following this https://packaging.python.org/specifications/core-metadata/#provides-extra-multiple-use it should be easy to classify absolutely invalid values. In the case of dist building enabled for env it should be quite easy to read the produced metadata which explicitly lists all of its extras. |
In the light of this, it might be a worthy workaround after all 🤔 |
But ... it wouldn't solve the problem when using a valid identifier that happens to be not a valid name, so it would only be a partial solution (and the complete solution could only be implemented inside pip in a feasible way). |
Long story short, tox ignores obviously invalid extra names. If could alert users about wrongly formatted items.
Background: pypa/virtualenv#1276
The text was updated successfully, but these errors were encountered: