Provide control over pex warning behavior. #680
Conversation
There was a problem hiding this comment.
I see why @mateor credits you with teaching him how to write tests. This all looks very solid and I appreciate the extensive testing.
| @@ -327,6 +327,17 @@ def PEX_IGNORE_RCFILES(self): | |||
| """ | |||
| return self._get_bool('PEX_IGNORE_RCFILES', default=False) | |||
|
|
|||
| @property | |||
| def PEX_EMIT_WARNINGS(self): | |||
There was a problem hiding this comment.
This will update https://pex.readthedocs.io/en/stable/api/index.html#pex-variables-module on the next release, right?
tests/test_pex_warnings.py
Outdated
| skip_py2_warnings = pytest.mark.skipif(PY2, | ||
| reason="The warnings.catch_warnings mechanism doesn't work " | ||
| "properly under python 2.7 / pypy2, etc. across " | ||
| "multiple tests") |
There was a problem hiding this comment.
It'd be helpful to specify why. Sounds like you investigated this and it has to do with C code. Any knowledge transfer would be helpful in case someone spends more time trying to figure this out.
There was a problem hiding this comment.
I have to disagree here. This will never be fixed in python 2.7 since it's eol and pypy appears to be trying to emulate CPython as much as possible - which makes sense. As such the detailed why is unimportant.
| reason="The warnings.catch_warnings mechanism doesn't work " | ||
| "properly under python 2.7 / pypy2, etc. across " | ||
| "multiple tests") | ||
|
|
There was a problem hiding this comment.
And helpful to reiterate this is not an issue with our code and things work IRL, only this is an issue with the testing mechanism, as you explained to me over Slack.
There was a problem hiding this comment.
I mean, I already say this.
If you have some exact wording changes you want me to toss in, I'll do it.
There was a problem hiding this comment.
Maybe something like:
"The warnings.catch_warnings mechanism doesn't work "
"properly under python 2.7 / pypy2, etc. across "
"multiple tests. This is an issue with the testing mechanism due to"
"issues with the underlying C code, and it does not negatively impact"
"our warning code when used outside of these tests."
Make it clear no one should waste time on trying to fix this.
There was a problem hiding this comment.
OK - left out the C reference since that doesn't explain pypy and I still don't think the underlying reason is important. I did add language though that makes it clear this is only a unit-test failure that can't be fixed and is otherwise covered by ITs. I will point out we should never have to say - but this actually works - in comments. If we're allowing code to hit master that doesn't work - we have big problems. It should be assumed!
Previously warnings were emitted to stderr unconditionally, but now both build-time and run-time controls are provided. At build-time --no-emit-warnings can be specified and a pex will be produced that does not emit warnings. At run-time, PEX_EMIT_WARNINGS and PEX_VERBOSE can be used to toggle warning behavior. Fixes pex-tool#677
This was eliminated in pex-tool#680 since it was (locally) unused but it turns out it was used over in lambdex. The long term fix allowing safe removal of the API is tracked by: pantsbuild/lambdex#5. Fixes pex-tool#684
This was eliminated in pex-tool#680 since it was (locally) unused but it turns out it was used over in lambdex. The long term fix allowing safe removal of the API is tracked by: pantsbuild/lambdex#5. Fixes pex-tool#684
This was eliminated in #680 since it was (locally) unused but it turns out it was used over in lambdex. The long term fix allowing safe removal of the API is tracked by: pantsbuild/lambdex#5. Fixes #684
Previously warnings were emitted to stderr unconditionally, but now
both build-time and run-time controls are provided. At build-time
--no-emit-warnings can be specified and a pex will be produced that
does not emit warnings. At run-time, PEX_EMIT_WARNINGS and PEX_VERBOSE
can be used to toggle warning behavior.
Fixes #677