-
Notifications
You must be signed in to change notification settings - Fork 45
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
Refactor CI configuration to use tox and Azure Pipelines for everything #229
Conversation
It's likely a problem with the installation of sqlite that is now used by |
Weird. Maybe coverage is upgraded later? In anycase, when pinning a version number like this one should always add a comment to explain why or pointing to a github issue so as to know when it's safe to remove the pinning. |
Indeed the codecov step upgrades coverage to version 5 which triggers the problem at the end. |
The Windows CI config should now be fixed. We still need to update the coverage token though:
@tomMoral could you give @ogrisel or me the rights to update this token? |
Adding for the record that this PR fixes a coverage issue that was probably present since |
What right do you need? azure-pipeline rights or codecov rights? |
Nice, the bash commands executed on Windows Azure CI are actually the true Windows commands and not ubuntu from the Windows Subsystem for Linux (WSL). This means that we can use bash everywhere on Azure CI, both for Windows, Linux and macOS. |
This is great news! This will simplify the CI maintenance. |
We shall do the same for other projects: joblib, cloudpickle, ... |
The codecov status has disappeared for the last builds on github commit status markers:
|
By adding Python 3.8 test I have found a real bug in loky:
@tomMoral any idea? It seems to be cause by a change in Python because this does not happen in py37. |
For codecov we need to:
|
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.
LGTM.
79.54% remains the same compared to ceaef5e ... C'est pas bon en fait. Il doit nous manquer des rapports codecov. |
Let me check. A few commits ago we were at 90%, a regression was introduced in the last ~10 commits or so. |
Coverage is back up. I fixed the issue by telling coverage where to find the coverage xml files. The reason the coverage decreased today is very obscure: for some reason, It seems to be a recurrent issue in many CI providers. (see a related github-actions issue) |
I changed the approach of the fix: I instead tell |
Sounds good enough to me. Let's merge. |
Just making sure windows CI errors in #228 also exist in master.
UPDATE: this PR fixes a coverage issue that was probably present since loky transitioned from appveyor to azure-pipelines, which made the overall coverage drop from 89% to 79% (first effective coverage drop noticed here: 6a0d37b)
UPDATE 2: this PR transitions the CI infrastructure of
loky
toazure-pipelines
for all platforms (Linux, Mac, Windows). Doing this, it also fixes a bug that was present since 6a0d37b and that prevented the windows coverage reports to be uploaded tocodecov
.