Repository navigation
ci: measure test coverage and report it to Codecov - #826
Draft
vishal-bala wants to merge 5 commits into
Draft
vishal-bala wants to merge 5 commits into
vishal-bala wants to merge 5 commits into
Conversation
RedisVL has not measured coverage since #262 removed the Codecov upload. Every test.yml run now collects it and uploads each CI segment under its own Codecov flag: - core: every leg of the PR test matrix, combined - service: the API-key suite, carried forward on fork and Dependabot PRs, which skip it A new Coverage job combines the data, writes per-segment totals and a per-file table to the job summary, uploads an HTML report and sends each segment to Codecov. Statuses are informational and a failed upload never fails CI. Coverage follows subprocesses (patch = ["subprocess"]), so the CLI tests that spawn `python -m redisvl.cli.runner` count. pytest-cov joins the dev group; local `make test` is unchanged unless ARGS passes --cov.
…ately The service flag measures the full suite plus the API-key tests, not the API-key tests alone, so on fork PRs the carried-forward value can hide a drop in non-API code; point readers at the core row. Dependabot PRs get no Codecov token and are not reported. Note in codecov.yml that only service is carried forward, and where a new segment must be added.
The cluster-tests job now measures coverage and uploads its data file, and the coverage job combines it as a third segment under the Codecov flag `cluster`. The job runs only on the schedule and on manual dispatch, so the flag is carried forward on every other run; only the nightly run on main measures every segment.
…ks a report A workflow_dispatch run also runs the cluster and service jobs, so it measures every segment, not just the nightly run. Codecov carries a flag forward from the nearest ancestor commit with a report for it, not from the latest report anywhere.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
RedisVL has not measured test coverage since #262 removed the Codecov upload, so nobody can see which modules the suite leaves untested. This PR measures coverage on every run of
test.ymland reports it to Codecov, which is what redis-py and other Redis projects use. The suite runs in three segments under different conditions (the PR matrix without API keys, the full suite with API keys on same-repo PRs, and the cluster suite that #796 adds to the nightly run), so each segment is uploaded under its own Codecov flag rather than as one number.Changes
Collecting coverage in each segment
Every job that runs tests now writes its own coverage data file and uploads it as an artifact, even when tests fail:
coretestmatrix.coverage.core-py<version>-redis-py-<version>-<image>serviceservice-tests(make test-all, Python 3.11).coverage.serviceclustercluster-tests(-m requires_cluster).coverage.cluster-redis-py-<version>The
service-testsjob passes--covthrough a new optionalpytest-argsinput on.github/actions/run-service-tests;test-fork-pr.ymldoes not pass it and runs exactly as before.pyproject.tomlgains[tool.coverage.*]settings: branch coverage ofredisvl, paths stored relative to the checkout so data from different runners combines, andpatch = ["subprocess"]. The last one matters because the CLI tests runpython -m redisvl.cli.runnerin a child process; without it the CLI is measured as untested.Combining and reporting
A new
coveragejob runs after the three test jobs whatever their outcome (!cancelled()), so a skipped job just leaves its segment out. For each segment it combines that segment's data files, writes a Cobertura XML report and uploads it to Codecov under the segment's flag. It then combines all segments into the job summary, as a per-segment totals table plus a per-file table, and uploads an HTML report as an artifact. Uploading once per segment, instead of once per leg, means Codecov never comments before all the matrix legs have reported.Per-segment data is kept in
coverage-segments/rather than beside.coverage, becausepatch = ["subprocess"]turns on coverage's parallel mode, andcoverage reportwould otherwise absorb any.coverage.*file next to its data file.codecov.ymlmakes both statuses informational and carriesserviceandclusterforward on runs that skip them, from the nearest ancestor commit with a report for that flag.pytest-cov>=7,<8joins thedevgroup. Localmake testis unchanged unlessARGSpasses--cov.CONTRIBUTING.mdgains a "Coverage in CI" section and a working local command;AGENTS.mdgains one line under Testing Notes.Notes
This PR is stacked on #796, because the
clustersegment instruments thecluster-testsjob that #796 adds. It merges #796's branch rather than rebasing onto it, so against #796 the diff contains only the coverage change.The
serviceflag measures the full suite plus the API-key tests, because the service job runsmake test-all. On a fork PR, which skips that job, Codecov carriesserviceforward, so the headline number can hide a drop in code thatservicecovered; thecorerow in the Codecov comment shows the PR's own change. Narrowing the service job to the API-key tests would remove the masking but also stop that job testing the rest of the suite, and not carrying the flag forward would make every fork PR show a false drop.Dependabot PRs receive no Codecov token and Codecov refuses tokenless uploads from branches in this repository, so they are not reported to Codecov. The
Coveragejob summary still shows their totals.codecov/codecov-actionis pinned by tag (@v7), matching how every other third-party action in this repository is pinned.Coverage is reported, not enforced: neither status is required, and a failed Codecov upload never fails CI (
fail_ci_if_error: false). A target can be chosen once there is some history.Reviewer Brief
Start here:
.github/workflows/test.yml, thecoveragejob's "Combine coverage" step. It decides which segments reach Codecov and what the job summary shows.Where it could fail
coveragejob'sneeds: [test, service-tests, cluster-tests]withif: ${{ !cancelled() }}: on every PR and pushcluster-testsis skipped, andcoremust still upload; check the expression and the "not run" branch.include-hidden-files: true.codecov.yml:clusteris carried forward through push-only commits onmain, which uploadcoreandservicebut nocluster; if Codecov does not carry it that far, PRs show noclusterflag.Not verified: no CI run of this workflow yet, so the 3.14 and 3.15 legs (which use coverage's
sys.monitoringcore), the integration suite and the cluster suite have not run under coverage, and the slowdown per leg is unknown; how Codecov resolves the XML paths (<source>redisvl</source>with paths relative to it) and how far it carries a flag forward; tokenless uploads from fork PRs; actionlint was not run.Beyond CI: on Python 3.13 the unit suite gives the same result with and without
--cov(1,993 passed, 1 skipped), with wall time going from 51.5 s to 55.2 s;redisvl/cli/runner.pyreads 0% without subprocess patching and 86% with it, and pytest-cov writes exactly the named data file both under xdist and with-n 0; the "Combine coverage" script, extracted from the workflow and run on local data files, produced the expected summary and outputs both with all three segments and withcorealone;codecov.ymlpasses Codecov's validator (before theclusterflag was added).Reviewed
6344472a(correctness, security, design, tests, docs, Redis behaviour) and6d8e9f98(correctness, tests, docs), fixes in7259bd51and7b67a746· Claude Code (claude-opus-5-5)