From 7b4cd206de9fa804866ea5d2364ae49f4563bc23 Mon Sep 17 00:00:00 2001
From: grokspawn <jordan@nimblewidget.com>
Date: Mon, 2 Nov 2026 13:19:29 -0600
Subject: [PATCH] Make TestRegistryProviderList order-independent

refreshCache fetches each package and its per-channel bundles from its
own goroutine, but the OneCatalog/ManyPackages/OneMissingBundle/Elided
case programs its counterfeiter stubs with call-index-bound returns
(GetPackageReturnsOnCall / GetBundleForChannelReturnsOnCall).

Counterfeiter stubs do not inspect request arguments: ReturnsOnCall(i)
hands response i to whichever goroutine happens to issue the i-th
call. When the two package goroutines interleave between their
GetPackage and GetBundleForChannel calls, the responses cross-wire:
the goroutine processing the "has-bundle" package receives the
index-0 "no bundle found" error and elides it, while the goroutine
processing "no-bundle" receives the index-1 good bundle and caches a
manifest for it. The test then fails with the wrong package elided
(expected [has-bundle], actual [no-bundle]).

The provider is correct -- a real registry answers per request; only
the test assumed a serialization that refreshCache does not provide.
The race detector does not flag this because counterfeiter fakes are
internally mutex-synchronized: it is a semantic ordering dependency,
not a data race.

Replace the index-bound stubs with argument-aware stub functions that
key responses off the requested package and channel, keeping the
scenario semantics (no-bundle's channel errors, has-bundle resolves)
while being independent of goroutine scheduling.

Observed failing run (go test -race -count=1, downstream OpenShift CI):
https://prow.ci.openshift.org/view/gs/test-platform-results-public/pr-logs/pull/openshift_operator-framework-olm/1396/pull-ci-openshift-operator-framework-olm-main-unit-olm/2116233294835093504

Signed-off-by: grokspawn <jordan@nimblewidget.com>
---
 pkg/package-server/provider/registry_test.go | 36 ++++++++++++++++----
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/pkg/package-server/provider/registry_test.go b/pkg/package-server/provider/registry_test.go
index 342bc82793..adea7e87c3 100644
--- a/pkg/package-server/provider/registry_test.go
+++ b/pkg/package-server/provider/registry_test.go
@@ -1490,7 +1490,14 @@ func TestRegistryProviderList(t *testing.T) {
 					clientFake := &fakes.FakeRegistryClient{}
 					clientFake.ListBundlesReturns(listBundlesFake, nil)
 					clientFake.ListPackagesReturns(listFake, nil)
-					clientFake.GetPackageReturnsOnCall(0, &api.Package{
+					// refreshCache fetches each package and its per-channel
+					// bundles from its own goroutine, so stub responses must
+					// be matched to the request rather than the call order:
+					// counterfeiter's ReturnsOnCall hands a response to
+					// whichever goroutine issues the call, so interleaved
+					// scheduling cross-wires order-bound responses between
+					// packages.
+					noBundlePkg := &api.Package{
 						Name: "no-bundle",
 						Channels: []*api.Channel{
 							{
@@ -1499,8 +1506,8 @@ func TestRegistryProviderList(t *testing.T) {
 							},
 						},
 						DefaultChannelName: "alpha",
-					}, nil)
-					clientFake.GetPackageReturnsOnCall(1, &api.Package{
+					}
+					hasBundlePkg := &api.Package{
 						Name: "has-bundle",
 						Channels: []*api.Channel{
 							{
@@ -1509,9 +1516,8 @@ func TestRegistryProviderList(t *testing.T) {
 							},
 						},
 						DefaultChannelName: "alpha",
-					}, nil)
-					clientFake.GetBundleForChannelReturnsOnCall(0, nil, fmt.Errorf("no bundle found"))
-					clientFake.GetBundleForChannelReturnsOnCall(1, &api.Bundle{
+					}
+					hasBundle := &api.Bundle{
 						CsvName:     "etcdoperator.v0.9.2",
 						PackageName: "has-bundle",
 						ChannelName: "alpha",
@@ -1522,7 +1528,23 @@ func TestRegistryProviderList(t *testing.T) {
 							etcdUpgradesCRDJSON,
 							etcdRestoresCRDJSON,
 						},
-					}, nil)
+					}
+					clientFake.GetPackageStub = func(_ context.Context, req *api.GetPackageRequest, _ ...grpc.CallOption) (*api.Package, error) {
+						switch req.GetName() {
+						case noBundlePkg.Name:
+							return noBundlePkg, nil
+						case hasBundlePkg.Name:
+							return hasBundlePkg, nil
+						default:
+							return nil, fmt.Errorf("unexpected package %q", req.GetName())
+						}
+					}
+					clientFake.GetBundleForChannelStub = func(_ context.Context, req *api.GetBundleInChannelRequest, _ ...grpc.CallOption) (*api.Bundle, error) {
+						if req.GetPkgName() == noBundlePkg.Name {
+							return nil, fmt.Errorf("no bundle found")
+						}
+						return hasBundle, nil
+					}
 
 					return &registryClient{clientFake, catsrc, nil}
 				}(),