Endpoint test depth: fuzz/adversarial ORM at trust boundary#154
Merged
wpak-ai merged 2 commits intoJul 2, 2026
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds shared length and language-code validation, updates add-or-update serializer checks for version, extensions, language counts, and submodule counts, and expands serializer and endpoint tests for invalid inputs and trust-boundary behavior. ChangesValidation hardening and adversarial testing
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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 (3)
tests/endpoint/test_views.py (2)
630-638: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate throttle fixture across two classes.
_high_throttle_limitsis defined identically inTestAddOrUpdateAdversarialandTestOrmTrustBoundary. Consider hoisting to a module-level fixture or shared base class to avoid drift.Also applies to: 847-855
🤖 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/endpoint/test_views.py` around lines 630 - 638, The autouse fixture _high_throttle_limits is duplicated in both TestAddOrUpdateAdversarial and TestOrmTrustBoundary, so move the shared throttle setup into one reusable place such as a module-level pytest fixture or a common base class and have both test classes use it. Keep the existing _throttle_rest_framework and _isolated_throttle_rates behavior unchanged, but reference the shared fixture from both classes to avoid drift and keep the throttle configuration consistent.
654-844: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract repeated
delay_mock/monkeypatchboilerplate into a fixture.The same 5-line block (create
MagicMock,monkeypatch.setattr("boost_weblate.endpoint.views.boost_add_or_update_task.delay", delay_mock)) is repeated in nearly every test in this class. A shared fixture would remove this duplication.♻️ Proposed fixture
+ `@pytest.fixture` + def delay_mock(self, monkeypatch: pytest.MonkeyPatch) -> MagicMock: + mock = MagicMock() + monkeypatch.setattr( + "boost_weblate.endpoint.views.boost_add_or_update_task.delay", mock + ) + return mock + - def test_non_json_body_rejected_without_enqueue( - self, monkeypatch: pytest.MonkeyPatch - ) -> None: - delay_mock = MagicMock() - monkeypatch.setattr( - "boost_weblate.endpoint.views.boost_add_or_update_task.delay", - delay_mock, - ) + def test_non_json_body_rejected_without_enqueue(self, delay_mock: MagicMock) -> None: request, _ = self._authenticated_post( None, content_type="application/json", data="not-json" )🤖 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/endpoint/test_views.py` around lines 654 - 844, Repeated MagicMock/monkeypatch setup for boost_add_or_update_task.delay is duplicated across the AddOrUpdateView tests. Add a shared fixture in test_views.py that creates the mock and applies monkeypatch.setattr to boost_weblate.endpoint.views.boost_add_or_update_task.delay, then inject that fixture into the affected test methods and use its returned mock for the assert_not_called checks.src/boost_weblate/endpoint/validators.py (1)
35-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor duplication between
validate_repo_segmentandvalidate_language_code.Both functions repeat the same non-empty → length → regex → raise pattern, differing only in the regex and error text. Could be consolidated into a single generic helper taking the pattern, field name, and "allowed chars" description, reducing future drift between the two validators.
🤖 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 `@src/boost_weblate/endpoint/validators.py` around lines 35 - 58, Both validate_repo_segment and validate_language_code duplicate the same validation flow, so factor the shared non-empty, length, and regex checks into a single generic helper. Update the helper to accept the input value, field name, regex pattern, and allowed-characters text, then have validate_repo_segment and validate_language_code delegate to it while preserving their current error messages and behavior.
🤖 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 `@src/boost_weblate/endpoint/serializers.py`:
- Around line 233-247: The validate_add_or_update method should short-circuit
immediately when the MAX_ADD_OR_UPDATE_LANGS limit is exceeded instead of only
appending an error and continuing into the per-language loop. Add an early
return (or equivalent guard) right after recording the invalid language-count
error in validate_add_or_update so oversized payloads do not reach the
validate_language_code and submodule validation work. Keep the existing error
accumulation shape consistent with the rest of validate_add_or_update and
preserve the RequestField.ADD_OR_UPDATE context.
---
Nitpick comments:
In `@src/boost_weblate/endpoint/validators.py`:
- Around line 35-58: Both validate_repo_segment and validate_language_code
duplicate the same validation flow, so factor the shared non-empty, length, and
regex checks into a single generic helper. Update the helper to accept the input
value, field name, regex pattern, and allowed-characters text, then have
validate_repo_segment and validate_language_code delegate to it while preserving
their current error messages and behavior.
In `@tests/endpoint/test_views.py`:
- Around line 630-638: The autouse fixture _high_throttle_limits is duplicated
in both TestAddOrUpdateAdversarial and TestOrmTrustBoundary, so move the shared
throttle setup into one reusable place such as a module-level pytest fixture or
a common base class and have both test classes use it. Keep the existing
_throttle_rest_framework and _isolated_throttle_rates behavior unchanged, but
reference the shared fixture from both classes to avoid drift and keep the
throttle configuration consistent.
- Around line 654-844: Repeated MagicMock/monkeypatch setup for
boost_add_or_update_task.delay is duplicated across the AddOrUpdateView tests.
Add a shared fixture in test_views.py that creates the mock and applies
monkeypatch.setattr to
boost_weblate.endpoint.views.boost_add_or_update_task.delay, then inject that
fixture into the affected test methods and use its returned mock for the
assert_not_called checks.
🪄 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
Run ID: 609e6958-7758-4393-9e44-b40105a783c1
📒 Files selected for processing (4)
src/boost_weblate/endpoint/serializers.pysrc/boost_weblate/endpoint/validators.pytests/endpoint/test_serializers.pytests/endpoint/test_views.py
henry0816191
approved these changes
Jul 2, 2026
wpak-ai
approved these changes
Jul 2, 2026
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.
Close #152.
Summary by CodeRabbit
Bug Fixes
extensionsinput (including whitespace-only entries and non-string values).Tests