Implement API test suite with 100% coverage#1442
Implement API test suite with 100% coverage#1442Ya-Sabyr wants to merge 2 commits intoOWASP:masterfrom
Conversation
WalkthroughTwo new test files have been added to the API test suite. A conftest.py file provides pytest fixtures that manage test configuration and setup/teardown, including an autouse fixture that snapshots and restores engine configuration state. A comprehensive test_engine.py file contains tests for the API's Flask error handlers, security headers, IP whitelisting, request logging, static assets, authentication, session management, scan operations, results/logs endpoints, attachments, and subprocess startup behavior. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Pull request overview
This PR adds a dedicated API testing fixture layer and a comprehensive test suite targeting nettacker/api/engine.py, aiming to fully exercise its routes, helpers, and error/edge-case behavior without changing production code.
Changes:
- Added
tests/api/conftest.pyto isolate Flask app config/state and redirect results output into a temp directory per test. - Added
tests/api/test_engine.pyto cover API error handlers, security/access logging middleware, auth/IP filtering, routes, export endpoints, search behavior, and API startup helpers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tests/api/conftest.py |
Introduces an autouse fixture to reset API app config and results directory state between tests, plus api_client/api_key helpers. |
tests/api/test_engine.py |
Adds a broad set of unit/integration-style Flask tests for nettacker.api.engine covering routes, middleware, and subprocess/server helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/api/test_engine.py (1)
591-600: Use the sharedapi_keyfixture instead of a hardcoded query value.This test already takes
api_key, so hardcodingtest-keyhere creates a second source of truth and makes fixture changes easier to miss.Suggested fix
- with engine.app.test_request_context("/logs/search?key=test-key&page=1"), patch.object( + with engine.app.test_request_context( + f"/logs/search?key={api_key}&page=1" + ), patch.object( engine, "get_value", side_effect=fake_get_value ), patch.object(engine, "search_logs", return_value=[{"query": ""}]) as mock_search:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/api/test_engine.py` around lines 591 - 600, The test test_go_for_search_logs_falls_back_when_query_lookup_raises hardcodes the key query parameter ("test-key") instead of using the provided api_key fixture; update the engine.app.test_request_context call (and any other occurrences in this test) to use the api_key fixture value (e.g., interpolate api_key into the request path or build the query params from api_key) so the test relies on the shared fixture rather than a second hardcoded source of truth; keep the fake_get_value patching of engine.get_value unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/api/conftest.py`:
- Around line 14-16: The teardown snapshots use shallow copies so nested mutable
state can leak between tests; replace the shallow copies with deep copies for
the saved config state by using a deep copy of engine.app.config and
engine.nettacker_application_config (and any other saved dicts such as the
variables captured as original_app_config and original_application_config)
before mutating globals, and ensure primitives like original_results_dir remain
copied as-is; locate the assignments that create original_app_config and
original_application_config and change them to use a deep copy operation (e.g.,
via copy.deepcopy) so nested structures like api_client_whitelisted_ips are
fully duplicated.
---
Nitpick comments:
In `@tests/api/test_engine.py`:
- Around line 591-600: The test
test_go_for_search_logs_falls_back_when_query_lookup_raises hardcodes the key
query parameter ("test-key") instead of using the provided api_key fixture;
update the engine.app.test_request_context call (and any other occurrences in
this test) to use the api_key fixture value (e.g., interpolate api_key into the
request path or build the query params from api_key) so the test relies on the
shared fixture rather than a second hardcoded source of truth; keep the
fake_get_value patching of engine.get_value unchanged.
🪄 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: Pro
Run ID: ff98b1a9-951e-4a73-b97a-1f48c86e56c8
📒 Files selected for processing (2)
tests/api/conftest.pytests/api/test_engine.py
Proposed change
This PR improves API test coverage by adding a dedicated API test fixture layer and a comprehensive test suite for
nettacker/api/engine.py.The changes include:
tests/api/conftest.pyto isolate Flask app state, API config, and temporary results paths during teststests/api/test_engine.pyto cover:nettacker/api/engine.pynow reaches 100% coverageThis is a test-only change and does not modify production behavior.
Validation performed:
poetry run pytest --cov=nettacker/api/engine --cov-report term-missing tests/api/test_engine.py -qmake testResults:
nettacker/api/engine.py: 0% -> 100%Type of change
Checklist
make pre-commitand confirm it didn't generate any warnings/changesmake test, I confirm all tests passed locallydocs/folder