test: move GetFreePort into test/framework/net#2122
Open
chethanuk wants to merge 2 commits into
Open
Conversation
The test/framework tree that llm-d#1188 introduces has no Makefile entry, so its packages would not run under make test-unit and would be absent from the coverage comparison. Wire the package set in before any helper moves land, so each follow-up move arrives with test execution and coverage already in place. The importas aliases for the planned framework packages are added in the same change: golangci-lint ignores entries for packages that do not exist yet, so the later moves do not each have to touch the lint config. Part of llm-d#1188 Signed-off-by: ChethanUK <chethanuk@outlook.com>
GetFreePort is a network helper, not a general test util. Moving it into the test/framework tree gives it an obvious home and keeps test/utils from growing into a grab bag. The function body is unchanged; the only edit is the package clause plus a license header and package comment on the new file. Part of llm-d#1188 Signed-off-by: ChethanUK <chethanuk@outlook.com>
github-actions
Bot
added
size/L
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the #1188 test-framework consolidation by relocating the GetFreePort helper into the structured test/framework/net package and updating callers to use the fwknet import alias (as enforced by golangci-lint). It also includes the test/framework unit-test wiring (from the #2121 base) so these helpers are executed under make test-unit and included in coverage output.
Changes:
- Move
GetFreePort()intotest/framework/net(package/headers/comment updates only) and keep its unit test alongside it. - Update callers to import
test/framework/netasfwknet. - Wire
./test/framework/...intomake test-unit/make test-coverageand add golangci-lintimportasaliases for planned framework packages.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
test/integration/epp/harness.go |
Switch GetFreePort calls to fwknet.GetFreePort() and update imports accordingly. |
test/framework/net/net.go |
Define test/framework/net package and host the moved GetFreePort() implementation. |
test/framework/net/net_test.go |
Update test package to net so it tests the relocated helper in-place. |
test/framework/framework_test.go |
Add a minimal test to ensure test/framework reports ok when executed under unit-test targets. |
test/framework/doc.go |
Document the intended layering/import rules for the test/framework tree. |
pkg/sidecar/proxy/data_parallel_test.go |
Switch GetFreePort usage to fwknet.GetFreePort() in sidecar proxy tests. |
Makefile |
Add framework_TEST_PACKAGES, run framework unit tests under test-unit, and make test-coverage depend on test-unit. |
.golangci.yml |
Add importas aliases for test/framework/* packages (including fwknet). |
shmuelk
reviewed
Jul 22, 2026
| @@ -81,6 +81,16 @@ linters: | |||
| alias: errcommon | |||
| - pkg: github.com/llm-d/llm-d-router/test/utils/igw | |||
Contributor
There was a problem hiding this comment.
Not related to this PR, but the path test/utils/igw no longer exists
Contributor
Author
There was a problem hiding this comment.
I didn't touch it since it wasn't part of this PR. I'll probably handle that separately in another PR?
Contributor
|
@chethanuk Please rebase and resolve the conflicts |
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
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:
Second of eight in the #1188 split. It follows the Makefile wiring PR that creates
test/frameworkand puts it undermake test-unit.This branch is based on #2121, so the diff currently includes that PR's commit as
well. Once #2121 merges this becomes a single commit touching only the
test/framework/netfiles.GetFreePortis a network helper. It sat intest/utils, which is how a packagebecomes a grab bag: nothing in the name says what belongs there, so everything does.
test/framework/netis where a port helper belongs.The move is verbatim. The function body is unchanged; the deltas are the package
clause, the llm-d license header and a package comment, none of which
test/utils/network.gocarried. Callers switch to thefwknetalias that.golangci.ymlalready declares.test/framework/netimports nothing frompkg/, so it stays an R1 leaf under thelayering rules in
test/framework/doc.go. The chain moves one leaf package per PR soeach is revertable on its own, with the harness last because it is the heaviest.
Test plan:
go buildandgo vet ./test/...clean at this commitgo test ./test/framework/...passes, including the moved testWhich issue(s) this PR fixes:
Part of #1188
Release note (write
NONEif no user-facing change):