Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ci: move coverage reporting to shared script #5403

Merged
merged 1 commit into from
Jun 6, 2019
Merged

ci: move coverage reporting to shared script #5403

merged 1 commit into from
Jun 6, 2019

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Jun 5, 2019 •
edited
Loading

No description provided.

@blueyed blueyed changed the title Azure: use backslashes with COVERAGE_{FILE,PROCESS_START} ci: Azure: use backslashes with COVERAGE_{FILE,PROCESS_START} Jun 5, 2019
@blueyed blueyed mentioned this pull request Jun 5, 2019
Merged
asottile
asottile approved these changes Jun 5, 2019 •
edited by blueyed
Loading
View reviewed changes
@codecov
Copy link

codecov bot commented Jun 5, 2019 •
edited
Loading

Codecov Report

Merging #5403 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5403   +/-   ##
=======================================
  Coverage   95.98%   95.98%           
=======================================
  Files         114      114           
  Lines       25523    25523           
  Branches     2480     2480           
=======================================
  Hits        24499    24499           
  Misses        718      718           
  Partials      306      306

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f8b566...0fd1f30. Read the comment docs.

@blueyed blueyed mentioned this pull request Jun 5, 2019
Closed
@blueyed blueyed changed the title ci: Azure: use backslashes with COVERAGE_{FILE,PROCESS_START} ci: Azure: fix coverage reporting Jun 5, 2019
This was referenced Jun 5, 2019
Closed
Closed
@blueyed blueyed force-pushed the fix-cov-win branch 2 times, most recently from 72752fe to 7891d69 Compare June 5, 2019 08:10
@blueyed

This comment has been minimized.

Sign in to view
@blueyed

This comment has been minimized.

Sign in to view
@blueyed

This comment has been minimized.

Sign in to view
@blueyed blueyed mentioned this pull request Jun 5, 2019
Merged
@blueyed blueyed self-assigned this Jun 5, 2019
@blueyed
Copy link
Contributor Author

blueyed commented Jun 5, 2019

So this is not required anymore strictly speaking (according to #5396), but I think it is good to have it in line with Travis.

Maybe we could move the common script part to an external file to keep them in sync?

@blueyed blueyed changed the title ci: Azure: fix coverage reporting [WIP/RFC] ci: Azure: fix coverage reporting Jun 5, 2019
@blueyed blueyed requested a review from asottile June 5, 2019 10:51
@nicoddemus
Copy link
Member

Maybe we could move the common script part to an external file to keep them in sync?

Agreed, please do. 😁

nicoddemus
nicoddemus requested changes Jun 5, 2019
View reviewed changes
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the common code to a separate script as we discussed. 👍

@blueyed blueyed changed the title [WIP/RFC] ci: Azure: fix coverage reporting ci: move coverage reporting to shared script Jun 6, 2019
@blueyed blueyed removed their assignment Jun 6, 2019
@blueyed blueyed requested a review from nicoddemus June 6, 2019 14:58
nicoddemus
nicoddemus reviewed Jun 6, 2019
View reviewed changes
azure-pipelines.yml
- script: |
call scripts/setup-coverage-vars.bat || goto :eof
- bash: |
if [[ "$PYTEST_COVERAGE" == "1" ]]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be calling scripts/report-coverage.sh no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that's the env setup.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be merged into tox.ini, and then we would end up using TOXENV=…-coverage basically.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh duh sorry

nicoddemus
nicoddemus approved these changes Jun 6, 2019
View reviewed changes
azure-pipelines.yml
- script: |
call scripts/setup-coverage-vars.bat || goto :eof
- bash: |
if [[ "$PYTEST_COVERAGE" == "1" ]]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh duh sorry

@nicoddemus nicoddemus merged commit 450d264 into pytest-dev:master Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicoddemus nicoddemus nicoddemus approved these changes

@asottile asottile Awaiting requested review from asottile

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@blueyed @nicoddemus @asottile