Skip to content

CNF-23384: mockgen deprecated: use uber-go/mock instead#443

Open
sebrandon1 wants to merge 1 commit into
openshift:masterfrom
sebrandon1:mockgen_deprecation
Open

CNF-23384: mockgen deprecated: use uber-go/mock instead#443
sebrandon1 wants to merge 1 commit into
openshift:masterfrom
sebrandon1:mockgen_deprecation

Conversation

@sebrandon1

@sebrandon1 sebrandon1 commented Nov 13, 2025

Copy link
Copy Markdown
Member

https://github.com/golang/mock is marked as archived as of June 27, 2023. They recommend using go.uber.org/mock.

This PR attempts to change the dependency to one that is maintained.

The changes to the generated/ files are because of the new library.

Tracking issue: redhat-best-practices-for-k8s/telco-bot#45

Summary by CodeRabbit

  • Chores

    • Updated mock dependency to a newer version.
  • Style

    • Improved formatting and alignment across multiple test files.
  • Tests

    • Added comprehensive test helper infrastructure for end-to-end testing, including resource creation utilities, credential management, and operator bootstrap capabilities.

@openshift-ci
openshift-ci Bot requested review from anispate and clcollins November 13, 2025 20:06
@codecov-commenter

codecov-commenter commented Nov 13, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 38.70968% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.60%. Comparing base (f2f967e) to head (7fda219).

Files with missing lines Patch % Lines
pkg/util/test/generated/mocks/client/cr-client.go 42.55% 22 Missing and 5 partials ⚠️
.../util/test/generated/mocks/client/status-writer.go 26.66% 10 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #443   +/-   ##
=======================================
  Coverage   57.60%   57.60%           
=======================================
  Files          31       31           
  Lines        2913     2913           
=======================================
  Hits         1678     1678           
  Misses       1110     1110           
  Partials      125      125           
Files with missing lines Coverage Δ
pkg/util/reconcile/reconcile_wrapper.go 100.00% <ø> (ø)
...til/test/generated/mocks/controllers/interfaces.go 75.52% <ø> (ø)
pkg/util/test/generated/mocks/reconcile/common.go 100.00% <ø> (ø)
.../util/test/generated/mocks/client/status-writer.go 38.09% <26.66%> (ø)
pkg/util/test/generated/mocks/client/cr-client.go 45.71% <42.55%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@clcollins

Copy link
Copy Markdown
Member

/lgtm

@clcollins

Copy link
Copy Markdown
Member

/hold

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 21, 2025
@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Nov 21, 2025
@openshift-ci

openshift-ci Bot commented Nov 21, 2025

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: clcollins, sebrandon1

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:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 21, 2025
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 22, 2025
@openshift-merge-robot

Copy link
Copy Markdown
Contributor

PR needs rebase.

Details

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-bot

Copy link
Copy Markdown

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

@openshift-ci openshift-ci Bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 21, 2026
@openshift-bot

Copy link
Copy Markdown

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

@openshift-ci 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 Mar 23, 2026
@clcollins

clcollins commented Mar 30, 2026

Copy link
Copy Markdown
Member

/lifecycle frozen

@sebrandon1
sebrandon1 force-pushed the mockgen_deprecation branch from 4c02cf4 to 54d460c Compare April 6, 2026 16:09
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 6, 2026
@openshift-ci

openshift-ci Bot commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 6, 2026
@coderabbitai

coderabbitai Bot commented Apr 6, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds RHOBS Synthetic Monitoring E2E helper functions to route_monitor_operator_tests.go: Kubernetes namespace/HostedControlPlane creation with cleanup, HCP status update with conflict retry, CRD installation, operator restart, and OIDC credential loading/writing via an exported OIDCCredentials type and a route-monitor-operator-config ConfigMap. Also bumps go.uber.org/mock from v0.4.0 to v0.6.0 and applies minor formatting fixes across several files.

Changes

RHOBS E2E Test Helpers

Layer / File(s) Summary
HCP, namespace, CRD, and operator bootstrap helpers
test/e2e/route_monitor_operator_tests.go
Adds helpers for creating Namespaces and HostedControlPlane objects with deferred cleanup (including RHOBS probe deletion polling), osde2e environment detection, HCP status update to Available with optimistic-concurrency retry, MC-style HCP spec construction with labels/annotations and osde2e testing annotation, Route Monitor Operator restart via controller-manager pod deletion, and embedded CRD installation that parses YAML into unstructured objects and aggregates Forbidden/missing-CRD failures.
OIDC credential loading and ConfigMap writing
test/e2e/route_monitor_operator_tests.go
Introduces the exported OIDCCredentials struct, an environment-variable credential reader that preserves errCredentialsNotAvailable, a ConfigMap loader with absent-or-incomplete fallback, and a ConfigMap writer that creates or updates route-monitor-operator-config with OIDC/Probe API fields and skip-infrastructure-health-check (defaulting to "false").

Formatting and Dependency Cleanup

Layer / File(s) Summary
Dependency bump and formatting fixes
go.mod, pkg/util/reconcile/reconcile_wrapper.go, pkg/reconcile/common_test.go, test/e2e/agent_manager.go, test/e2e/process_manager.go, test/e2e/helpers.go, test/e2e/probe_deletion_retry_test.go
Bumps go.uber.org/mock from v0.4.0 to v0.6.0; reorders time import; inserts a blank line in an import block; removes a whitespace-only line; reformats ProcessManager struct fields and labels map entries; rewrites updateProbeStatus and probe deletion retry inline comments.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • geowa4

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 errors, 3 warnings)

Check name Status Explanation Resolution
Stable And Deterministic Test Names ❌ Error Multiple Ginkgo test step names in test/e2e/route_monitor_operator_tests.go contain dynamic values that change between runs: timestamps in clusterID/hcpName (Unix timestamps), randomly-generated na... Remove dynamic values from By() step titles: use static descriptive names like "creating test namespace" instead of "creating MC-style namespace: test-namespace--"; keep dynamic values in test bodies only.
No-Sensitive-Data-In-Logs ❌ Error PR introduces logging of unfiltered HTTP response bodies from OIDC token endpoint and RHOBS API in test/e2e files, risking exposure of sensitive error details. Remove or sanitize response body logging in error messages (e.g., lines 856, 897 in route_monitor_operator_tests.go and 109, 142, 211, 238, 335 in helpers.go).
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Microshift Test Compatibility ⚠️ Warning New RHOBS test suite adds Ginkgo tests using hypershift.openshift.io/v1beta1 HostedControlPlane API, which is not available on MicroShift, without [apigroup:hypershift.openshift.io] tag or [Skipped... Add [apigroup:hypershift.openshift.io] tag to "RHOBS Synthetic Monitoring" Describe block name, or add [Skipped:MicroShift] label, to prevent test execution on MicroShift CI.
Ipv6 And Disconnected Network Test Compatibility ⚠️ Warning New RHOBS Synthetic Monitoring e2e tests make external API calls to RHOBS and OIDC services via HTTP, requiring public internet connectivity unavailable in disconnected IPv6-only CI environments. Add [Skipped:Disconnected] tag to test names that require external RHOBS/OIDC API connectivity, or wrap the RHOBS suite in a skip conditional for disconnected environments using environment variable checks.
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: updating the mocking library dependency from deprecated golang/mock to maintained go.uber.org/mock.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Test Structure And Quality ✅ Passed Ginkgo tests follow quality requirements: (1) Each It block tests one behavior (e.g., "creates probe for public HostedControlPlane"). (2) Setup uses BeforeAll for suite initialization and DeferClea...
Single Node Openshift (Sno) Test Compatibility ✅ Passed The new test file test/e2e/route_monitor_operator_tests.go is an osde2e-specific integration test (//go:build osde2e), not a standard SNO e2e test. Tests do not assume multiple nodes and make no mu...
Topology-Aware Scheduling Compatibility ✅ Passed This PR is a dependency version update (go.uber.org/mock v0.4.0→v0.6.0) with generated mock file changes and test code updates. No deployment manifests, operator controllers, or scheduling constrai...
Ote Binary Stdout Contract ✅ Passed No OTE Binary Stdout Contract violations found. PR contains no process-level stdout writes (init/main/suite setup). Test code and stderr-safe patterns verified across all modified files.
No-Weak-Crypto ✅ Passed No weak crypto algorithms (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custom crypto implementations, or unsafe token/secret comparisons detected in any changed files.
Container-Privileges ✅ Passed PR contains no privileged container settings. Container SecurityContext in test/e2e/route_monitor_operator_tests.go explicitly sets AllowPrivilegeEscalation to false, enforces non-root execution, d...
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@sebrandon1
sebrandon1 force-pushed the mockgen_deprecation branch from 54d460c to 181742f Compare April 30, 2026 19:01
@sebrandon1 sebrandon1 changed the title mockgen deprecated: use uber-go/mock instead CNF-23384: mockgen deprecated: use uber-go/mock instead Apr 30, 2026
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 30, 2026
@openshift-ci-robot

openshift-ci-robot commented Apr 30, 2026

Copy link
Copy Markdown

@sebrandon1: This pull request references CNF-23384 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

https://github.com/golang/mock is marked as archived as of June 27, 2023. They recommend using go.uber.org/mock.

This PR attempts to change the dependency to one that is maintained.

The changes to the generated/ files are because of the new library.

Tracking issue: redhat-best-practices-for-k8s/telco-bot#45

Summary by CodeRabbit

  • Chores
  • Updated mock dependency to the latest version for improved tooling functionality and compatibility.

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 openshift-eng/jira-lifecycle-plugin repository.

@sebrandon1
sebrandon1 force-pushed the mockgen_deprecation branch from 181742f to 7ea089e Compare May 7, 2026 20:59
@sebrandon1

Copy link
Copy Markdown
Member Author

/retest

@sebrandon1
sebrandon1 force-pushed the mockgen_deprecation branch from 7ea089e to 6c39da1 Compare May 20, 2026 15:25
@sebrandon1
sebrandon1 force-pushed the mockgen_deprecation branch from 6c39da1 to 4177dc0 Compare May 29, 2026 15:08
@sebrandon1
sebrandon1 force-pushed the mockgen_deprecation branch from 4177dc0 to 95c0b4f Compare June 18, 2026 17:31
@sebrandon1
sebrandon1 force-pushed the mockgen_deprecation branch 2 times, most recently from 8319546 to 8619038 Compare July 8, 2026 20:41
@sebrandon1
sebrandon1 force-pushed the mockgen_deprecation branch from 8619038 to 7fda219 Compare July 20, 2026 18:55
@openshift-ci

openshift-ci Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

@sebrandon1: The following test 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/generate 7fda219 link true /test generate

Full PR test history. Your PR dashboard.

Details

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants