perf(tests): optimize coverage instrumentation and reporting pipeline #484

Closed
brent.edwards wants to merge 3 commits from perf/optimize-coverage-pipeline into master
Member

Summary

  • Replace coverage.py (sys.settrace) with slipcover (bytecode instrumentation) for faster coverage collection
  • Each behave-parallel worker produces per-feature JSON coverage files; slipcover --merge combines them
  • CI workflow JSON key lookups handle both slipcover and coverage.py output formats
  • Documentation updated to reflect slipcover as the coverage tool

Type of Change

  • Performance improvement
  • CI/CD changes
  • Documentation update

Quality Checklist

  • Code follows the project's coding standards (see CONTRIBUTING.md)
  • nox -s typecheck passes with no errors
  • nox -s lint passes with no errors
  • Integration tests pass (Robot coverage_threshold.robot: 6/6)
  • Coverage remains above 97%
  • No security issues introduced
  • Documentation updated

Testing

  • Robot Framework coverage_threshold.robot passes all 6 tests
  • Coverage JSON output verified: 97.5% with slipcover

Part of #478. Closes #482.

## Summary - Replace coverage.py (sys.settrace) with slipcover (bytecode instrumentation) for faster coverage collection - Each behave-parallel worker produces per-feature JSON coverage files; slipcover --merge combines them - CI workflow JSON key lookups handle both slipcover and coverage.py output formats - Documentation updated to reflect slipcover as the coverage tool ## Type of Change - [x] Performance improvement - [x] CI/CD changes - [x] Documentation update ## Quality Checklist - [x] Code follows the project's coding standards (see CONTRIBUTING.md) - [x] `nox -s typecheck` passes with no errors - [x] `nox -s lint` passes with no errors - [x] Integration tests pass (Robot coverage_threshold.robot: 6/6) - [x] Coverage remains above 97% - [x] No security issues introduced - [x] Documentation updated ## Testing - Robot Framework `coverage_threshold.robot` passes all 6 tests - Coverage JSON output verified: 97.5% with slipcover ## Related Issues Part of #478. Closes #482.
perf(tests): optimize coverage instrumentation and reporting pipeline
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 14s
CI / build (pull_request) Successful in 17s
CI / quality (pull_request) Successful in 17s
CI / security (pull_request) Successful in 30s
CI / typecheck (pull_request) Successful in 33s
CI / integration_tests (pull_request) Successful in 2m42s
CI / unit_tests (pull_request) Successful in 11m53s
CI / coverage (pull_request) Successful in 11m52s
CI / docker (pull_request) Successful in 1m3s
CI / benchmark-regression (pull_request) Successful in 25m37s
4629c44fc0
Replace coverage.py (sys.settrace-based) with slipcover (bytecode-based
instrumentation) for significantly faster coverage collection:

- Each behave-parallel worker runs under slipcover, producing per-feature
  JSON coverage files with unique UUIDs to avoid write contention.
- After all workers finish, slipcover --merge combines per-worker data
  into a single build/coverage.json report.
- XML report generated via slipcover --merge --xml for CI tooling.
- Terminal report with --fail-under=97 threshold enforcement.
- Robust JSON key-fallback logic handles both slipcover ('summary') and
  coverage.py ('totals') output formats.

CI workflow fixes:
- ci.yml 'Surface coverage summary' step uses defensive key lookup
  instead of hardcoded ['totals']['percent_covered'].
- nightly-quality.yml applies same fallback pattern.

Documentation updated to reflect slipcover as the coverage tool.

Part of #478. Closes #482.
Merge branch 'master' into perf/optimize-coverage-pipeline
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 14s
CI / build (pull_request) Successful in 24s
CI / quality (pull_request) Successful in 30s
CI / typecheck (pull_request) Successful in 33s
CI / security (pull_request) Successful in 50s
CI / integration_tests (pull_request) Successful in 4m44s
CI / coverage (pull_request) Successful in 12m31s
CI / benchmark-regression (pull_request) Successful in 20m58s
CI / unit_tests (pull_request) Successful in 25m18s
CI / docker (pull_request) Successful in 38s
812a880811
Merge branch 'master' into perf/optimize-coverage-pipeline
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 16s
CI / build (pull_request) Successful in 15s
CI / quality (pull_request) Successful in 23s
CI / security (pull_request) Successful in 30s
CI / typecheck (pull_request) Successful in 55s
CI / integration_tests (pull_request) Successful in 2m55s
CI / benchmark-regression (pull_request) Successful in 21m55s
CI / unit_tests (pull_request) Successful in 24m41s
CI / docker (pull_request) Successful in 52s
CI / coverage (pull_request) Successful in 24m45s
fd82a9852c
freemo left a comment

Code Review: PR #484 — perf(tests): optimize coverage instrumentation and reporting pipeline

What I Fixed

  • Labels: Removed incorrectly applied State/Verified, Priority/High, MoSCoW/Should have, Points/8 labels (these belong on issues only per CONTRIBUTING.md §8.1). Only Type/Task remains.

Code Quality Assessment

The code changes are solid. Key observations:

  1. slipcover migration — Clean replacement of coverage.py with slipcover. The per-worker UUID file approach avoids write contention correctly.
  2. Defensive JSON parsing — The fallback logic in noxfile.py (checking summary, totals, multiple key names, computing percentage from raw counts) is thorough — perhaps overly so, but it won't break anything and handles format differences gracefully.
  3. CI workflow fixes — The ci.yml and nightly-quality.yml changes correctly apply the same defensive key lookup pattern.
  4. Documentationdocs/development/testing.md properly updated to reflect slipcover.
  5. Dependency changecoverage>=7.11.0slipcover>=1.0.17 in pyproject.toml is clean.
  6. Whitespace cleanup in CI YAML files is fine.

No bugs or design issues identified in the code itself.

Process Violations Requiring Changes

  1. Merge commits present — This PR has 3 commits (2 merge commits from syncing with master + 1 real commit). Per CONTRIBUTING.md §4.1, each PR should contain exactly one atomic commit. Please rebase and squash to a single commit.

  2. Missing ISSUES CLOSED: footer in commit message — The commit message uses Closes #482. in the body text, but CONTRIBUTING.md §5.5 requires the footer format:

    ISSUES CLOSED: #482
    
  3. Missing CHANGELOG.md update — Per CONTRIBUTING.md §8.3, every PR must include a CHANGELOG entry.

  4. Missing milestone — Per CONTRIBUTING.md §8.2, every PR must have a milestone matching the linked issue. Neither this PR nor issue #482 has a milestone set. Please determine the correct milestone and set it on both.

  5. Missing dependency links — Per CONTRIBUTING.md §7.3-7.4, this PR should have a "blocks #482" link, and issue #482 should have a "depends on #484" link. Please add these in the Forgejo UI.

Verdict

REQUEST CHANGES — The code is well-implemented, but the process requirements above must be addressed before merge.

## Code Review: PR #484 — perf(tests): optimize coverage instrumentation and reporting pipeline ### What I Fixed - **Labels**: Removed incorrectly applied `State/Verified`, `Priority/High`, `MoSCoW/Should have`, `Points/8` labels (these belong on issues only per CONTRIBUTING.md §8.1). Only `Type/Task` remains. ### Code Quality Assessment The code changes are solid. Key observations: 1. **slipcover migration** — Clean replacement of coverage.py with slipcover. The per-worker UUID file approach avoids write contention correctly. 2. **Defensive JSON parsing** — The fallback logic in `noxfile.py` (checking `summary`, `totals`, multiple key names, computing percentage from raw counts) is thorough — perhaps overly so, but it won't break anything and handles format differences gracefully. 3. **CI workflow fixes** — The `ci.yml` and `nightly-quality.yml` changes correctly apply the same defensive key lookup pattern. 4. **Documentation** — `docs/development/testing.md` properly updated to reflect slipcover. 5. **Dependency change** — `coverage>=7.11.0` → `slipcover>=1.0.17` in `pyproject.toml` is clean. 6. **Whitespace cleanup** in CI YAML files is fine. No bugs or design issues identified in the code itself. ### Process Violations Requiring Changes 1. **Merge commits present** — This PR has 3 commits (2 merge commits from syncing with `master` + 1 real commit). Per CONTRIBUTING.md §4.1, each PR should contain exactly one atomic commit. Please rebase and squash to a single commit. 2. **Missing `ISSUES CLOSED:` footer in commit message** — The commit message uses `Closes #482.` in the body text, but CONTRIBUTING.md §5.5 requires the footer format: ``` ISSUES CLOSED: #482 ``` 3. **Missing CHANGELOG.md update** — Per CONTRIBUTING.md §8.3, every PR must include a CHANGELOG entry. 4. **Missing milestone** — Per CONTRIBUTING.md §8.2, every PR must have a milestone matching the linked issue. Neither this PR nor issue #482 has a milestone set. Please determine the correct milestone and set it on both. 5. **Missing dependency links** — Per CONTRIBUTING.md §7.3-7.4, this PR should have a "blocks #482" link, and issue #482 should have a "depends on #484" link. Please add these in the Forgejo UI. ### Verdict **REQUEST CHANGES** — The code is well-implemented, but the process requirements above must be addressed before merge.
Owner

Superseded by #493 — this PR has been consolidated with all code review fixes applied into a single clean branch (perf/bdd-test-optimization) with one commit per issue and no merge commits.

Superseded by #493 — this PR has been consolidated with all code review fixes applied into a single clean branch (`perf/bdd-test-optimization`) with one commit per issue and no merge commits.
freemo closed this pull request 2026-03-02 01:45:31 +00:00
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 16s
Required
Details
CI / build (pull_request) Successful in 15s
Required
Details
CI / quality (pull_request) Successful in 23s
Required
Details
CI / security (pull_request) Successful in 30s
Required
Details
CI / typecheck (pull_request) Successful in 55s
Required
Details
CI / integration_tests (pull_request) Successful in 2m55s
Required
Details
CI / benchmark-regression (pull_request) Successful in 21m55s
CI / unit_tests (pull_request) Successful in 24m41s
Required
Details
CI / docker (pull_request) Successful in 52s
Required
Details
CI / coverage (pull_request) Successful in 24m45s
Required
Details

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core!484
No description provided.