CI add codecov to GitHub Action workflow #31941

StefanieSenger · 2025-08-13T12:48:40Z

Reference Issues/PRs

Follow up on #31832

What does this implement/fix? Explain your changes.

This PR adds steps to the github actions workflow (unit-tests.yml) that merge and upload codecov coverage reports.

Thanks for your support @lesteve!

github-actions · 2025-08-13T12:49:43Z

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: 1634b47. Link to the linter CI: here}

StefanieSenger · 2025-08-13T13:00:00Z

This will not fail, but no coverage report will be created, because we are lacking a token. It needs to be collected from codecov and added into the project's secrets, like this guide explains: https://docs.codecov.com/docs/adding-the-codecov-token

When I try to sign in to codecov, I am asked to confirm to an oauth which among others lists "act on your behalf". Now, I am unsure. Should I confirm that, or would you rather add the token, @lesteve?

lesteve · 2025-08-13T13:05:42Z

I just added the Codecov token in the GitHub secrets.

Let's see what happens with the CI. My guess is that the codecov upload will work because no token is needed on a public repo PR, see doc for more details.

It's only when this PR is merged that a Codecov token will be needed for the Codecov upload of the main branch.

lesteve · 2025-08-13T13:23:19Z

One slightly weird thing I noticed in the log is that it uploads multiple coverage.xml the one in the root folder and the one in the TEST_DIR

info - 2025-08-13 12:59:23,247 -- Found 2 coverage files to report
info - 2025-08-13 12:59:23,247 -- > /home/runner/work/scikit-learn/scikit-learn/tmp_folder/coverage.xml
info - 2025-08-13 12:59:23,247 -- > /home/runner/work/scikit-learn/scikit-learn/coverage.xml

Having a quick look at the codecov-action README, maybe we want to use files: ./coverage.xml (i.e. additional ./) and/or disable_search: true?

…n non-editable mode

lesteve · 2025-08-13T13:54:27Z

I pushed a possible fix for the pylatest_pip_openblas_pandas failure. We'll see what the CI says about it ...

What's special about this build is that we are setting PIP_BUILD_ISOLATION: 'true' in azure-pipelines.yml and we build in non-editable mode. I guess in this case you need to be out of the scikit-learn root folder to be able to do python -c 'import sklearn'.

StefanieSenger · 2025-08-13T14:04:36Z

Having a quick look at the codecov-action README, maybe we want to use files: ./coverage.xml (i.e. additional ./) and/or disable_search: true?

Thanks, @lesteve. Let's try it.

… into gha_coverage

lesteve · 2025-08-14T10:30:14Z

.github/workflows/unit-tests.yml

@@ -88,9 +88,10 @@ jobs:

      - name: Upload coverage report to Codecov
        uses: codecov/codecov-action@v5
-        if: ${{ env.COVERAGE == 'true' }} # also condition on that SELECTED_TESTS is empty?
+        # This step (also the previous step) should depend on whether we run the whole
+        # test suite (maybe adding && env.SELECTED_TESTS == ''):


I think for now we can ignore this and leave it for later when we tackle commit-based markers. SELECTED_TESTS is something that is used when the [all random seeds] commit marker is used. When that happens you generally only run a few tests (but with all random seeds) and it wouldn't make sense to upload the coverage because it would be small.

Ignoring it for now sounds like a practical way to deal with it. Do you think this comment helps remind us what we need to do here, or should I remove it?

Leave the comment if you prefer, but I would put a TODO in front to make it slightly clearer that we are planning to tackle it

StefanieSenger · 2025-08-14T10:38:10Z

.github/workflows/unit-tests.yml

+        # This step (also the previous step) should depend on whether we run the whole
+        # test suite (maybe adding && env.SELECTED_TESTS == ''):
+        if: ${{ env.COVERAGE == 'true' }}


In Azure we here test if SELECTED_TESTS is empty (like adding && env.SELECTED_TESTS == '' in the condition below).

Right now, we cannot do that here, because get_selected_tests.py only seems to create an environmental variable valid in azure I believe and also calls get_commit_message.py and that's a separate task that is discussed here: #31832 (comment). I'm not sure if the discussion let to us having a plan and which one.

After SELECTED_TESTS is available here, we could add this condition in this step, or, alternatively, set COVERAGE: 'false' if SELECTED_TESTS is not empty early on, so that codecov doesn't run at all and we save this CI time.

lesteve · 2025-08-14T14:44:42Z

.codecov.yml

-    after_n_builds: 6
+    # Prevent codecov from calculating the coverage results before all expected uploads
+    # are in. This value is set to the total number of jobs uploading coverage reports:
+    # 6 Azure Pipeline jobs plus 1 GitHub Actions job.


I would remove this line because the exact number of Azure and GitHub jobs will change during the transition to GHA. IMO this line will become outdated quickly because it's close to impossible to remember to update it each time we move a CI build to GHA.

Okay, I have removed it. The idea had been to make it easy to detect if the number is out of date. Let's see if we manage to remember to change this number. 🤞 😄

lesteve · 2025-08-14T14:53:10Z

build_tools/azure/combine_coverage_reports.sh

 pushd $TEST_DIR
 coverage combine --append
 coverage xml
 popd

 # Copy the combined coverage file to the root of the repository:
-cp $TEST_DIR/coverage.xml $BUILD_REPOSITORY_LOCALPATH
+cp $TEST_DIR/coverage.xml .


Comment for potential other reviewers: BUILD_REPOSITORY_LOCALPATH is a Azure-specific environment variable that points where the scikit-learn root folder is located.

Maybe GitHub has an equivalent environment variable, but it feels slightly simpler to rely on the assumption that the current working directory is the same as the scikit-learn repo root folder.

lesteve · 2025-08-14T14:55:30Z

build_tools/azure/test_script.sh

    # CI logs. The coverage data is consolidated by codecov to get an online
    # web report across all the platforms so there is no need for this text
    # report that otherwise hides the test failures and forces long scrolls in
    # the CI logs.
-    export COVERAGE_PROCESS_START="$BUILD_SOURCESDIRECTORY/.coveragerc"
+    export COVERAGE_PROCESS_START="$CHECKOUT_FOLDER/.coveragerc"


Similar comment as https://github.com/scikit-learn/scikit-learn/pull/31941/files#r2276870682. It feels slightly preferable to rely on the assumption that the current working directory is the scikit-learn repo root folder.

lesteve · 2025-08-14T15:20:50Z

.github/workflows/unit-tests.yml

+        if: ${{ env.COVERAGE == 'true' }}
+
+      - name: Upload coverage report to Codecov
+        uses: codecov/codecov-action@v5


I guess one thing to think about for potential reviewers is whether we are OK with using an external GitHub action. Full disclosure: a few years ago the Codecov uploader was compromised see this.

The only secret being exposed would be the Codecov token. I am not sure how much harm can be done with this token (upload fake coverage reports to Codecov sure but something else?).

An alternative would be to use our Azure script build_tools/azure/upload_coverage.sh but it would need to be adapted for GitHub, it uses Azure-specific BUILD_ environment variables.

It is the build_tools/azure/upload_codecov.sh file.

StefanieSenger · 2025-08-15T12:57:03Z

build_tools/azure/test_script.sh

-    TEST_CMD="$TEST_CMD --cov-config='$COVERAGE_PROCESS_START' --cov sklearn --cov-report="
+    TEST_CMD="$TEST_CMD --cov-config='$COVERAGE_PROCESS_START' --cov=sklearn --cov-report="


This diff was necessary to fix the order of the cli arguments.

For some reason, this was evaluated as TEST_CMD="$TEST_CMD --cov-config='$COVERAGE_PROCESS_START' --cov --cov-report= sklearn" when eval "$TEST_CMD" ran and thus not understood. With the = it works on azure and github actions.

CI add codecov to GitHub Action workflow

310a4dd

github-actions bot added the Build / CI label Aug 13, 2025

StefanieSenger added the No Changelog Needed label Aug 13, 2025

[azure parallel] fix for pylatest_pip_openblas_pandas that is built i…

b915d6d

…n non-editable mode

attempt to constrain upload file to one file

5bd4b76

StefanieSenger added 3 commits August 13, 2025 16:04

Merge branch 'gha_coverage' of github.com:StefanieSenger/scikit-learn…

83c26bb

… into gha_coverage

change number of reports to await

5a93069

improve comment

64ce809

lesteve reviewed Aug 14, 2025

View reviewed changes

StefanieSenger commented Aug 14, 2025

View reviewed changes

StefanieSenger marked this pull request as ready for review August 14, 2025 10:57

lesteve reviewed Aug 14, 2025

View reviewed changes

StefanieSenger and others added 2 commits August 15, 2025 14:45

nicer comments

a9f2a7a

Merge branch 'main' into gha_coverage

1634b47

StefanieSenger commented Aug 15, 2025

View reviewed changes

		TEST_CMD="$TEST_CMD --cov-config='$COVERAGE_PROCESS_START' --cov sklearn --cov-report="
		TEST_CMD="$TEST_CMD --cov-config='$COVERAGE_PROCESS_START' --cov=sklearn --cov-report="

Uh oh!

CI add codecov to GitHub Action workflow #31941

Are you sure you want to change the base?

CI add codecov to GitHub Action workflow #31941

Conversation

StefanieSenger commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Uh oh!

github-actions bot commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✔️ Linting Passed

Uh oh!

StefanieSenger commented Aug 13, 2025

Uh oh!

lesteve commented Aug 13, 2025

Uh oh!

lesteve commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lesteve commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

StefanieSenger commented Aug 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lesteve Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lesteve Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

StefanieSenger commented Aug 13, 2025 •

edited

Loading

github-actions bot commented Aug 13, 2025 •

edited

Loading

lesteve commented Aug 13, 2025 •

edited

Loading

lesteve commented Aug 13, 2025 •

edited

Loading

lesteve Aug 14, 2025 •

edited

Loading

lesteve Aug 14, 2025 •

edited

Loading