:seedling: Replace hand-written test mocks with gomock-generated mocks by pedjak · Pull Request #2775 · operator-framework/operator-controller

pedjak · 2026-06-19T15:44:48Z

Description

Replace all hand-written mock structs across the test suite with go.uber.org/mock (gomock) generated mocks. This eliminates duplicated mocks, establishes a single consistent mocking pattern, and ensures mocks stay in sync with interface changes automatically via make generate-mocks.

Hand-written test code shrank by 373 lines (1901 added, 2274 removed). The generated mocks add 3580 lines, but those are auto-generated and never manually maintained.

Infrastructure

Add mockgen via bingo, consistent with how all other build tools are managed
Add generate-mocks Makefile target wired into generate/verify so CI catches stale mocks
//go:generate directives live in internal/testutil/mock/generate.go for all exported interfaces, and inline at interface definitions for unexported ones (trackingCache, sourcerer)
Generated mocks for unexported interfaces use _test.go files to keep go.uber.org/mock out of the production binary

Mock migration

~40 hand-written mock structs removed across 20+ test files
19 generated mock packages created under internal/testutil/mock/
2 in-package generated mocks for unexported interfaces (source mode)
Mocks cover external interfaces (controller-runtime client, helm-operator-plugins, k8s apiextensions, logr, boxcutter machinery, k8s rbac validation, net/http) and internal interfaces (storage, applier, authorization, controllers, contentmanager, config, catalogd server/service, rukpak render)

Naming cleanup

Rename MockPuller/MockCache → FakePuller/FakeCache in image/fakes.go to clarify they are test fakes (preconfigured return values), not mocks (interaction verification)
Remove duplicated FakeCertProvider from render/fake.go and rukpak/util/testing/testing.go, replaced by generated MockCertificateProvider
FakeBundleSource (a function type, not an interface) retained as-is

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

netlify · 2026-06-19T15:44:53Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`39c3498`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/6a37d59b3c9dcc0008241268
😎 Deploy Preview	https://deploy-preview-2775--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot

Pull request overview

This PR standardizes the repository’s test mocking approach by replacing hand-written mock implementations with go.uber.org/mock (gomock) generated mocks, and adds tooling + make targets to keep mocks regenerated and in sync with interface changes.

Changes:

Added mock generation infrastructure (bingo-managed mockgen, make generate-mocks, centralized go:generate directives).
Introduced generated gomock packages under internal/testutil/mock/** and migrated many tests to use them.
Kept/clarified true “test fakes” (preconfigured return values) vs interaction-verifying mocks (e.g., rename to FakePuller / FakeCache).

Reviewed changes

Copilot reviewed 56 out of 59 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
Makefile	Adds `mockgen` symlink + `generate-mocks` target and wires it into `make generate`.
.bingo/Variables.mk	Installs `mockgen` via bingo and exposes `MOCKGEN`.
.bingo/variables.env	Adds `MOCKGEN` env var for bingo tooling.
.bingo/mockgen.mod	Adds bingo module for `mockgen`.
.bingo/mockgen.sum	Adds checksums for `mockgen` bingo module.
go.mod	Adds `go.uber.org/mock` dependency (and tidies indirect deps).
go.sum	Adds `go.uber.org/mock` sums.
AGENTS.md	Documents gomock as the standard for interface mocks + regeneration workflow.
internal/testutil/mock/generate.go	Centralizes `//go:generate mockgen ...` directives for exported interfaces.
internal/testutil/mock/helmclient/interfaces.go	Adds composite interface used for generating a combined helm client mock.
internal/testutil/mock/helmclient/mock_actionclient.go	Generated mocks for helm-operator-plugins action interfaces/getter.
internal/testutil/mock/helmclient/mock_composite.go	Generated mock for composite helm interface.
internal/testutil/mock/ctrlclient/mock_client.go	Generated mocks for controller-runtime `client.Client` and sub-writers.
internal/testutil/mock/crdclient/mock_crdinterface.go	Generated mock for CRD typed client interface.
internal/testutil/mock/logrsink/mock_logsink.go	Generated mock for `logr.LogSink`.
internal/testutil/mock/httputil/mock_roundtripper.go	Generated mock for `net/http.RoundTripper`.
internal/testutil/mock/rbac/mock_rulesresolver.go	Generated mock for Kubernetes RBAC rule resolver.
internal/testutil/mock/machinery/mock_results.go	Generated mocks for boxcutter machinery result interfaces.
internal/testutil/mock/storage/mock_instance.go	Generated mock for catalogd storage `Instance`.
internal/testutil/mock/catalogdserver/mock_catalogstore.go	Generated mock for catalogd server `CatalogStore`.
internal/testutil/mock/catalogdservice/mock_graphqlservice.go	Generated mock for catalogd `GraphQLService`.
internal/testutil/mock/catalogclient/mock_cache.go	Generated mock for catalogmetadata client cache interface.
internal/testutil/mock/config/mock_schemaprovider.go	Generated mock for config `SchemaProvider`.
internal/testutil/mock/contentmanager/mock_contentmanager.go	Generated mock for contentmanager `Manager`.
internal/testutil/mock/cmcache/mock_cache.go	Generated mocks for contentmanager cache interfaces.
internal/testutil/mock/controllers/mock_controllers.go	Generated mocks for operator-controller controller interfaces.
internal/testutil/mock/authorization/mock_authorization.go	Generated mock for authorization `PreAuthorizer`.
internal/testutil/mock/applier/mock_applier.go	Generated mocks for applier interfaces.
internal/testutil/mock/render/mock_certprovider.go	Generated mock for rukpak render `CertificateProvider`.
internal/shared/util/image/fakes.go	Renames `Mock` to `Fake` for non-interaction fakes.
internal/shared/util/image/pull_test.go	Updates tests to use renamed fakes.
internal/shared/util/featuregates/logging_test.go	Migrates to gomock-based `LogSink` mock.
internal/operator-controller/rukpak/render/fake.go	Removes duplicated handwritten cert provider fake (replaced by generated mock).
internal/operator-controller/rukpak/util/testing/testing.go	Removes duplicated handwritten cert provider fake from shared test util.
internal/operator-controller/rukpak/render/render_test.go	Switches to generated `MockCertificateProvider`.
internal/operator-controller/rukpak/render/certprovider_test.go	Switches to generated `MockCertificateProvider`.
internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go	Switches to generated `MockCertificateProvider` with expectations.
internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go	Replaces handwritten CRD getter with generated mock.
internal/operator-controller/controllers/clusterobjectset_controller.go	Adds `go:generate` for unexported interface mock (source mode).
internal/operator-controller/controllers/mock_trackingcache_gen_test.go	Adds generated in-package mock for `trackingCache` (test-only).
internal/operator-controller/controllers/resolve_ref_test.go	Refactors tests to use gomock-based helpers/mocks.
internal/operator-controller/controllers/clusterobjectset_controller_internal_test.go	Replaces handwritten tracking cache with gomock-generated mock.
internal/operator-controller/controllers/suite_test.go	Replaces handwritten controller deps with gomock-generated mocks/helpers.
internal/operator-controller/controllers/clustercatalog_controller_test.go	Replaces handwritten catalog cache/populator mocks with gomock mocks.
internal/operator-controller/contentmanager/cache/cache.go	Adds `go:generate` for sourcerer/watch/source mocks (test-only).
internal/operator-controller/contentmanager/cache/mock_sourcerer_gen_test.go	Adds generated in-package mock for unexported interfaces (test-only).
internal/operator-controller/contentmanager/cache/cache_test.go	Migrates tests to gomock-generated mocks.
internal/operator-controller/config/config_test.go	Migrates schema-provider tests to gomock-generated mock.
internal/operator-controller/catalogmetadata/client/client_test.go	Migrates cache and HTTP transport fakes to gomock mocks.
internal/operator-controller/authorization/rbac_test.go	Migrates RBAC resolver test double to gomock mock.
internal/operator-controller/applier/provider_test.go	Migrates manifest provider / certificate provider fakes to generated mocks.
internal/operator-controller/action/helm_test.go	Migrates from testify/mock to gomock-generated helm client mocks.
internal/catalogd/serverutil/serverutil_test.go	Migrates storage instance test double to generated gomock mock.
internal/catalogd/server/handlers_test.go	Moves to `server_test` package and uses generated mocks.
internal/catalogd/controllers/core/clustercatalog_controller_test.go	Migrates storage instance test double to generated gomock mock.
internal/operator-controller/applier/helm_test.go	Migrates applier Helm tests to generated mocks and helper builders.
internal/operator-controller/applier/boxcutter_test.go	Migrates boxcutter tests to generated mocks and helper builders.

Files not reviewed (2)

internal/operator-controller/contentmanager/cache/mock_sourcerer_gen_test.go: Generated file
internal/operator-controller/controllers/mock_trackingcache_gen_test.go: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov · 2026-06-20T10:18:30Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.46%. Comparing base (a12398a) to head (39c3498).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2775      +/-   ##
==========================================
+ Coverage   66.78%   70.46%   +3.67%     
==========================================
  Files         149      143       -6     
  Lines       11386    10617     -769     
==========================================
- Hits         7604     7481     -123     
+ Misses       3222     2577     -645     
+ Partials      560      559       -1

Flag	Coverage Δ
e2e	`35.14% <0.00%> (+0.06%)`	⬆️
experimental-e2e	`52.58% <0.00%> (+0.03%)`	⬆️
unit	`59.49% <100.00%> (+7.37%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Copilot reviewed 57 out of 60 changed files in this pull request and generated 3 comments.

Files not reviewed (2)

internal/operator-controller/contentmanager/cache/mock_sourcerer_gen_test.go: Generated file
internal/operator-controller/controllers/mock_trackingcache_gen_test.go: Generated file

Copilot

Pull request overview

Copilot reviewed 57 out of 60 changed files in this pull request and generated 1 comment.

Files not reviewed (2)

internal/operator-controller/contentmanager/cache/mock_sourcerer_gen_test.go: Generated file
internal/operator-controller/controllers/mock_trackingcache_gen_test.go: Generated file

Copilot

Pull request overview

Copilot reviewed 56 out of 59 changed files in this pull request and generated no new comments.

Files not reviewed (2)

internal/operator-controller/contentmanager/cache/mock_sourcerer_gen_test.go: Generated file
internal/operator-controller/controllers/mock_trackingcache_gen_test.go: Generated file

Copilot

Pull request overview

Copilot reviewed 57 out of 60 changed files in this pull request and generated 1 comment.

Files not reviewed (2)

internal/operator-controller/contentmanager/cache/mock_sourcerer_gen_test.go: Generated file
internal/operator-controller/controllers/mock_trackingcache_gen_test.go: Generated file

Introduce go.uber.org/mock (gomock) with mockgen code generation to replace all hand-written mock structs across the test suite. This eliminates duplicated mocks, establishes a single consistent mocking pattern, and ensures mocks stay in sync with interface changes automatically via make generate-mocks. Infrastructure: - Add mockgen via bingo with unversioned symlink for go generate - Add generate-mocks Makefile target (go generate ./...) - Wire into existing generate/verify targets for CI enforcement - Centralized //go:generate directives in internal/testutil/mock/ for exported interfaces (external and internal) - Source-mode directives at interface definitions for unexported interfaces (trackingCache, sourcerer), output to _test.go files to keep gomock out of the production binary - Mock helpers accept shared *gomock.Controller to avoid controller proliferation in nested mock scenarios Rename MockPuller/MockCache to FakePuller/FakeCache in image/fakes.go to clarify they are test fakes (preconfigured return values), not mocks (interaction verification). Remove duplicated FakeCertProvider from render/fake.go and rukpak/util/testing.go, replaced by generated MockCertificateProvider. FakeBundleSource (a function type, not an interface) is retained as-is. Co-Authored-By: Claude <noreply@anthropic.com>

perdasilva · 2026-06-22T09:21:17Z

Nice one! Thank you!

perdasilva · 2026-06-22T09:21:21Z

/approve

dtfranz · 2026-06-22T12:51:15Z

/lgtm

rashmigottipati

/lgtm

openshift-ci · 2026-06-22T12:52:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: perdasilva, rashmigottipati

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [perdasilva]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot AI review requested due to automatic review settings June 19, 2026 15:44

openshift-ci Bot requested review from ankitathomas and rashmigottipati June 19, 2026 15:44

Copilot started reviewing on behalf of pedjak June 19, 2026 15:45 View session

pedjak requested review from perdasilva and removed request for ankitathomas June 19, 2026 15:46

Copilot AI reviewed Jun 19, 2026

View reviewed changes

pedjak force-pushed the gomock-migration branch from e694bca to 40982d5 Compare June 20, 2026 09:53

pedjak force-pushed the gomock-migration branch from 40982d5 to c54cf6e Compare June 20, 2026 10:50

Copilot AI review requested due to automatic review settings June 20, 2026 10:50

Copilot started reviewing on behalf of pedjak June 20, 2026 10:51 View session

Copilot AI reviewed Jun 20, 2026

View reviewed changes

Comment thread internal/shared/util/featuregates/logging_test.go

Comment thread internal/operator-controller/action/helm_test.go

Comment thread internal/operator-controller/controllers/clusterobjectset_controller_test.go

pedjak force-pushed the gomock-migration branch 2 times, most recently from 32a4020 to 3f7ec8e Compare June 20, 2026 12:09

Copilot AI review requested due to automatic review settings June 20, 2026 12:09

Copilot started reviewing on behalf of pedjak June 20, 2026 12:09 View session

Copilot AI reviewed Jun 20, 2026

View reviewed changes

Comment thread internal/catalogd/controllers/core/clustercatalog_controller_test.go

pedjak force-pushed the gomock-migration branch from 3f7ec8e to a3c372d Compare June 20, 2026 15:07

Copilot AI review requested due to automatic review settings June 20, 2026 16:30

pedjak force-pushed the gomock-migration branch from a3c372d to 80ac4fd Compare June 20, 2026 16:30

Copilot started reviewing on behalf of pedjak June 20, 2026 16:31 View session

Copilot AI reviewed Jun 20, 2026

View reviewed changes

pedjak force-pushed the gomock-migration branch from 80ac4fd to 56414b4 Compare June 20, 2026 22:03

Copilot AI review requested due to automatic review settings June 20, 2026 22:05

pedjak force-pushed the gomock-migration branch from 56414b4 to a2bb6ba Compare June 20, 2026 22:05

Copilot started reviewing on behalf of pedjak June 20, 2026 22:06 View session

Copilot AI reviewed Jun 20, 2026

View reviewed changes

Comment thread .bingo/Variables.mk

pedjak force-pushed the gomock-migration branch from a2bb6ba to 39c3498 Compare June 21, 2026 12:14

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 22, 2026

openshift-ci Bot assigned dtfranz Jun 22, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 22, 2026

rashmigottipati approved these changes Jun 22, 2026

View reviewed changes

openshift-ci Bot assigned rashmigottipati Jun 22, 2026

openshift-merge-bot Bot merged commit e4879a3 into operator-framework:main Jun 22, 2026
26 checks passed

Uh oh!

Conversation

pedjak commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Infrastructure

Mock migration

Naming cleanup

Reviewer Checklist

Uh oh!

netlify Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

perdasilva commented Jun 22, 2026

Uh oh!

perdasilva commented Jun 22, 2026

Uh oh!

dtfranz commented Jun 22, 2026

Uh oh!

rashmigottipati left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Jun 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pedjak commented Jun 19, 2026 •

edited

Loading

netlify Bot commented Jun 19, 2026 •

edited

Loading

codecov Bot commented Jun 20, 2026 •

edited

Loading