test(api): speed up API test suite#11681
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe test settings file adds ChangesTest Suite Performance Optimizations
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 docstrings
🧪 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 |
github-actions
Bot
added
github_actions
|
Please add an entry to the corresponding |
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
🔒 Container Security ScanImage: ✅ No Vulnerabilities DetectedThe container image passed all security checks. No known CVEs were found.📋 Resources:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #11681 +/- ##
=======================================
Coverage 94.12% 94.12%
=======================================
Files 247 247
Lines 36541 36581 +40
=======================================
+ Hits 34393 34433 +40
Misses 2148 2148
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
github-actions
Bot
removed
the
github_actions
![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
🤖 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 `@api/src/backend/conftest.py`:
- Around line 116-129: The cached loader wrappers in conftest are using
session-wide state, which can leak mutated Compliance objects and bypass
per-test patches in the affected tests. Update the caching around
cached_bulk_frameworks, cached_get_bulk, and cached_load so it is either scoped
to the tests that opt in or can be explicitly reset between tests via a
fixture/hook. Make sure the cache is cleared before or after each test that
monkeypatches these loaders, and keep the original loader references intact so
patched test behavior is still honored.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: e4ba4fd7-78d8-4447-9b64-a0e9fe3b91f8
📒 Files selected for processing (3)
api/src/backend/config/django/testing.pyapi/src/backend/conftest.pyapi/src/backend/pytest.ini
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 `@api/src/backend/conftest.py`:
- Around line 163-167: The post-`monkeypatch` cleanup in `conftest.py` only
clears the backend compliance caches, but
`api.compliance.get_compliance_frameworks()` also memoizes results in
`AVAILABLE_COMPLIANCE_FRAMEWORKS`. Update the fixture cleanup path that runs
after `yield` to clear that API-level cache as well, alongside
`_COMPLIANCE_FRAMEWORK_CACHE`, `_COMPLIANCE_CHECKS_CACHE`, and
`_COMPLIANCE_PATH_CACHE`, so patched catalog data does not leak stale framework
IDs into later tests.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 44de642b-845f-4fbf-8c33-4b482d86bcc1
📒 Files selected for processing (1)
api/src/backend/conftest.py
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
3 participants
Context
The API test suite had grown very slow. Profiling with
--durations=50showed two dominant cost centers:create_user()/check_password()ran PBKDF2 (~hundreds of ms each), which dominated fixture setup across the whole suite.get_bulk_compliance_frameworks_universal()andCheckMetadata.get_bulk()re-read and Pydantic-validate ~100 compliance JSONs (~20 MB) and ~1k check metadata files on every call. Tests parametrize over every provider and deliberately reset the API-level caches, so the same catalogs were re-parsed dozens of times (≈3s/call locally, ≈19s under coverage in CI). The worst offenders wereTestGetComplianceFrameworks::test_listing_is_subset_of_bulk[*](~19s × 18 providers) andTestComplianceOverviewViewSet::test_compliance_overview_attributes_resolves_provider_from_scan(~64s).This PR speeds up the suite without touching SDK production behavior — all caching lives in the test harness (
conftest.py) and test settings.Description
Test/CI-only changes:
api/src/backend/config/django/testing.py: useMD5PasswordHasherin tests. Secure hashing is unnecessary for tests and PBKDF2 dominated fixture setup time.api/src/backend/pytest.ini: add--reuse-dbso the local loop doesn't recreate/migrate the test DB on every run.api/src/backend/conftest.py: install a test-only memoization of the heavy SDK catalog loaders at conftest import time (before test modules are collected, sofrom ... importbindings resolve to the cached wrappers). This is the test-only equivalent of anlru_cacheon the SDK functions:get_bulk_compliance_frameworks_universal()andCheckMetadata.get_bulk()— avoids repeating the directory scan + filtering per provider.load_compliance_framework_universal()— sinceget_bulk_*parses every JSON and only then filters by provider, a per-provider cache still re-parses all ~100 files on the first load of each provider. Caching by file path means the first provider parses the files once and every other provider/test reuses the already-parsedComplianceFrameworkobjects (only the cheaplistdir+ filtering re-runs)._load_jsons_from_dircallsload_compliance_framework_universalas a module global, so patching the attribute is picked up without touching the SDK..github/workflows/api-tests.yml: add--durations=50to surface the slowest tests in CI output.Why it's safe: the catalog files are immutable during a run and all consumers treat the parsed objects as read-only (they only read
.provider,.supports_provider(),.requirements,.checksto build new structures), so sharing the parsed objects across providers/tests cannot leak state.Steps to review
Run the previously-slow compliance module and check the durations:
Expected:
test_listing_is_subset_of_bulk[*]drops from ~7-8s each (≈19s under coverage) to ~1.7s for the first provider and ~0.01s for every other one. Full module ≈60s → ≈8s.Confirm no regressions on the endpoint path that lazily loads compliance:
Confirm tests that mock these functions still behave correctly (the per-test
unittest.mock.patchoverrides the wrappers and restores them afterward):Performance evidence (
test_compliance.py, local):test_listing_is_subset_of_bulk[<first provider>]test_listing_is_subset_of_bulk[<each other provider>]test_compliance.pymoduleChecklist
Community Checklist
SDK/CLI
UI
API
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Summary by CodeRabbit