From fb99e9bbdc8166b8c8345f6663b1a2d696d6882d Mon Sep 17 00:00:00 2001 From: Joshua Terranova Date: Mon, 10 Aug 2026 18:32:52 -0700 Subject: [PATCH 1/2] CI: keep per-matrix coverage artifacts and fail Codecov errors (#1088) --- .github/workflows/test_pytest.yaml | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test_pytest.yaml b/.github/workflows/test_pytest.yaml index a207282fd..c6f0b7a59 100644 --- a/.github/workflows/test_pytest.yaml +++ b/.github/workflows/test_pytest.yaml @@ -98,12 +98,16 @@ jobs: - name: Run Acceptance Tests run: pytest tests/acceptance --cov=rocketpy --cov-append --cov-report=xml + # Unique basename so CodecovUpload can merge-multiple without + # clobbering six coverage.xml files into one. + - name: Name coverage report for matrix leg + run: mv coverage.xml "coverage-${{ matrix.os }}-py${{ matrix.python-version }}.xml" + - name: Upload coverage to artifacts uses: actions/upload-artifact@main with: - name: coverage - path: coverage.xml - overwrite: true + name: coverage-${{ matrix.os }}-py${{ matrix.python-version }} + path: coverage-${{ matrix.os }}-py${{ matrix.python-version }}.xml if-no-files-found: error CodecovUpload: @@ -111,11 +115,15 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@main - - name: Download latest coverage report + - name: Download coverage reports uses: actions/download-artifact@main + with: + pattern: coverage-* + path: coverage-reports + merge-multiple: true - name: Upload to Codecov uses: codecov/codecov-action@main with: token: ${{ secrets.CODECOV_TOKEN }} - files: | - coverage.xml + directory: coverage-reports + fail_ci_if_error: true From 59851ee0abdf1a99faf20da35ca6afbd44247a9a Mon Sep 17 00:00:00 2001 From: Gui-FernandesBR Date: Wed, 12 Aug 2026 00:08:45 -0300 Subject: [PATCH 2/2] CI: address review on the per-matrix coverage upload (#1088) The artifact renaming was the right diagnosis, but review turned up eight follow-ups: - fail_ci_if_error was unconditional, and forks get no secrets, so an external contributor would see red whenever the tokenless upload got rate limited. Tie it to the token, as #1088 prescribed. - overwrite: true was dropped. Artifacts are keyed by (run, name) and survive across attempts, so every leg of a re-run failed with 409 Conflict. The flaky VTK tests make re-runs routine. - needs: Pytest with no if: guard skipped CodecovUpload entirely when a single leg failed, sending Codecov nothing at all for the commit while .codecov.yml resolves that as an error. Run on !cancelled() and upload whatever legs did finish. - download-artifact exits 0 when pattern matches fewer artifacts than expected, so five of six reports looked like a clean run. Verify the count against the matrix size and refuse the silently-partial case. - Both .codecov.yml statuses were scoped to a `unit` flag that no upload has ever tagged, so the gate resolved over an empty flag set and measured nothing. The reports are unit + doctest + integration + acceptance combined, so drop the flag rather than mislabel them. - --cov-report=xml: writes the per-leg filename directly, so the extra mv step goes away and if-no-files-found: error is reachable again instead of being masked by the mv failing first. - retention-days: 1, since artifacts per run went from 1 to 6 and CodecovUpload consumes them minutes later. - env: OS/PYTHON and .github/workflows/upload-to-codecov.yml were dead. The reusable workflow was called by nothing and was the only consumer of those two variables, while holding a second, diverging copy of the Codecov settings. Co-Authored-By: Claude Opus 5 (1M context) --- .codecov.yml | 10 ++-- .github/workflows/test_pytest.yaml | 62 +++++++++++++++++++++---- .github/workflows/upload-to-codecov.yml | 29 ------------ 3 files changed, 59 insertions(+), 42 deletions(-) delete mode 100644 .github/workflows/upload-to-codecov.yml diff --git a/.codecov.yml b/.codecov.yml index 39d77580d..3d023771c 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -6,8 +6,11 @@ coverage: target: auto threshold: 1% base: auto - flags: - - unit + # No `flags:` on purpose. This used to scope the status to a `unit` flag + # that no upload has ever tagged, so Codecov resolved it over an empty + # flag set and the gate measured nothing. The reports test_pytest.yaml + # sends are unit + doctest + integration + acceptance combined, so there + # is no honest flag to name here. paths: - "rocketpy" # advanced settings @@ -29,7 +32,6 @@ coverage: - develop if_ci_failed: error # success, failure, error, ignore only_pulls: false - flags: - - "unit" + # See the note on project.default above: no upload tags a `unit` flag. paths: - "rocketpy" diff --git a/.github/workflows/test_pytest.yaml b/.github/workflows/test_pytest.yaml index c6f0b7a59..e8e2bdfef 100644 --- a/.github/workflows/test_pytest.yaml +++ b/.github/workflows/test_pytest.yaml @@ -13,6 +13,13 @@ defaults: run: shell: bash +env: + # The Pytest matrix below is 3 os x 2 python-version, so CodecovUpload must + # receive six coverage reports. Nothing can derive a matrix size from another + # job, so it is written out here; the guard in CodecovUpload fails loudly if + # the matrix grows and this does not. + COVERAGE_LEG_COUNT: 6 + jobs: Pytest: runs-on: ${{ matrix.os }} @@ -24,8 +31,6 @@ jobs: os: [ubuntu-latest, macos-latest, windows-latest] python-version: ["3.10", "3.14"] env: - OS: ${{ matrix.os }} - PYTHON: ${{ matrix.python-version }} MPLBACKEND: Agg steps: - uses: actions/checkout@main @@ -95,13 +100,14 @@ jobs: fi done + # This is the only step that writes XML; the earlier ones only --cov-append + # into .coverage. Naming the report here, rather than renaming it after + # the fact, is what lets CodecovUpload merge all six into one directory + # without them clobbering each other as six identical coverage.xml. - name: Run Acceptance Tests - run: pytest tests/acceptance --cov=rocketpy --cov-append --cov-report=xml - - # Unique basename so CodecovUpload can merge-multiple without - # clobbering six coverage.xml files into one. - - name: Name coverage report for matrix leg - run: mv coverage.xml "coverage-${{ matrix.os }}-py${{ matrix.python-version }}.xml" + run: >- + pytest tests/acceptance --cov=rocketpy --cov-append + --cov-report=xml:coverage-${{ matrix.os }}-py${{ matrix.python-version }}.xml - name: Upload coverage to artifacts uses: actions/upload-artifact@main @@ -109,9 +115,20 @@ jobs: name: coverage-${{ matrix.os }}-py${{ matrix.python-version }} path: coverage-${{ matrix.os }}-py${{ matrix.python-version }}.xml if-no-files-found: error + # Artifacts are keyed by (run, name) and survive across attempts, so + # without this every leg of a re-run fails with 409 Conflict. The VTK + # tests above are flaky enough that re-runs are routine. + overwrite: true + # CodecovUpload consumes these minutes later and nothing else reads + # them, so the 90 day default would be six dead artifacts per run. + retention-days: 1 CodecovUpload: needs: Pytest + # `needs` alone skips this job when a single leg fails, which sends Codecov + # nothing at all for the commit and .codecov.yml resolves that as an error. + # Run unless the workflow was cancelled and upload the legs that did finish. + if: ${{ !cancelled() }} runs-on: ubuntu-latest steps: - uses: actions/checkout@main @@ -121,9 +138,36 @@ jobs: pattern: coverage-* path: coverage-reports merge-multiple: true + + # download-artifact exits 0 when `pattern` matches fewer artifacts than + # expected, so five of six reports would otherwise look like a clean run + # and quietly under-report coverage. That silence is what #1088 is about. + - name: Verify every leg reported coverage + id: reports + env: + PYTEST_RESULT: ${{ needs.Pytest.result }} + run: | + mkdir -p coverage-reports + count=$(find coverage-reports -maxdepth 1 -name '*.xml' | wc -l) + echo "count=$count" >> "$GITHUB_OUTPUT" + echo "Downloaded $count of $COVERAGE_LEG_COUNT coverage reports." + if [ "$PYTEST_RESULT" = "success" ] && [ "$count" -ne "$COVERAGE_LEG_COUNT" ]; then + echo "::error::Every Pytest leg passed, but only $count of $COVERAGE_LEG_COUNT coverage reports arrived." + exit 1 + fi + if [ "$count" -eq 0 ]; then + echo "::warning::No coverage report to upload; no Pytest leg produced one." + fi + - name: Upload to Codecov + # Nothing to send, and Codecov would fail on an empty directory. The + # failing Pytest leg is already reporting the real problem. + if: steps.reports.outputs.count != '0' uses: codecov/codecov-action@main with: token: ${{ secrets.CODECOV_TOKEN }} directory: coverage-reports - fail_ci_if_error: true + # Forks get no secrets, so the token above is empty for them and the + # tokenless upload is rate limited. An external contributor should not + # see red for a Codecov-side hiccup in their pull request. + fail_ci_if_error: ${{ secrets.CODECOV_TOKEN != '' }} diff --git a/.github/workflows/upload-to-codecov.yml b/.github/workflows/upload-to-codecov.yml deleted file mode 100644 index e83be8536..000000000 --- a/.github/workflows/upload-to-codecov.yml +++ /dev/null @@ -1,29 +0,0 @@ -name: Upload to Codecov - -on: - workflow_call: - inputs: - codecov_token: - required: true - type: string - os: - required: true - type: string - python: - required: true - type: string - -jobs: - upload: - runs-on: ubuntu-latest - steps: - - name: Upload coverage report to Codecov - uses: codecov/codecov-action@main - with: - token: ${{ inputs.codecov_token }} - directory: ./coverage/reports/ - env_vars: OS,PYTHON - files: ./coverage.xml, ./rocketpy/coverage.xml - flags: unittests - name: codecov-umbrella - verbose: true