WIP: Add gomock support #1102

r4f4 · 2025-03-03T17:56:09Z

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Github / Jira issue:

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Code Improvements (Refactoring, Performance, CI upgrades, etc)
Internal repo assets (diagrams / docs on github repo)
This change requires a documentation update on openshift docs

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.

Expected Outcome

Please describe the outcome expected from the tests.

sherine-k · 2025-03-05T08:27:46Z

Hey @r4f4
Thanks for sharing about this library!
I think we are already using something similar: from testify/mock, here for example.

Interested to understand the pros and cons of these 2 libraries.

aguidirh

I really like this mock approach. Thanks @r4f4

it generates the code automatically for you
it is possible to reuse the mocked structs from a different package instead of duplicating code (as it is currently)

aguidirh · 2025-03-05T11:06:25Z

internal/pkg/additional/local_stored_collector_test.go

@@ -247,75 +247,3 @@ func TestAdditionalImageCollector(t *testing.T) {
 		assert.ElementsMatch(t, expected, res)
 	})
 }


It is really nice to reuse the mock from other packages instead of duplicating them.

+1

aguidirh · 2025-03-05T11:07:49Z

internal/pkg/additional/mock/interface_generated.go

+// Code generated by MockGen. DO NOT EDIT.
+// Source: ./interface.go
+//
+// Generated by this command:
+//
+//	mockgen -source=./interface.go -destination=./mock/interface_generated.go -package=mock
+//
+
+// Package mock is a generated GoMock package.


Generating the mock automatically based on the interface is really a big win.

+1

openshift-bot · 2025-07-30T01:00:19Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

aguidirh · 2025-07-30T08:35:35Z

@r4f4 I really like this approach and I think oc-mirror could benefit a lot from it.

Is there anything missing that we can do to have it merged ?

r4f4 · 2025-07-30T12:59:25Z

@r4f4 I really like this approach and I think oc-mirror could benefit a lot from it.

Is there anything missing that we can do to have it merged ?

Other than rebasing, I think it depends on what we want out of this PR. I just did it for one test file as an example but there still a lot of work to do it for the rest.

aguidirh · 2025-08-04T10:23:22Z

Got it @r4f4, so maybe we can get this merged and use it as an example to migrate the older tests (it does not need to be all at once) and when creating new ones.

What do you think about it @lmzuccarelli ?

lmzuccarelli · 2025-08-05T15:12:51Z

@r4f4 - could you rebase and once passing we can merge

openshift-bot · 2025-10-02T00:30:32Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

r4f4 · 2025-10-02T07:39:01Z

/remove-lifecycle rotten

openshift-ci · 2025-11-04T10:34:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: r4f4

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [r4f4]

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

This should make it easier to write dynamic mocks in unit tests.

openshift-ci · 2025-11-09T21:59:45Z

@r4f4: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/lint	`9e26d86`	link	true	`/test lint`
ci/prow/v1-sanity	`9e26d86`	link	true	`/test v1-sanity`
ci/prow/integration	`9e26d86`	link	true	`/test integration`
ci/prow/unit	`9e26d86`	link	true	`/test unit`
ci/prow/sanity	`9e26d86`	link	true	`/test sanity`
ci/prow/v1-unit	`9e26d86`	link	true	`/test v1-unit`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-merge-robot · 2025-11-09T21:59:52Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 3, 2025

openshift-ci bot requested review from lmzuccarelli and sherine-k March 3, 2025 17:56

r4f4 force-pushed the gomock-test branch from ad23b18 to c552870 Compare March 4, 2025 18:18

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 5, 2025

aguidirh reviewed Mar 5, 2025

View reviewed changes

openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 30, 2025

r4f4 force-pushed the gomock-test branch from c552870 to ceb8e1a Compare September 1, 2025 13:57

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 1, 2025

r4f4 force-pushed the gomock-test branch from ceb8e1a to f18710d Compare September 1, 2025 13:58

openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 2, 2025

openshift-ci bot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Oct 2, 2025

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 28, 2025

r4f4 force-pushed the gomock-test branch from f18710d to 139b483 Compare November 4, 2025 10:34

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 4, 2025

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 4, 2025

r4f4 force-pushed the gomock-test branch from 139b483 to 8318f0f Compare November 4, 2025 11:00

r4f4 added 3 commits November 4, 2025 14:06

v2: add gomock support

4e0022d

This should make it easier to write dynamic mocks in unit tests.

v2: pkg/additional: use generated mock

b9ce192

v2: pkg/manifest: use generated manifest mock

eddf487

r4f4 added 6 commits November 4, 2025 14:06

v2: pkg/mirror: use generated mirror mock

1852dd1

v2: pkg/release: use generated mock

e73d703

v2: pkg/imagebuilder: use generated mock

9b36389

v2: pkg/release: use generated client mock

7a2abef

v2: pkg/helm: use generated mocks

2a30ee5

v2: pkg/mirror: silence gosec lint issue

9e26d86

r4f4 force-pushed the gomock-test branch from 8318f0f to 9e26d86 Compare November 4, 2025 13:06

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 9, 2025

WIP: Add gomock support #1102

Are you sure you want to change the base?

WIP: Add gomock support #1102

Uh oh!

Conversation

r4f4 commented Mar 3, 2025

Description

Type of change

How Has This Been Tested?

Expected Outcome

Uh oh!

sherine-k commented Mar 5, 2025

Uh oh!

aguidirh left a comment

Choose a reason for hiding this comment

Uh oh!

aguidirh Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

aguidirh Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Jul 30, 2025

Uh oh!

aguidirh commented Jul 30, 2025

Uh oh!

r4f4 commented Jul 30, 2025

Uh oh!

aguidirh commented Aug 4, 2025

Uh oh!

lmzuccarelli commented Aug 5, 2025

Uh oh!

openshift-bot commented Oct 2, 2025

Uh oh!

r4f4 commented Oct 2, 2025

Uh oh!

openshift-ci bot commented Nov 4, 2025

Uh oh!

openshift-ci bot commented Nov 9, 2025

Uh oh!

openshift-merge-robot commented Nov 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants