Opened 4 months ago

Closed 2 months ago

Last modified 10 days ago

#36620 closed Cleanup/optimization (fixed)

Automate test coverage check on pull requests

Reported by: Jacob Walls Owned by: Saurabh
Component: Core (Other) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

We could automate a significant part of checking test coverage on pull requests. Instead of using a cloud service, there's an implementation using diff-cover in a PR.

PR

Forum post

Change History (18)

comment:1 by Jacob Walls, 4 months ago

Owner: set to Saurabh
Status: newassigned

comment:2 by Simon Charette, 4 months ago

I think the last time this was attempted the efforts stalled due the difficulty of combining coverage reports from different suite runs. Some part of the code is only covered by tests run on Postgres or a particular Python version for example so there needs to be a coordinated job that collects all of the .coverage artifacts and then combine them otherwise the resulting coverage report will be lacking or improperly reporting that some areas are not covered (e.g. if we only use the SQLite test run and Postgres only changes are introduced).

comment:3 by Jacob Walls, 4 months ago

(The linked implementation just treats that complexity as outside of MVP and makes this caveat explicit in the posted comment.)

comment:4 by Sarah Boyce, 4 months ago

Triage Stage: UnreviewedAccepted

comment:5 by Sarah Boyce, 4 months ago

Needs documentation: set
Patch needs improvement: set

comment:6 by Jacob Walls, 2 months ago

Needs documentation: unset
Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:7 by Jacob Walls, 2 months ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

comment:8 by Jacob Walls, 2 months ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:9 by Jacob Walls <jacobtylerwalls@…>, 2 months ago

Resolution: fixed
Status: assignedclosed

In a89183e6:

Fixed #36620 -- Added coverage workflow to summarize coverage in pull requests.

Part of GSoC 2025. Thanks Lily for mentorship, and Sarah Boyce and
Jacob Walls for reviews.

comment:10 by Jacob Walls <jacobtylerwalls@…>, 2 months ago

In e4c4a17:

Reverted "Fixed #36620 -- Added coverage workflow to summarize coverage in pull requests."

This reverts commit a89183e63844a937aacd3ddb73c4952ef869d2cc.

comment:11 by Jacob Walls, 2 months ago

Patch needs improvement: set
Resolution: fixed
Status: closednew
Triage Stage: Ready for checkinAccepted

Revision is being progressed here: https://github.com/django/django/pull/20302

comment:12 by GitHub <noreply@…>, 2 months ago

Resolution: fixed
Status: newclosed

In 26b0e2bb:

Fixed #36620 -- Fixed workflow to summarize coverage in PRs.

Follow-up to a89183e63844a937aacd3ddb73c4952ef869d2cc, which was
reverted in e4c4a178aa642f8493b7ae2c0ad58527af51f67e because a change
to the workflow trigger resulted in the PR branch not being checked out.

We used this opportunity to reimplement the coverage tracing and coverage
commenting in a two-workflow pattern with more granular permissions.

To reduce duplicative workflows, we removed the existing python test workflow
on PRs, at least until we run more distinct configurations on GitHub actions. The
run with coverage tracing enabled is sufficient for now. The existing workflow still
runs on pushes to main. We can revisit when adding more test configurations.

comment:13 by Jacob Walls, 2 months ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:14 by GitHub <noreply@…>, 2 months ago

In 1dfd5aa:

Refs #36620 -- Removed PR number null check from coverage_comment workflow.

comment:15 by GitHub <noreply@…>, 2 months ago

In fd4c5fa3:

Refs #36620 -- Fixed PR number extraction in coverage_comment workflow.

Passing the PR number as an artifact is more reliable in cross-fork workflows.

comment:16 by Jacob Walls <jacobtylerwalls@…>, 2 months ago

In e726254a:

Refs #36620 -- Added contributor documentation for code coverage reports.

This was included in the original reverted patch:
a89183e63844a937aacd3ddb73c4952ef869d2cc
Follow-up to 26b0e2bb92caf2d16cabe455792350f20d6f42ca.

comment:17 by Jacob Walls <jacobtylerwalls@…>, 2 months ago

In 2e7133c:

[6.0.x] Refs #36620 -- Added contributor documentation for code coverage reports.

This was included in the original reverted patch:
a89183e63844a937aacd3ddb73c4952ef869d2cc
Follow-up to 26b0e2bb92caf2d16cabe455792350f20d6f42ca.

Backport of e726254a380f2a35a2fcf71143e96cb5987d8102 from main.

comment:18 by Jacob Walls <jacobtylerwalls@…>, 10 days ago

In 2351c1b1:

Refs #36620 -- Ran coverage tests workflow on forks.

We can continue to limit the coverage comment workflow to django/django,
but now that this workflow is the main python test workflow, it should
run on forks by default. The other tests workflow (currently running
only JavaScript tests) may start running python tests again once we flesh
out the matrix, but since it was duplicating the coverage tests configuration,
we temporarily removed it.

Follow-up to 26b0e2bb92caf2d16cabe455792350f20d6f42ca.

Note: See TracTickets for help on using tickets.
Back to Top