Skip to content

fix(test): register field indexer in controller test suite (#351) - #352

Merged
cwiklik merged 1 commit into
mainfrom
fix/351-indexers-test-field-indexer
May 11, 2026
Merged

fix(test): register field indexer in controller test suite (#351)#352
cwiklik merged 1 commit into
mainfrom
fix/351-indexers-test-field-indexer

Conversation

@cwiklik

@cwiklik cwiklik commented May 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Register AgentCard .spec.targetRef.name field indexer in the controller test suite's BeforeSuite
  • Replace plain client.New() with a ctrl.Manager-backed client that has the indexer registered
  • Fixes the mapWorkloadToAgentCards test that fails with field label not supported: .spec.targetRef.name

Fixes #351

Root Cause

suite_test.go created a plain client.New(cfg, ...) without a ctrl.Manager. The mapWorkloadToAgentCards function queries with client.MatchingFields{".spec.targetRef.name": ...} which requires the field indexer registered by RegisterAgentCardTargetRefIndex(mgr). In production, the three controllers all register this in SetupWithManager, but the test suite skipped it.

Changes

File What
internal/controller/suite_test.go Create ctrl.Manager, register AgentCardTargetRefIndex, start manager, use mgr.GetClient()

Test Results

All 5 mapWorkloadToAgentCards specs pass (previously 1 failed):

Ran 5 of 163 Specs in 7.441 seconds
SUCCESS! -- 5 Passed | 0 Failed | 0 Pending | 158 Skipped

Assisted-By: Claude Code

The mapWorkloadToAgentCards test fails because suite_test.go creates a
plain client.New() without a ctrl.Manager, so the field indexer for
.spec.targetRef.name is never registered. Add a Manager in BeforeSuite
to register the AgentCard targetRef index and expose indexedClient for
tests that need field-indexed queries. Existing tests keep using the
plain k8sClient unchanged.

Fixes #351

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: cwiklik <cwiklikj@gmail.com>
@cwiklik
cwiklik force-pushed the fix/351-indexers-test-field-indexer branch from 3690c81 to addef85 Compare May 11, 2026 16:32

@huang195 huang195 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix addresses root cause of #351: test suite's client.New() bypassed ctrl.Manager, so the AgentCard.spec.targetRef.name field indexer was never registered, breaking mapWorkloadToAgentCards. This PR adds a Manager in BeforeSuite, registers the indexer, and exposes indexedClient for tests that need field-indexed queries. Existing tests stay on plain k8sClient — minimal blast radius. Dobra robota.

Areas reviewed: Go test setup
Commits: 1, signed-off ✓, Assisted-By footer ✓
CI: all 15 checks green (E2E, Integration, Unit, Lint, CodeQL, Trivy)

defer GinkgoRecover()
Expect(mgr.Start(ctx)).To(Succeed())
}()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking suggestion: after starting the manager in a goroutine, the cache-backed client from mgr.GetClient() isn't guaranteed ready. Consider adding Expect(mgr.GetCache().WaitForCacheSync(ctx)).To(BeTrue()) before assigning indexedClient — would prevent flakes if future tests query immediately after suite setup. CI is green today so not blocking.

@Ladas Ladas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@huang195 one question, how did you get your agent giving Dobra robota in review? ("good work" in Czech and I guess some Slavic languages?). I know some models/agents started to use Chinese for internal thinking, to have less token usage. But I wonder what is Slavic good for, that it got in? 😄

@cwiklik
cwiklik merged commit 6f0f224 into main May 11, 2026
15 checks passed
@pdettori
pdettori deleted the fix/351-indexers-test-field-indexer branch July 15, 2026 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

bug: indexers_test.go mapWorkloadToAgentCards fails — missing field indexer in test suite

4 participants