Skip to content

Commit 15c8df6

Browse files
tmshortclaude
andcommitted
tests: address CodeRabbit review comments from downstream PR
- Rename local variable `tag` to `bundleTag` in catalog.Build to avoid shadowing the method's `tag` parameter - Log and skip (instead of silently dropping) invalid LargeCRD field counts in the bundle option parser - Look up channels by name in TestCatalog_FBCGeneration instead of assuming insertion order - Add a semaphore (8 slots) to ScenarioCleanup deletion loop to bound concurrent kubectl calls - Clarify README that ClusterObjectSet deletion is conditional on the BoxcutterRuntime feature gate - Add missing `text` language specifier to fenced code block in e2e-isolation design doc - Remove extra blank line in setup.sh Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
1 parent a47a0b4 commit 15c8df6

7 files changed

Lines changed: 30 additions & 13 deletions

File tree

docs/designs/testing/2026-04-13-e2e-isolation/design.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Each scenario dynamically builds and pushes its own bundle and catalog OCI image
1212
parameterized by scenario ID. All cluster-scoped resource names include the scenario ID, making
1313
conflicts structurally impossible.
1414

15-
```
15+
```text
1616
Scenario starts
1717
-> Generate parameterized bundle manifests (CRD names, deployments, etc. include scenario ID)
1818
-> Build + push bundle OCI images to e2e registry via go-containerregistry

test/e2e/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ Each scenario runs in its own namespace with unique resource names, ensuring com
292292

293293
The `ScenarioCleanup` hook ensures all resources are deleted after each scenario:
294294

295-
- Deletes ClusterExtensions and ClusterObjectSets
295+
- Deletes ClusterExtensions and (when BoxcutterRuntime gate is enabled) ClusterObjectSets
296296
- Deletes ClusterCatalogs
297297
- Deletes namespaces
298298
- Deletes added resources

test/e2e/steps/hooks.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,11 +222,15 @@ func ScenarioCleanup(ctx context.Context, _ *godog.Scenario, err error) (context
222222
}
223223
forDeletion = append(forDeletion, resource{name: sc.namespace, kind: "namespace"})
224224

225+
const maxConcurrentDeletes = 8
226+
sem := make(chan struct{}, maxConcurrentDeletes)
225227
var wg sync.WaitGroup
226228
for _, r := range forDeletion {
227229
wg.Add(1)
228230
go func(res resource) {
229231
defer wg.Done()
232+
sem <- struct{}{}
233+
defer func() { <-sem }()
230234
args := []string{"delete", res.kind, res.name, "--ignore-not-found=true"}
231235
if res.namespace != "" {
232236
args = append(args, "-n", res.namespace)

test/e2e/steps/steps.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1683,9 +1683,11 @@ func parseContents(contents string) []catalog.BundleOption {
16831683
// LargeCRD(250)
16841684
countStr := part[len("LargeCRD(") : len(part)-1]
16851685
count, err := strconv.Atoi(countStr)
1686-
if err == nil {
1687-
opts = append(opts, catalog.WithLargeCRD(count))
1686+
if err != nil {
1687+
logger.V(1).Info("ignoring invalid LargeCRD field count", "value", countStr, "error", err)
1688+
continue
16881689
}
1690+
opts = append(opts, catalog.WithLargeCRD(count))
16891691
case strings.HasPrefix(part, "ClusterRegistry(") && strings.HasSuffix(part, ")"):
16901692
// ClusterRegistry(mirrored-registry.operator-controller-e2e.svc.cluster.local:5000)
16911693
host := part[len("ClusterRegistry(") : len(part)-1]

test/extension-developer-e2e/setup.sh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ reg_bundle_img="${cluster_registry_host}/bundles/registry-v1/registry-bundle:v0.
9292
# because docker push goes through the Docker daemon which
9393
# may be in a different network context (e.g. colima VM).
9494

95-
9695
###############################
9796
# Create the FBC that contains
9897
# the registry+v1 extensions

test/internal/catalog/catalog.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,9 @@ func (c *Catalog) Build(ctx context.Context, tag, localRegistry, clusterRegistry
154154
return nil, fmt.Errorf("failed to set bundle labels for %s:%s: %w", paramPkgName, bd.version, err)
155155
}
156156

157-
tag := fmt.Sprintf("%s/bundles/%s:v%s", localRegistry, paramPkgName, bd.version)
158-
if err := crane.Push(img, tag, crane.Insecure, crane.WithContext(ctx)); err != nil {
159-
return nil, fmt.Errorf("failed to push bundle image %s: %w", tag, err)
157+
bundleTag := fmt.Sprintf("%s/bundles/%s:v%s", localRegistry, paramPkgName, bd.version)
158+
if err := crane.Push(img, bundleTag, crane.Insecure, crane.WithContext(ctx)); err != nil {
159+
return nil, fmt.Errorf("failed to push bundle image %s: %w", bundleTag, err)
160160
}
161161
bundleClusterRegistry := clusterRegistry
162162
if spec.clusterRegistryOverride != "" {

test/internal/catalog/catalog_test.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,17 +134,29 @@ func TestCatalog_FBCGeneration(t *testing.T) {
134134
t.Errorf("expected 2 channels, got %d", len(pkg.channels))
135135
}
136136

137-
// Verify alpha channel has 1 entry
138-
alpha := pkg.channels[0]
139-
if alpha.name != "alpha" {
140-
t.Errorf("expected channel 'alpha', got %q", alpha.name)
137+
// Verify channels by name rather than by index so the test is robust to ordering changes.
138+
var alpha, beta *channelDef
139+
for i := range pkg.channels {
140+
switch pkg.channels[i].name {
141+
case "alpha":
142+
alpha = &pkg.channels[i]
143+
case "beta":
144+
beta = &pkg.channels[i]
145+
}
146+
}
147+
if alpha == nil {
148+
t.Fatal("alpha channel not found")
149+
}
150+
if beta == nil {
151+
t.Fatal("beta channel not found")
141152
}
153+
154+
// Verify alpha channel has 1 entry
142155
if len(alpha.entries) != 1 {
143156
t.Errorf("expected 1 alpha entry, got %d", len(alpha.entries))
144157
}
145158

146159
// Verify beta channel has replaces edge
147-
beta := pkg.channels[1]
148160
if len(beta.entries) != 2 {
149161
t.Fatalf("expected 2 beta entries, got %d", len(beta.entries))
150162
}

0 commit comments

Comments
 (0)