-
-
Notifications
You must be signed in to change notification settings - Fork 539
azure pipeline restructure and migration to codeclimate #1025
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
Conversation
Looks good. Something still missing? |
Failing Windows tests on Python 2.7 (seems someone somewhere sets a unicode PYTHONPATH environ, and that what breaks the world; trying to identify it) and coverall notifications. |
src/tox/_pytestplugin.py
Outdated
@@ -404,7 +404,7 @@ def mock_venv(monkeypatch): | |||
and cannot install any packages. """ | |||
|
|||
# first ensure we have a clean python path | |||
monkeypatch.delenv("PYTHONPATH", raising=False) | |||
monkeypatch.delenv(str("PYTHONPATH"), raising=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicoddemus @RonnyPfannschmidt @asottile slight bug here inside pytest monkeypatch. The key passed in here is used to re-insert the value at teardown, instead of the key that was there. Because during the get
unicode keys degrade to bytes keys this can mean that by invoking this operation we transform bytes key to unicode inside os.environ
when the key passed into delenv is unicode.
In such cases, python 2 Windows will refuse to run subprocesses leaving the user to wonder who and where and how it set non bytes value into os.environ
. It makes it even harder to discover because the effect is usually discovered later on by another test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the ping: pytest-dev/pytest#4056
@obestwalter seems merge-ing the coverages by different jobs is not yet supported out of box on non-Travis CI... |
oh ... maybe @zooba knows more when this will hit production? Shall we just leave this hanging around for now then? |
Hit production? Seems not supported yet, and no plans to add support. |
Turns out coveralls does not support merge reports from Azure DevOps. Will need to do somehow a manual merge and upload that. |
Fixes the bug described in: tox-dev/tox#1025 (comment) Which is more evident when using `unicode_literals`.
Just because they are not talking about it yet, does not mean it could be supported at some point not too far in the future. But o.k. let's try coveralls, ignore my approval in the other PR then :) |
Fixes the bug described in: tox-dev/tox#1025 (comment) Which is more evident when using `unicode_literals`.
Fixes the bug described in: tox-dev/tox#1025 (comment) Which is more evident when using `unicode_literals`.
Fixes the bug described in: tox-dev/tox#1025 (comment) Which is more evident when using `unicode_literals`.
…hon 2 Fixes the bug described in: tox-dev/tox#1025 (comment) Which is more evident when using `unicode_literals`.
Eh, feels like we got hit by:
https://github.com/Linuxbrew/brew/issues/746#issuecomment-422955300 |
Just curious, why the switch from codecov to coveralls? |
just saw your comment #1028 (comment)
Pity |
@anthrotype furthermore codecov turned out to be really flaky for us in the last few months (many unexplainable results) and we got no support when reaching out to them. ``coveralls` also does not implicitly supports it but at least their API is somehow easier to integrate at some level. |
Closing in favour of #1031; I've kept this option only to validate that PR-ing from a fork still works correctly (although no global coverage report). Once Azure Pipelines offers artifact uploads for forks (ETA around 3 weeks), we'll enable coverage generation on all branches. Note in case Azure Pipelines would not deliver this we can migrate to storing the coverage artifacts on Amazon S3 (for a very small cost probably). |
👍 Resolves #1024.