RHAIENG-4029: add tier-2 integration test for agentic RAG agent#101
Merged
Conversation
Add integration test that builds, deploys, and health-checks the LangGraph Agentic RAG agent on OpenShift. Unlike tier-1 agents, this test validates RAG-specific env vars (EMBEDDING_MODEL, VECTOR_STORE_ID, EMBEDDING_DIMENSION, VECTOR_STORE_PROVIDER) are present before attempting deployment, and writes them to .env so the Helm deploy target passes them through to the pod. Also fixes the `make test` target to ignore the integration directory, preventing import errors when running unit tests locally. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds integration testing for agentic_rag: a Makefile test-integration target, pytest fixtures that build/deploy the agent and yield its route, an integration health-check test, and updates to redact VECTOR_STORE_ID in test logs. ChangesDeployment Integration Testing
Sequence DiagramsequenceDiagram
participant Fixture as deployed_agent fixture
participant Make as Makefile (build/deploy)
participant Route as Route discovery
participant Test as test_health_endpoint
participant Agent as Deployed agent service
Fixture->>Fixture: write .env with RAG config
Fixture->>Make: make build-openshift
Fixture->>Make: make deploy
Fixture->>Route: get_route(agent_name)
Route-->>Fixture: agent URL
Fixture->>Test: yield agent URL
Test->>Agent: GET /health
Agent-->>Test: {status, agent_initialized}
Test->>Test: assert status==healthy AND agent_initialized==True
Fixture->>Make: make undeploy (cleanup)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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. ✨ 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 |
Clarify why agent.yaml-optional vars are required in the integration test, and extend _REDACT_PATTERNS to cover VECTOR_STORE_ID which may contain connection strings with credentials. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@agents/langgraph/agentic_rag/tests/integration/test_deployment.py`:
- Line 49: The EMBEDDING_DIMENSION value taken from os.environ.get in
test_deployment.py is written to the .env as-is and may be non-integer; update
the test to validate and normalize it by attempting to parse
int(os.environ.get('EMBEDDING_DIMENSION', '768')) (catch ValueError) and fall
back to 768 (or raise a clear assertion if you prefer strictness), then write
the normalized integer back into the .env string (e.g., use the validated int
converted to str for the f"EMBEDDING_DIMENSION=..." line); reference the
EMBEDDING_DIMENSION environment access and the code that constructs the
f"EMBEDDING_DIMENSION={...}" string to locate where to add the try/except and
normalization.
In `@tests/integration/utils.py`:
- Around line 28-29: The existing redaction regexes
re.compile(r"(VECTOR_STORE_ID=)\S+") and
re.compile(r"(--set\s+env\.VECTOR_STORE_ID=)\S+") only match unquoted values;
add the quoted-value counterparts to cover Helm/Makefile usage (e.g., add
re.compile(r'(VECTOR_STORE_ID=")[^"]+"') and
re.compile(r'(--set\s+env\\.VECTOR_STORE_ID=")[^"]+"') ) so deployment logs with
VECTOR_STORE_ID="$${VECTOR_STORE_ID}" are properly redacted.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 012f483d-5c4f-4d08-ac08-ca398e410942
📒 Files selected for processing (5)
agents/langgraph/agentic_rag/Makefileagents/langgraph/agentic_rag/tests/integration/__init__.pyagents/langgraph/agentic_rag/tests/integration/conftest.pyagents/langgraph/agentic_rag/tests/integration/test_deployment.pytests/integration/utils.py
Keep main's updated test target (--ignore=tests/behavioral, PYTEST_ARGS) alongside the new test-integration target. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ub.com:red-hat-data-services/agentic-starter-kits into feat/RHAIENG-4029-agentic-rag-integration-test
Validate EMBEDDING_DIMENSION as int before writing to .env, and add quoted-format redaction pattern for VECTOR_STORE_ID to cover Helm --set usage in deployment logs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tarun-etikala
previously approved these changes
May 18, 2026
andrewdonheiser
requested changes
May 18, 2026
- Move helpers (_write_env_file, INTERNAL_REGISTRY) and fixtures (agent_dir, agent_name, deployed_agent) to conftest.py so the test file contains only test functions (andrewdonheiser) - Add broad `except Exception` fallback in deployed_agent fixture so unexpected errors surface clearly in CI (andrewdonheiser) - Wrap EMBEDDING_DIMENSION int() cast with try/except and pytest.fail for a clear error message (tarun-etikala) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@agents/langgraph/agentic_rag/tests/integration/conftest.py`:
- Line 7: The current direct imports cluster_auth and repo_root into this module
namespace cause a name collision with fixture arguments and trigger Ruff F811;
replace the direct-from import with a module-level import (e.g., import
integration.conftest as integration_conftest # noqa: F401) or alias the
imported module so you do not introduce cluster_auth or repo_root symbols into
this file’s global namespace, leaving the fixture names available for use in
test function signatures (referencing the module import rather than the bare
symbols prevents the lint error).
- Around line 70-87: The fixture currently yields inside the try/except that
catches MakeTargetError/RouteNotFoundError and a broad Exception, so assertion
failures after yield get swallowed; to fix, wrap the deployment steps
(logger.info("Building image..." ), run_make("build-openshift"),
run_make("deploy"), get_route(...), setting deployed/route_url) in an inner try
that only catches MakeTargetError and RouteNotFoundError and calls pytest.fail
with the exception, keep teardown in a finally, then move the yield route_url
outside that inner try/except so test failures aren’t caught by the deployment
error handlers; reference run_make, get_route, deployed, route_url,
MakeTargetError, RouteNotFoundError when making the change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 9dd4a0bb-96fe-4081-a305-32bc830b7425
📒 Files selected for processing (4)
agents/langgraph/agentic_rag/Makefileagents/langgraph/agentic_rag/tests/integration/conftest.pyagents/langgraph/agentic_rag/tests/integration/test_deployment.pytests/integration/utils.py
✅ Files skipped from review due to trivial changes (1)
- tests/integration/utils.py
🚧 Files skipped from review as they are similar to previous changes (1)
- agents/langgraph/agentic_rag/Makefile
andrewdonheiser
previously approved these changes
May 19, 2026
- Use module import for shared fixtures to avoid F811 lint collision with fixture argument names - Move yield outside exception handlers so test assertion failures aren't masked as "deployment errors" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@agents/langgraph/agentic_rag/tests/integration/conftest.py`:
- Around line 1-14: The import block in conftest.py is mis-sorted: third-party
import "pytest" must be separated from local package imports
"integration.conftest" and "integration.utils" by a blank line to satisfy lint
rules; edit the top of the file so there is a blank line between "import pytest"
and the subsequent "import integration.conftest # noqa: F401" / "from
integration.utils import (... get_route, load_agent_name, run_make, ...)" lines,
preserving the existing names and comments.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 8bb6474f-dcb4-4dc0-b8a5-e9e8917a6db8
📒 Files selected for processing (1)
agents/langgraph/agentic_rag/tests/integration/conftest.py
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
3 participants
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.
Ticket
RHAIENG-4029
Summary
EMBEDDING_MODEL,VECTOR_STORE_ID,EMBEDDING_DIMENSION,VECTOR_STORE_PROVIDER) before deploying, unlike tier-1 tests that only needBASE_URL/MODEL_IDtest-integrationMakefile target and fixestesttarget to ignore the integration directoryTest plan
cd agents/langgraph/agentic_rag && make test— unit tests pass (ignoring integration dir)make test-integration— integration test runs on demo cluster with required env vars exportedmake undeploy) runs even on test failure (fixture finalizer pattern)