-
Notifications
You must be signed in to change notification settings - Fork 27
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
Set up our pytest suite for the codecov integration #502
base: main
Are you sure you want to change the base?
Conversation
ci/scripts/run_pytest.sh
Outdated
gpgv codecov.SHA256SUM.sig codecov.SHA256SUM | ||
|
||
shasum -a 256 -c codecov.SHA256SUM | ||
sudo chmod +x codecov |
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.
any way we can make this work without sudo? I worry run_pytest.sh
will be run in some CI environment where sudo isn't available.
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.
Thinking out loud, you might want to make this bash script with a command like arg to optionally disable codecov
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.
hmm good catch, this was a snippet from the codecov CLI help I copied + pasted in.
For now I'm fairly sure that we have sudo in our base container; the other option would be to set this up during the container build
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.
might be easier to just install codecov-cli
in our requirements-test.txt. edit, well, that leads to a conflicting httpx version. So I think the options are
- use sudo here for now (benefits being that codecov doesn't end up in our release container, but tests might take slightly longer to run, and we use
sudo
.) - install the codecov binary as part of the docker image. Might get cached more easily, but we'd end up shipping this as part of our release image.
if we had a bionemo-testing
image, this might be a bit cleaner 🤷
ci/scripts/run_pytest.sh
Outdated
junitparser merge *.junit.xml combined.junit.xml | ||
|
||
# Upload test analytics to codecov. | ||
./codecov do-upload --report-type test_results --file combined.junit.xml |
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.
maybe I don't know how codecov works, but where is this being uploaded?
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.
Yeah IIUC this will upload by default to the standard codecov cloud, using our CODECOV_TOKEN to identify / authenticate
ci/scripts/run_pytest.sh
Outdated
gpgv codecov.SHA256SUM.sig codecov.SHA256SUM | ||
|
||
shasum -a 256 -c codecov.SHA256SUM | ||
sudo chmod +x codecov |
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.
I would set integration with codecov
as optional, controlled by a flag --codecov
to run_pytest.sh
script
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.
ok, added a --no-codecov
option
/build-ci |
0164880
to
5bd84a2
Compare
/build-ci |
This will require @dorotat-nv to add the CODECOV_TOKEN secret to our blossom runners.