-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Upload junit reports codecov #13778
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
base: main
Are you sure you want to change the base?
Upload junit reports codecov #13778
Changes from all commits
ac8efe1
59a49f5
3fec230
c5d3ae4
3d7b325
64b9635
8f484d2
6af094b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -277,11 +277,15 @@ jobs: | |||||
| - name: Test without coverage | ||||||
| if: "! matrix.use_coverage" | ||||||
| shell: bash | ||||||
| env: | ||||||
| _PYTEST_TOX_POSARGS_JUNIT: --junitxml=junit-${{ matrix.name }}.xml | ||||||
| run: tox run -e ${{ matrix.tox_env }} --installpkg `find dist/*.tar.gz` | ||||||
|
|
||||||
| - name: Test with coverage | ||||||
| if: "matrix.use_coverage" | ||||||
| shell: bash | ||||||
| env: | ||||||
| _PYTEST_TOX_POSARGS_JUNIT: --junitxml=junit-${{ matrix.name }}.xml | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| run: tox run -e ${{ matrix.tox_env }}-coverage --installpkg `find dist/*.tar.gz` | ||||||
|
|
||||||
| - name: Upload coverage to Codecov | ||||||
|
|
@@ -292,6 +296,14 @@ jobs: | |||||
| files: ./coverage.xml | ||||||
| verbose: true | ||||||
|
|
||||||
| - name: Upload JUnit report to Codecov | ||||||
| uses: codecov/codecov-action@5a1091511ad55cbe89839c7260b706298ca349f7 | ||||||
| with: | ||||||
| fail_ci_if_error: false | ||||||
| files: junit-${{ matrix.name }}.xml | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency with the coverage upload step done above this one, let's use |
||||||
| report_type: test_results | ||||||
| verbose: true | ||||||
|
|
||||||
| check: # This job does nothing and is only used for the branch protection | ||||||
| if: always() | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1 @@ | ||||||||||||||
| automated uploading JUnit reports to codecov in the test workflow with codecov GitHub Action | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change notes are supposed to communicate the effect of upgrading since the previous release. They make up a coherent change log document, which is expected to follow consistent writing style and use full sentences and such. They shouldn't generally reference internal implementation details and be higher level and approachable to the end-users. This is a contrib note, though, so the "end users" are other contributors (and core devs). Here's a preview of your addition: https://pytest--13778.org.readthedocs.build/en/13778/changelog.html#contributor-facing-changes. To me, it looks a bit out of place because it doesn't have a full stop in the sentence, nor does the sentence start with a capital letter. This will probably fit better:
Suggested change
P.S. Technically, this PR does not require a change note but since you added it, let's make it more coherent with the rest. |
||||||||||||||
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.
Since this produces one file per job that doesn't go anywhere with this name, we can skip making the name dynamic.