Conversation
There was a problem hiding this comment.
Large PR Detected
This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.
How to unblock this PR:
Add a section to your PR description with the following format:
## Large PR Justification
[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformationAlternative:
Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.
See our Contributing Guidelines for more details.
This review will be automatically dismissed once you add the justification section.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 077e6ef27f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_v2_test.go
Outdated
Show resolved
Hide resolved
test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_v2_test.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR refactors the Session Management V2 test suite by elevating tests from the package-level unit integration layer (pkg/vmcp/session/) to a full end-to-end test suite running against a real Kubernetes cluster. It consolidates the previously separate virtualmcp_hmac_secret_test.go e2e file and the sessionv2_integration_test.go unit-integration file into a single, comprehensive new e2e file.
Changes:
- New file
test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_v2_test.go: Adds three GinkgoOrderedcontexts covering HMAC secret auto-management (creation, structure, env-var injection), verification that V2 is disabled by default, and session hijacking prevention with a real in-cluster JWT-issuing OIDC server. - Deleted
test/e2e/thv-operator/virtualmcp/virtualmcp_hmac_secret_test.go: Tests superseded and consolidated into the new file. - Deleted
pkg/vmcp/session/sessionv2_integration_test.go: Unit-level integration tests replaced by the new e2e suite.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_v2_test.go |
New comprehensive e2e test file consolidating HMAC secret lifecycle, disabled-V2 negative checks, and session hijacking prevention against a real cluster |
test/e2e/thv-operator/virtualmcp/virtualmcp_hmac_secret_test.go |
Deleted; contents superseded by the new consolidated e2e file |
pkg/vmcp/session/sessionv2_integration_test.go |
Deleted; in-process integration tests replaced by the new e2e file (with partial coverage gaps) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_v2_test.go
Show resolved
Hide resolved
test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_v2_test.go
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4069 +/- ##
==========================================
+ Coverage 68.48% 68.54% +0.05%
==========================================
Files 446 446
Lines 45573 45573
==========================================
+ Hits 31211 31237 +26
+ Misses 11948 11922 -26
Partials 2414 2414 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Large PR justification has been provided. Thank you!
|
✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_v2_test.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_v2_test.go
Outdated
Show resolved
Hide resolved
9f66b3f to
74945ce
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_v2_test.go
Outdated
Show resolved
Hide resolved
test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_v2_test.go
Outdated
Show resolved
Hide resolved
Replace the unit-level integration tests in pkg/vmcp/session with a comprehensive e2e test suite that validates Session Management V2 behaviour against a real Kubernetes cluster. Related-to: #3867
Replace the unit-level integration tests in pkg/vmcp/session with a comprehensive e2e test suite that validates Session Management V2 behaviour against a real Kubernetes cluster. Related-to: #3867 Co-authored-by: taskbot <taskbot@users.noreply.github.com>
Summary
Replace the unit-level integration tests in pkg/vmcp/session with a comprehensive e2e test suite that validates Session Management V2 behaviour against a real Kubernetes cluster.
Fixes #3867
Type of change
Test plan
task test)task test-e2e)task lint-fix)Changes
Does this introduce a user-facing change?
Special notes for reviewers
Large PR Justification
This is a refactor of testing sessions, that includes a comprehensive e2e test and removing redundant ones. Cannot be split.