Add integration tests for MCPRemoteProxy resource references#3608
Open
ChrisJBurns wants to merge 6 commits intomainfrom
Open
Add integration tests for MCPRemoteProxy resource references#3608ChrisJBurns wants to merge 6 commits intomainfrom
ChrisJBurns wants to merge 6 commits intomainfrom
Conversation
ChrisJBurns
requested review from
JAORMX,
jerm-dro,
jhrozek and
yrobla
as code owners
ChrisJBurns
force-pushed
the
mcp-remote-proxy-int-tests
branch
from
1f6e931 to
fa890fc
Compare
github-actions
bot
added
size/L
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3608 +/- ##
==========================================
- Coverage 65.99% 65.94% -0.05%
==========================================
Files 415 415
Lines 41192 41194 +2
==========================================
- Hits 27186 27167 -19
- Misses 11911 11930 +19
- Partials 2095 2097 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
github-actions
bot
added
size/L
Signed-off-by: Chris Burns <29541485+ChrisJBurns@users.noreply.github.com>
- Add WaitForPhase helper to reduce test duplication - Add WaitForStatusMessage helper for status message assertions - Update ExternalAuthConfigRef and ToolConfigRef tests to use helpers - Improve "not found" assertions to verify both config name and "not found" substring for stronger validation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The MCPGroup controller requires a field index for MCPServer.Spec.GroupRef to list servers belonging to a group. This was missing from the mcp-remote-proxy test suite, causing failures when tests created MCPGroups. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- ToolConfigRef "not found" test: Adjusted assertion to check only Phase=Failed since the controller doesn't set status.Message for ToolConfig validation failures (only logs the error). This is tracked in issue #3607. - GroupRef "not Ready" test: Removed test that tried to manually set MCPGroup status to Pending, which doesn't work because the MCPGroup controller immediately reconciles empty groups to Ready state. - GroupRef transition test: Changed from testing "not ready -> ready" to testing "not found -> found and ready", which avoids the race condition with the MCPGroup controller. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update MCPRemoteProxy integration tests to use the structured status conditions added in #3634 for ToolConfigRef and ExternalAuthConfigRef validation: - ToolConfigRef "not found": Assert ToolConfigValidated=False with ToolConfigNotFound reason and config name in message - ToolConfigRef "success": Assert ToolConfigValidated=True with ToolConfigValid reason - ExternalAuthConfigRef "not found": Assert AuthConfigured=False with AuthInvalid reason - ExternalAuthConfigRef "success": Assert ExternalAuthConfigValidated=True with ExternalAuthConfigValid reason Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ChrisJBurns
force-pushed
the
mcp-remote-proxy-int-tests
branch
from
fd171b0 to
78860ef
Compare
github-actions
bot
added
size/L
Extract repeated patterns into reusable helpers to reduce duplication and align with sibling test suites (MCPServer, MCPRegistry): - Add name derivation helpers (ServiceName, ConfigMapName, ServiceAccountName) - Create k8s_helpers.go with WaitFor* resource and config hash helpers - Add verifyRemoteProxyOwnerReference for owner ref verification - Add WaitForCondition/WaitForConditionReason to status helpers - Fix status helper constructor to accept existing proxy helper - Fix noisy By() calls inside Eventually loops - Consolidate two Describe blocks into one with shared setup/teardown - Fix weak AuthConfigured test to assert condition does not exist - Add owner reference verification to Deployment, Service, ConfigMap, and RBAC resource tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
github-actions
bot
added
size/XL
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
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.
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.
Summary
WaitForPhase)WithExternalAuthConfigRef,WithToolConfigRef,WithGroupRef)Test Coverage Added
ExternalAuthConfigRef Integration (3 tests)
AuthConfigured=FalsewithAuthInvalidreasonExternalAuthConfigValidated=TruewithExternalAuthConfigValidreasonToolConfigRef Integration (3 tests)
ToolConfigValidated=FalsewithToolConfigNotFoundreason and config name in messageToolConfigValidated=TruewithToolConfigValidreasonGroupRef Integration (4 tests)
GroupRefValidatedcondition toFalsewhen MCPGroup does not existGroupRefValidatedcondition toTruewhen MCPGroup exists and is ReadyGroupRefValidatedcondition when no GroupRef is specifiedGroupRefValidatedcondition fromFalse(NotFound) toTruewhen MCPGroup is createdTest plan
go vetandgofmtRelated Issues
Closes #3362 (Integration with Other Resources section)
Builds on #3634 (structured status conditions for ToolConfigRef and ExternalAuthConfigRef validation)
🤖 Generated with Claude Code