test: export EPP harness API and root discovery - #2611
Open
chethanuk wants to merge 4 commits into
Open
Conversation
The Pod builders in pkg/epp/util/testing are plain Kubernetes object helpers with nothing EPP-specific about them, and they are used from outside the EPP tree. Moving them to test/framework/k8s leaves wrappers.go holding only the GAIE wrappers it is named for. The moved code is unchanged apart from the package clause and a package comment, and keeps the copyright header it carried; the corev1 import drops from wrappers.go because the remainder never used it. doc.go and .golangci.yml drop their test/framework/context references. That package is out of scope for the issue, so the R1 rule and the importas alias named a package that will not exist. Part of llm-d#1188 Signed-off-by: ChethanUK <chethanuk@outlook.com>
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>
test/integration held a 686-line grab bag of ext_proc request builders and live executors that the integration suite imported as a package, which is the cyclic-dependency risk llm-d#1188 calls out: helpers living inside the tree that consumes them. The builders and executors are framework-generic, so they move to test/framework/epp and test/integration stops being importable. Five symbols are dropped rather than moved. ReqLLMWithStream, ReqLLMUnary and StartExtProcServer have no callers, and GenerateRequestWithStream is reachable only from ReqLLMWithStream, so moving them would ship zero-caller exports into the new package. SendRequest had a single caller, in the test that moves with it; that call site now uses the client Send and Recv pair SendRequest wrapped. util_test.go moves with the code it covers, which is why the root package goes away entirely rather than being left importable but empty. Its three tests carry over, with assertions unchanged. The stub ext_proc server they shared becomes an echo server that also backs new coverage for the moved builders and client helpers. The moved files keep the copyright header they carried. Part of llm-d#1188 Signed-off-by: ChethanUK <chethanuk@outlook.com>
The next step moves this harness into test/framework, and a move is only reviewable as a move if the surface it exposes is already settled. Export the entrypoint and accessors here, in place, so the semantic changes land in a diff the hermetic suite can be read against without a package move on top of them. Root discovery stops counting parent directories and asks the module system instead, because the count is exactly what a move would silently invalidate. Part of llm-d#1188 Signed-off-by: ChethanUK <chethanuk@outlook.com>
github-actions
Bot
added
size/XXL
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind cleanup
/kind test
What this PR does / why we need it:
Part of the #1188 split, following the ext_proc builders move. Nothing moves here.
The hermetic EPP harness gets the surface it will be moved with, and it stops
locating the repository by counting parent directories.
This branch is based on #2610, so the diff currently includes that PR's commit and
the two before it. Once those merge this becomes a single commit touching only
test/integration/epp.test/integration/epp/harness.gogainsRun(m *testing.M) int, which absorbs theTestMainbody fromhermetic_test.goin its existing order, along withrepoRootPathandloadBaseResourcesso the harness travels complete.TestMainbecomes a one-line delegation.
K8sClient,Config,LoggerandSchemewrap thesame package vars;
GRPCConnandSetPodMetricsexpose the harness fields the testsalready reach into;
runMode,standaloneStrategy,testPoolName,cleanMetric,metricReqTotalandmetricReadyPodsbecome their exported forms. Callers in thepackage follow.
Root discovery does change behavior.
TestMaintookruntime.Caller(0)and joinedthree
..onto the source file's directory for the repository root, then shelledgo list -mseparately for the gateway-api-inference-extension CRD directory. AmoduleDirhelper does both, resolving each module path throughgo list -m -f {{.Dir}}. The old walk was source-tree relative and the replacement ismodule relative; they agree with this repository as the main module and would differ
under a
go.workreplace.That count of
..is why the two halves are separated. The harness lands intest/framework/epp/harness, at a different depth, where a stale../../..stillcompiles and resolves to a wrong directory that envtest reports as a missing CRD path.
Discovery is fixed ahead of the move so the move itself stays mechanical, under the
rule that no PR mixes a move with a behavior change.
Test plan:
go vet ./...,go vet -tags integration_tests ./test/...andgofmt -l .clean at this commit
go test ./test/framework/...andgo test ./pkg/epp/server/...passgo list -m -f {{.Dir}}resolves both the llm-d module root andsigs.k8s.io/gateway-api-inference-extension, and the llm-d root holdsconfig/crd/basesWhich issue(s) this PR fixes:
Part of #1188
Release note (write
NONEif no user-facing change):