Skip to content

refactor: decompose BuildContractCoverageFunc + dedup utilities#192

Merged
yvonnedevlinrh merged 1 commit into
unbound-force:mainfrom
yvonnedevlinrh:opsx/crapload-decompose-pr2a
Jul 16, 2026
Merged

refactor: decompose BuildContractCoverageFunc + dedup utilities#192
yvonnedevlinrh merged 1 commit into
unbound-force:mainfrom
yvonnedevlinrh:opsx/crapload-decompose-pr2a

Conversation

@yvonnedevlinrh

@yvonnedevlinrh yvonnedevlinrh commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Phase 2a of CRAPload fragility reduction (#166). Decomposes BuildContractCoverageFunc — the single worst CRAPload offender (CRAP 506, complexity 22) — into 3 independently testable helpers. Eliminates 2 duplicated utility functions across goprovider and aireport, fixing a latent pkg.Errors validation bug in the process.

Changes

Decomposition

Extracted 3 helpers from BuildContractCoverageFunc (complexity 22 → 7):

Helper Purpose Testability
computeCoverageReason Pure function: report → ContractCoverageInfo with reason/confidence Direct struct-literal tests
buildEffectsSet Side effect analysis per package → map[string]bool DI via loadAndAnalyzeFn parameter
buildCoverageMap Quality pipeline per package → coverage map + degraded list DI via contractCoverageDeps

Deduplication

Duplicate Consolidated to Bug fixed
loadTestPackageForQuality (aireport) goprovider.LoadTestPackage (exported) Missing pkg.Errors validation in aireport copy
loadGazeConfigBestEffort (goprovider + aireport) config.LoadFromDir (new) N/A — identical copies

Files changed (6 source + 5 spec + 3 learnings)

internal/config/config.go              (+15)  — Added LoadFromDir
internal/config/config_test.go         (+68)  — 3 new tests
internal/provider/goprovider/contract.go           (+207/-174) — 3 helpers, export, dedup
internal/provider/goprovider/contract_internal_test.go (+293) — 13 new tests
internal/aireport/runner_steps.go      (-47)  — Removed duplicates, wired imports
internal/aireport/runner_steps_test.go (-40)  — Removed 4 duplicate tests

Metrics

Metric Before After
BuildContractCoverageFunc complexity 22 7
BuildContractCoverageFunc CRAP 506 < 30
Net new tests +12 (16 added, 4 removed)
Duplicate functions eliminated 2

Verification

go build ./...                              # PASS
go test -race -count=1 -short ./...         # 16/16 packages PASS
golangci-lint run                           # 0 issues
gocyclo contract.go | grep BuildContract    # 7 (target ≤ 8)

Spec artifacts

Full OpenSpec change at openspec/changes/crapload-decompose-pr2a/:

  • proposal.md — motivation + constitution alignment
  • design.md — 5 design decisions with alternatives rejected
  • specs/decompose-dedup.md — 5 ADDED, 3 MODIFIED, 2 REMOVED requirements with 17 Given/When/Then scenarios
  • tasks.md — 9 tasks in 4 groups, all checked off

Review council

  • Spec review: 5/5 APPROVE (Adversary, Architect, Guard, Testing, SRE)
  • Code review: 4/4 APPROVE (Architect, Testing, Guard, Adversary)

Extract 3 helpers from BuildContractCoverageFunc (complexity 22 → 7,
CRAP 506 → <30):
- computeCoverageReason: pure function for coverage reason computation
- buildEffectsSet: effects discovery with DI for testability
- buildCoverageMap: coverage pipeline with DI for testability

Deduplicate 2 cross-package utility functions:
- Export loadTestPackage as goprovider.LoadTestPackage (fixes missing
  pkg.Errors validation in aireport copy)
- Add config.LoadFromDir to replace both loadGazeConfigBestEffort copies

+16 new tests (13 helper tests + 3 config tests), -4 duplicate tests.
All new tests run without testing.Short() guard for CRAPload visibility.

Phase 2a of issue unbound-force#166.

Assisted-by: OpenCode (claude-opus-4-6)
Signed-off-by: Yvonne Devlin <ydevlin@redhat.com>
@yvonnedevlinrh
yvonnedevlinrh force-pushed the opsx/crapload-decompose-pr2a branch from 2025e14 to aec6554 Compare July 15, 2026 13:51

@jflowers jflowers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean refactoring PR that achieves its stated goal: decomposing the worst CRAPload offender (BuildContractCoverageFunc, CRAP 506 → <30) into three independently testable helpers, while consolidating two duplicated utility functions. The pkg.Errors validation bug fix in the aireport copy of loadTestPackage is a legitimate side-effect improvement.

  • All 5 CI checks pass (MegaLinter, unit/integration on Go 1.24+1.25, E2E on Go 1.24+1.25)
  • Spec artifacts comprehensive (proposal, design, specs with 17 Given/When/Then scenarios, 9 tasks all checked)
  • Constitution compliance verified (Accuracy, Testability improved; Minimal Assumptions, Actionable Output N/A)
  • No security concerns — no new inputs, no path construction changes, no dependency additions
  • Net +12 tests (16 added, 4 removed), all new helper tests run without testing.Short() guard

This review was generated by /review-pr (AI-assisted).

@yvonnedevlinrh
yvonnedevlinrh merged commit 185ca46 into unbound-force:main Jul 16, 2026
5 checks passed
@yvonnedevlinrh
yvonnedevlinrh deleted the opsx/crapload-decompose-pr2a branch July 16, 2026 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review 👀

Development

Successfully merging this pull request may close these issues.

2 participants