test: move NewTestContext into test/framework/context#31
Conversation
NewTestContext is the last piece of test/utils that every plugin test depends on, so it belongs in the shared test/framework tree the issue defines rather than in a package named for what it is not. The function body is unchanged; the only edits are the package clause, a license header the old file lacked, and a package comment on the new file. The twenty callers switch to the fwkcontext alias. test/utils keeps its package comment, which moves to handle.go with the file that carried it deleted. 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 context utility by moving NewTestContext from test/utils to a new package under test/framework/context, updating all importing test files to use the new path. The review feedback suggests renaming the new package from context to fwkcontext to avoid shadowing the Go standard library's context package.
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.
| // Package context provides context helpers for tests. | ||
| package context |
There was a problem hiding this comment.
Naming the package context can easily lead to shadowing of the Go standard library's context package if imported without an alias.
By declaring the package as fwkcontext instead of context, the default package identifier when imported becomes fwkcontext. This completely eliminates the risk of shadowing the standard library context package, and also makes the explicit fwkcontext import alias optional in all importing files.
| // Package context provides context helpers for tests. | |
| package context | |
| // Package fwkcontext provides context helpers for tests. | |
| package fwkcontext |
| limitations under the License. | ||
| */ | ||
|
|
||
| package context |
There was a problem hiding this comment.
Update the package name to fwkcontext to match the package declaration in context.go.
| package context | |
| package fwkcontext |
1 participant
What type of PR is this?
/kind cleanup
/kind test
What this PR does / why we need it:
Third of eight in the llm-d#1188 split. It follows the
test/framework/netmove.NewTestContextbuilds acontext.Contextcarrying atesting.T-bound logger, andit was the last piece of
test/utilsthat every plugin test depends on. The body isunchanged. Twenty test files under
pkg/epp/framework/plugins/swap to thefwkcontextalias, andtest/utils/context.gois deleted.Two deviations from a pure move. The new file carries the llm-d Apache header, which
the old file had none of. And package
utils's package comment moves totest/utils/handle.go. That one is forced: the previous PR removednetwork.go, socontext.goheld the last copy, and dropping it makes revive'spackage-commentsrule fire.
Anyone adding a caller later must import this package as
fwkcontext. A bare importbinds the name
contextand shadows the stdlib package, andimportashasno-unaliased: false, so it will not catch that.Test plan:
go buildandgo vet ./test/...clean at this commitgo test ./test/framework/...passes, including the moved helper's testWhich issue(s) this PR fixes:
Part of llm-d#1188
Release note (write
NONEif no user-facing change):