OLS-3219 - creating script for running e2e tests in a real cluster with real sandbox#170
OLS-3219 - creating script for running e2e tests in a real cluster with real sandbox#170JoaoFula wants to merge 8 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds ChangesE2E Real-Provider Testing Infrastructure
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
scripts/e2e-cluster.sh (1)
72-72: 🏗️ Heavy liftConsolidate fixture ownership to one layer.
create_provider_fixtureshere andcreateRealProviderFixturesintest/e2e/helpers_test.goboth create provider resources. Keeping both paths risks drift and cross-layer contract breaks. Prefer one source of truth and let the other layer only pass configuration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/e2e-cluster.sh` at line 72, The call to create_provider_fixtures in the bash script duplicates the functionality of createRealProviderFixtures in the Go test file, creating two separate sources of truth for provider resource creation that risk diverging. Consolidate fixture ownership to a single layer by removing one of these creation paths and ensuring the other layer (bash script or Go tests) handles all provider fixture creation, while the alternate layer focuses only on passing necessary configuration parameters to the primary creation mechanism.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/e2e-cluster.sh`:
- Around line 81-83: The E2E_POLL_TIMEOUT variable is hardcoded to "20m" in the
provider runs section, preventing caller environment overrides. Change the
hardcoded assignment `E2E_POLL_TIMEOUT="20m"` to use the same pattern as
VERTEX_PROJECT_ID and VERTEX_REGION by using parameter expansion syntax to allow
environment variable override with a default fallback. Apply this same fix to
all occurrences mentioned in lines 90-92 where similar hardcoding exists.
In `@scripts/e2e-lib.sh`:
- Around line 98-103: The cleanup_operator() function runs `make undeploy` from
the caller's current working directory without ensuring it's in the repository
root, which can cause the command to fail silently due to the error suppression.
To fix this, modify the function to either change directory to the repository
root before executing the make undeploy command, or use an absolute path to
invoke make from the correct location. Ensure the directory change happens
before the `make undeploy ignore-not-found=true` call and that it properly
handles the case where the script is invoked from outside the repo root so the
operator is actually cleaned up instead of silently failing.
In `@test/e2e/helpers_test.go`:
- Around line 300-301: The error returned by c.Create(ctx, stagingNS) is being
ignored using the blank identifier, which masks potential cluster, RBAC, or API
failures and makes test failures harder to diagnose. Instead of ignoring the
error, explicitly check the error returned by c.Create and fail the test fast if
it is not nil, except when the error is an AlreadyExists error which can be
safely ignored. This ensures that any real failures during namespace creation
are caught immediately rather than causing cryptic failures later in the test.
---
Nitpick comments:
In `@scripts/e2e-cluster.sh`:
- Line 72: The call to create_provider_fixtures in the bash script duplicates
the functionality of createRealProviderFixtures in the Go test file, creating
two separate sources of truth for provider resource creation that risk
diverging. Consolidate fixture ownership to a single layer by removing one of
these creation paths and ensuring the other layer (bash script or Go tests)
handles all provider fixture creation, while the alternate layer focuses only on
passing necessary configuration parameters to the primary creation mechanism.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 98c0d768-76ff-49d4-baf8-ed401bbeb42f
📒 Files selected for processing (3)
scripts/e2e-cluster.shscripts/e2e-lib.shtest/e2e/helpers_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
addressing comments
JoaoFula
force-pushed
the
implement-ols-3219
branch
from
4fbff65 to
67bb6fb
Compare
|
@JoaoFula: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
Summary
Add
scripts/e2e-cluster.shandscripts/e2e-lib.sh— the entry point for running the existing e2e test suite against real LLM providers (Claude, Gemini, OpenAI) instead of the mock agent.Rather than creating a separate product-e2e test suite, this parameterizes the existing e2e tests via env vars. When
E2E_PROVIDERis set,helpers_test.gocreates real provider fixtures (Secret, LLMProvider, Agent CRs with actual credentials). When unset, the existing mock agent path is unchanged — PR/integration CI runs are unaffected.What's included
scripts/e2e-cluster.sh— orchestrator that parses args, checks prerequisites (oc,make,go,jq), deploys the operator idempotently viainstall-operator.sh, then loops through each provider: creates fixtures, runsmake test-e2ewith provider env vars, collects artifacts, cleans up, and reports per-provider pass/failscripts/e2e-lib.sh— reusable functions for operator lifecycle, fixture CRUD, artifact collection, and loggingtest/e2e/helpers_test.go— newcreateRealProviderFixtures()path that readsE2E_PROVIDER,E2E_MODEL,E2E_PROVIDER_KEY_PATH(and Vertex-specific vars) to create properly-typed CRs with real credentials;pollTimeoutis now overridable viaE2E_POLL_TIMEOUTenv varUsage
Provider mapping
claudeclaude-sonnet-4-6geminigemini-2.5-flashopenaigpt-4.1-miniJira: https://redhat.atlassian.net/browse/OLS-3219