Skip to content

codecov coverage uploaded after tests succeed #6705

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

Closed

Conversation

michaelraczycki
Copy link
Contributor

@michaelraczycki michaelraczycki commented May 4, 2023

What is this PR about?
Adaptations of tests.yaml requested in #6660

**Implementation details:
Removed partial codecov upload that was triggered after each subtest, instead added one that is made only In case of all tests passing


📚 Documentation preview 📚: https://pymc--6705.org.readthedocs.build/en/6705/

@michaelraczycki michaelraczycki changed the title changing upload to trigger only with other test success codecov coverage uploaded after tests succeed May 5, 2023
Comment on lines 490 to 507
merge-reports:
runs-on: ubuntu-latest
needs: [all_tests, changes]
steps:
- name: Checkout code
uses: actions/checkout@v2
- name: Merge coverage reports
run: |
# Activate conda environment
conda activate pymc-test
# Find all coverage reports and merge them into a single file
python -m coverage combine --append coverage-*.xml

- name: Save combined coverage report
uses: actions/upload-artifact@v2
with:
name: coverage
path: coverage
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this extra job? PyTensor doesn't seem to have an equivalent one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pytensor has just one job ('test_ubuntu') that generates the codecov report. in pymc there are many more, that generate partial reports. Adding step that combines them into one final report after all report-generating jobs finish seemed like an easier task

Copy link
Member

@ricardoV94 ricardoV94 May 11, 2023

Choose a reason for hiding this comment

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

Okay, but it can still be done in a single job no? Just merge the two steps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

by two steps do you mean merge-reports and upload coverage?

Copy link
Member

@ricardoV94 ricardoV94 May 11, 2023

Choose a reason for hiding this comment

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

Yes, this way there's a bit less clutter in the PR checks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, should be possible

@ricardoV94 ricardoV94 closed this Mar 4, 2024
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.

2 participants