test: move GAIE CRD builders into test/framework/gaie#33
Conversation
This completes the wrappers.go removal llm-d#1188 asks for. The GAIE builders are framework-generic: they touch the GAIE and apix API types and pkg/common/routing, never pkg/epp, so they belong beside the other shared test builders rather than under pkg/epp/util. With the last symbols gone, the package, its directory and its importas alias go too, and no config references the dead path afterwards. Part of llm-d#1188 Signed-off-by: ChethanUK <chethanuk@outlook.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the test builders by moving them from pkg/epp/util/testing to a new package test/framework/gaie (aliased as fwkgaie), and adds comprehensive unit tests for these builders in builders_test.go. Feedback is provided to leverage the newly introduced TargetPorts builder method in test/utils/server.go to simplify the test pool setup.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| pool := fwkgaie.MakeInferencePool(poolName).Namespace(namespace).ObjRef() | ||
| pool.Spec.TargetPorts = []v1.Port{{Number: v1.PortNumber(poolPort)}} |
There was a problem hiding this comment.
The newly introduced TargetPorts builder method can be used here to simplify the setup of the test InferencePool and leverage the fluent API design.
| pool := fwkgaie.MakeInferencePool(poolName).Namespace(namespace).ObjRef() | |
| pool.Spec.TargetPorts = []v1.Port{{Number: v1.PortNumber(poolPort)}} | |
| pool := fwkgaie.MakeInferencePool(poolName).Namespace(namespace).TargetPorts(poolPort).ObjRef() |
1 participant
What type of PR is this?
/kind cleanup
/kind test
What this PR does / why we need it:
Fifth of eight in the llm-d#1188 split. It follows the Pod builder move.
This is where
pkg/epp/util/testingloses its last symbols, so the package, thedirectory and the
testutilalias in.golangci.ymlall go with it.The GAIE builders go to
test/framework/gaie. They cannot share a package with theenvtest harness that arrives later in the chain, which is why
gaieandepp/harnessare separate rather than one
test/framework/epp.pkg/epp/datastore/datastore_test.gois an in-package test and it callsMakeInferencePool. The harness importscmd/epp/runner, which importspkg/epp/datastore, so a shared package would put that test in a cycle.This PR and the Pod builder PR both touch
wrappers.goandpkg/epp/controller/inferencepool_reconciler_test.go. The line changes are disjointbut the import blocks are not, so the order is strict.
Test plan:
go buildandgo vet ./test/...clean at this commit, so the delete left nodangling reference and no cycle
go test ./test/framework/...passes, builders at 100% coverageWhich issue(s) this PR fixes:
Part of llm-d#1188
Release note (write
NONEif no user-facing change):