test: deduplicate GetFreePort helpers#17
Conversation
Two copies of a free-port helper existed with divergent signatures: test/integration/util.go returned (int, error) on IPv4 127.0.0.1:0, and test/utils/network.go returned (string, error) on dual-stack :0. Consolidate to a single canonical GetFreePort (int, error) in the lightweight test/utils package (the sidecar test tree cannot import the heavy test/integration tree). Delete the integration copy, migrate the three harness call sites, and drop the now-redundant strconv.Atoi in the sidecar data-parallel test. Refs: llm-d#1188 Signed-off-by: ChethanUK <chethanuk@outlook.com>
🤖 CodeAnt AI — Review Status
Updated in place by CodeAnt AI · last 5 reviews |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe shared test port helper now returns an integer bound to IPv4 localhost. Integration EPP setup uses it for server ports, the old integration helper is removed, and the data-parallel test adjusts to integer port handling. ChangesTest port allocation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request consolidates the GetFreePort utility function into test/utils/network.go, refactoring it to return an integer port instead of a string and forcing IPv4 localhost. This change is propagated across tests in pkg/sidecar/proxy/data_parallel_test.go and test/integration/epp/harness.go, and the duplicate implementation in test/integration/util.go is removed. A new test file test/utils/network_test.go is also added. The reviewer pointed out that the test case "returns a distinct bindable port on repeat call" does not actually verify distinct ports because it uses the same logic as the first test case in the table-driven loop, and suggested rewriting the test to explicitly call GetFreePort twice while keeping the first port bound.
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.
The table-driven TestGetFreePort had two named cases sharing one body, so the second case never verified that repeat calls return distinct ports. Allocate N ports while holding each listener open, which forces the OS to hand out a fresh port per call, and assert range, bindability, and distinctness together. Signed-off-by: ChethanUK <chethanuk@outlook.com>
User description
What type of PR is this?
/kind cleanup
/kind test
What this PR does / why we need it:
Two copies of a "give me a free TCP port" test helper existed with divergent
signatures:
test/integration/util.go—GetFreePort() (int, error), IPv4127.0.0.1:0test/utils/network.go—GetFreePort() (string, error), dual-stack:0This PR collapses them into a single canonical
GetFreePort() (int, error)living in the lightweight
test/utilspackage. The direction is forced byimports:
pkg/sidecar/proxy(viatest/utils) must not pull in the heavytest/integrationEPP/k8s dependency tree, so the shared helper belongs intest/utils.Changes:
test/utils/network.go: canonicalGetFreePort() (int, error), IPv4127.0.0.1:0(keeps the anti-dual-stack-flake behavior).test/integration/util.go: delete the duplicate helper; drop the now-unusederrorsimport.test/integration/epp/harness.go: migrate the 3 call sites indefaultEppServerOptionstotestutils.GetFreePort().pkg/sidecar/proxy/data_parallel_test.go: theintreturn drops astrconv.Atoiround-trip.test/utils/network_test.go: table-driven test that the helper returns anin-range, actually-bindable port.
Pure test-infra refactor, no product code. Net negative diff.
Which issue(s) this PR fixes:
Fixes llm-d#1188
Release note:
CodeAnt-AI Description
Use one shared helper for finding a free test port
What Changed
Impact
✅ Fewer flaky test port conflicts✅ Consistent test setup across suites✅ Shorter test startup path💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Bug Fixes
Tests