feat(tests): add integration tests for API endpoints #149#154
feat(tests): add integration tests for API endpoints #149#154Aharshi3614 wants to merge 7 commits into
Conversation
|
@Aharshi3614 is attempting to deploy a commit to the s3dfx-cyber's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR adds a Python integration test suite for TENET AI API endpoints and updates selected dependency versions in ChangesIntegration Test Suite for TENET AI API Endpoints
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@tests/integration/test_api_integration.py`:
- Around line 88-92: The test test_ingest_health_response_schema currently
asserts an exact version string ("0.1.0") which will break on normal version
bumps; change the assertion in test_ingest_health_response_schema (and the
similar assertions around lines 101-105) to validate the version more
flexibly—e.g., assert that data["version"] is a non-empty string or matches a
semantic version pattern, or compare it against the package/app runtime version
constant if available—so the test verifies format/presence rather than a
hard-coded value.
- Around line 150-167: The tests test_valid_key_accepted_on_ingest and
test_valid_key_accepted_on_analyzer currently assert a strict 200 response,
coupling auth checks to downstream availability; update both assertions to
accept successful auth responses even if downstream is degraded by asserting
r.status_code is in an allowed set (e.g., {200, 503}) or by asserting the
response is not an authentication failure (e.g., not 401/403), so the tests only
validate that VALID_HEADERS are accepted rather than the full service health.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eae01221-6742-43f1-86e5-c725a7210263
📒 Files selected for processing (1)
tests/integration/test_api_integration.py
|
@Aharshi3614 needs fixes |
|
Kindly make the fixes within 3-4 days or this pr will be closed as stale @Aharshi3614 |
Aharshi3614
force-pushed
the
feat/integration-tests-149
branch
3 times, most recently
from
dbf42b5 to
9bab66a
Compare
Removed unnecessary comment about numpy version.
|
@S3DFX-CYBER done! |
|
@Aharshi3614 merge conflicts |
|
@S3DFX-CYBER resolved! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/integration/test_api_integration.py (2)
1-25: 🎯 Functional Correctness | 🔵 TrivialNo tests for rate limiting despite it being an explicit objective of issue
#149.The linked issue lists "Test rate limiting behavior" as a required deliverable, but this suite has no test exercising rate-limit responses (e.g., repeated requests triggering
429). Want me to draft a rate-limit test class (e.g., burst requests against/v1/events/llmasserting a429after the configured threshold)?Also applies to: 657-703
🤖 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 `@tests/integration/test_api_integration.py` around lines 1 - 25, Add a rate-limiting test to the integration suite because the current tests cover auth and endpoint flow but not the required 429 behavior. Extend the existing test module with a dedicated rate-limit test/class (for example alongside TestAuthFlow) that targets a representative endpoint such as /v1/events/llm, sends a burst of repeated requests using the configured API_KEY, and asserts that once the limit is exceeded the response becomes 429. Keep the test aligned with the current environment-variable setup and reuse the existing request/session helpers in the integration file so it fits the rest of the suite.
556-577: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoffFixed
sleep(0.5)for eventual consistency is flaky and weakens assertions.Both
test_get_event_by_id_after_ingestandtest_get_event_isolation_by_orguse a hardcodedtime.sleep(0.5)then accept multiple outcomes (200, 503, 404) as "acceptable," which means a real regression (e.g., writes never persisted) can pass silently. Consider a short polling/retry loop with a timeout instead of a fixed sleep, and narrow the accepted outcomes.Also applies to: 588-607
🤖 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 `@tests/integration/test_api_integration.py` around lines 556 - 577, The event retrieval integration tests rely on a fixed `time.sleep(0.5)` and overly broad status acceptance, which can hide real persistence regressions. Update `test_get_event_by_id_after_ingest` and `test_get_event_isolation_by_org` to use a short polling/retry loop after the create request until the event becomes available or a timeout is reached, and then tighten the assertions so `get_r` only accepts the statuses that are genuinely expected for the behavior under test. Use the existing `event_id`, `create_r`, and `get_r` flow to keep the checks aligned with the current test structure.
🤖 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 `@tests/integration/test_api_integration.py`:
- Around line 588-607: The org isolation test is using an invalid API key
instead of a second valid org key, so it only checks auth rejection rather than
cross-tenant visibility. Update test_get_event_isolation_by_org in
test_api_integration.py to create or load a second valid API key for a different
org (preferably via a fixture or env var), then fetch the created event with
that key and assert it returns 404. Keep the existing event creation flow with
llm_payload and requests.get/post, but change the retrieval step to exercise
true org isolation.
---
Nitpick comments:
In `@tests/integration/test_api_integration.py`:
- Around line 1-25: Add a rate-limiting test to the integration suite because
the current tests cover auth and endpoint flow but not the required 429
behavior. Extend the existing test module with a dedicated rate-limit test/class
(for example alongside TestAuthFlow) that targets a representative endpoint such
as /v1/events/llm, sends a burst of repeated requests using the configured
API_KEY, and asserts that once the limit is exceeded the response becomes 429.
Keep the test aligned with the current environment-variable setup and reuse the
existing request/session helpers in the integration file so it fits the rest of
the suite.
- Around line 556-577: The event retrieval integration tests rely on a fixed
`time.sleep(0.5)` and overly broad status acceptance, which can hide real
persistence regressions. Update `test_get_event_by_id_after_ingest` and
`test_get_event_isolation_by_org` to use a short polling/retry loop after the
create request until the event becomes available or a timeout is reached, and
then tighten the assertions so `get_r` only accepts the statuses that are
genuinely expected for the behavior under test. Use the existing `event_id`,
`create_r`, and `get_r` flow to keep the checks aligned with the current test
structure.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b7b8c81-2453-4601-96bc-5686b1ebb15c
📒 Files selected for processing (2)
requirements.txttests/integration/test_api_integration.py
| def test_get_event_isolation_by_org(self): | ||
| """Events from one org should not be visible with a different API key.""" | ||
| create_r = requests.post( | ||
| f"{INGEST_URL}/v1/events/llm", | ||
| headers=VALID_HEADERS, | ||
| json=llm_payload("Org isolation test"), | ||
| timeout=5, | ||
| ) | ||
| if create_r.status_code != 200: | ||
| pytest.skip("Could not create event for isolation test") | ||
|
|
||
| event_id = create_r.json()["event_id"] | ||
| time.sleep(0.5) | ||
|
|
||
| get_r = requests.get( | ||
| f"{INGEST_URL}/v1/events/{event_id}", | ||
| headers=INVALID_HEADERS, | ||
| timeout=5, | ||
| ) | ||
| assert get_r.status_code in (401, 404) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Org isolation test doesn't actually test org isolation.
test_get_event_isolation_by_org creates an event with VALID_HEADERS then retrieves it with INVALID_HEADERS (a garbage key), asserting 401 or 404. This only re-verifies invalid-key rejection (already covered by TestAuthFlow) — it doesn't confirm that a different valid org's API key can't see another org's event, which is the actual isolation guarantee the test name implies.
Consider adding a second valid API key for a different org (e.g., via env var or fixture) and asserting a 404 (not found for that org) when accessed with it, to truly exercise cross-tenant isolation.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 589-594: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.post(
f"{INGEST_URL}/v1/events/llm",
headers=VALID_HEADERS,
json=llm_payload("Org isolation test"),
timeout=5,
)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
[warning] 601-605: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.get(
f"{INGEST_URL}/v1/events/{event_id}",
headers=INVALID_HEADERS,
timeout=5,
)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
🤖 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 `@tests/integration/test_api_integration.py` around lines 588 - 607, The org
isolation test is using an invalid API key instead of a second valid org key, so
it only checks auth rejection rather than cross-tenant visibility. Update
test_get_event_isolation_by_org in test_api_integration.py to create or load a
second valid API key for a different org (preferably via a fixture or env var),
then fetch the created event with that key and assert it returns 404. Keep the
existing event creation flow with llm_payload and requests.get/post, but change
the retrieval step to exercise true org isolation.
2 participants
Description
Implements comprehensive integration tests for all TENET AI API endpoints as specified in issue #149.
Tests cover the full request/response cycle for both the Ingest (port 8000) and Analyzer (port 8100) services across 10 test classes:
TestHealthEndpoints- health checks for both servicesTestAuthFlow- authentication and authorization edge casesTestIngestEndpoint- full ingest happy pathTestIngestValidation- input validation and edge casesTestAnalyzerEndpoint- analyzer verdict, schema, error handlingTestEventRetrieval- event retrieval by ID, pagination, org isolationTestStatsAndCircuit- stats endpoint and circuit breaker statusTestThreatDetection- 12 parametrized accuracy test casesTestPerformance- response time and batch request testsTestErrorHandling- 404, 405, 422 error code verificationRelated Issue
Fixes #149
Type of Change
How Has This Been Tested?
Ran locally with both services running via uvicorn:
pytest tests/integration/test_api_integration.py -v
Result: 80 passed in 200s
Checklist
Summary by cubic
Adds a comprehensive integration test suite for the Ingest and Analyzer APIs to validate end-to-end flows, auth, validation, events, stats/circuit, audit export, performance, and threat detection. Tests handle degraded Redis states and enforce consistent JSON errors, aligning with issue #149.
New Features
tests/integration/test_api_integration.pycovering health schemas, auth, ingest/analyze validation and verdicts, events (list/get with pagination and org isolation), stats, circuit status, audit export, and JSON error codes; auto-skips if services are unreachable.risk_score/confidencein [0,1],threat_typein {prompt_injection,jailbreak,data_extraction}; accepts optionalcontext.event_id; detects injection/jailbreaks and flags data extraction. Includes 12 threat cases, performance checks (<3s), and a 10-request batch; configurable viaINGEST_URL,ANALYZER_URL,API_KEY.Dependencies
numpyto>=2.4.6, addsprometheus-client>=0.21.0, and alignspython-dateutilto>=2.8.2; cleans uprequirements.txtcomments.Written for commit a9d2ea3. Summary will update on new commits.
Summary by CodeRabbit