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.

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

first shot at adding ~coveralls~ codecov integration #187

Merged
merged 11 commits into from
Nov 20, 2020

Conversation

amueller
Copy link
Contributor

No description provided.

@amueller amueller changed the title first shot at adding coveralls integration first shot at adding ~coveralls~ codecov integration Nov 19, 2020
@@ -175,6 +175,7 @@ RUN python3.7 -m pip install pip && \
COPY ./source/Mlos.Python/requirements.txt /tmp/
RUN python3.7 -m pip install -r /tmp/requirements.txt


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Copy link
Contributor

Choose a reason for hiding this comment

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

Or do we need pip install pytest-cov?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@2623df7). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #187   +/-   ##
=======================================
  Coverage        ?   79.28%           
=======================================
  Files           ?      168           
  Lines           ?     9457           
  Branches        ?        0           
=======================================
  Hits            ?     7498           
  Misses          ?     1959           
  Partials        ?        0           
Flag Coverage Δ
unittests 79.28% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 2623df7...877be1d. Read the comment docs.

@amueller
Copy link
Contributor Author

right now the PR coverage diff isn't working because there's no coverage for the base branch. So I think we should merge so that we can get coverage for that?

I have to see how to enable it as a CI check, but we already have file-level coverage for this PR: https://codecov.io/gh/microsoft/MLOS/pull/187/tree?path=source%2FMlos.Python%2Fmlos

@amueller
Copy link
Contributor Author

Not sure if you can see it, but in the settings here:
https://app.codecov.io/gh/microsoft/MLOS/settings

it tells me to enable something in the MS org which I don't have access to :-/

@bpkroth
Copy link
Contributor

bpkroth commented Nov 20, 2020

Not sure if you can see it, but in the settings here:
https://app.codecov.io/gh/microsoft/MLOS/settings

it tells me to enable something in the MS org which I don't have access to :-/

No, I get a "sorry you don't have access to this", but I didn't try very hard ...

We should check with @ksaur about what they did for the hummingbird repo.

@bpkroth
Copy link
Contributor

bpkroth commented Nov 20, 2020

Related: #33 (though perhaps not all of it)

@ksaur
Copy link

ksaur commented Nov 20, 2020

@bpkroth Hmm....I don't remember having to enable something special. Ping me tomorrow and we can try to troubleshoot. (I just checked that I can verify that link above if I s/MLOS/hummingbird/g, but I'm not sure what I did to enable it.)

@amueller
Copy link
Contributor Author

btw looks like we don't need to have a yaml file:
https://docs.codecov.io/docs/codecov-yaml

When I go to the config page, at the top it says "Github Integration is installed. However, this repository is not enabled.".

@amueller
Copy link
Contributor Author

I still think we should merge this as a first pass btw.

@bpkroth
Copy link
Contributor

bpkroth commented Nov 20, 2020

Do you want to add a badge to the front page, or leave that for later?
https://app.codecov.io/gh/microsoft/MLOS/settings/badge

Co-authored-by: Brian Kroth <bpkroth@users.noreply.github.com>
@amueller
Copy link
Contributor Author

Co-authored-by: Brian Kroth <bpkroth@users.noreply.github.com>
Copy link
Contributor

@bpkroth bpkroth left a comment

Choose a reason for hiding this comment

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

Thanks!

@amueller amueller merged commit f7b76f2 into microsoft:main Nov 20, 2020
bpkroth added a commit to bpkroth/MLOS that referenced this pull request Dec 2, 2020
- Increase the timeouts on the pytest runs.
  There have been some timeouts recently, due in part to the addition of code
  coverage tracking (microsoft#187).

- Separate the pylint checks (short) from the pytest runs (long, flaky) so that
  we can have the docker image publish task proceed in nightly runs even if the
  pytest run was flaky.
bpkroth added a commit that referenced this pull request Dec 2, 2020
* Tweaks to Python Test CI pipelines

- Increase the timeouts on the pytest runs.
  There have been some timeouts recently, due in part to the addition of code
  coverage tracking (#187).

- Separate the pylint checks (short) from the pytest runs (long, flaky) so that
  we can have the docker image publish task proceed in nightly runs even if the
  pytest run was flaky.

* Avoid duplicate ctest runs during Github CI runs

Fixes #196

* pass the GITHUB_WORKFLOW env var through the docker process

* fixup argument order
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants