feat: increase backend test coverage to 87%#3631
Merged
arkid15r merged 8 commits intoOWASP:mainfrom Jan 29, 2026
Merged
Conversation
Contributor
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds many new unit tests across backend apps (AI agent, common middleware, GitHub, OWASP, Slack) and a single dictionary entry; tests cover AgentNodes flows, null-character middleware, GitHub command/model logic, OWASP admin/views/commands, Slack handlers/commands, and PDF/URL endpoints. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
arkid15r
requested changes
Jan 29, 2026
Collaborator
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
Please don't forget to check your code locally before requesting review from maintainers.
backend/tests/apps/common/middlewares/block_null_characters_test.py
Outdated
Show resolved
Hide resolved
backend/tests/apps/owasp/management/commands/owasp_sync_board_candidates_test.py
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/tests/apps/ai/agent/nodes_test.py`:
- Around line 8-11: The test fixture mock_openai patches openai.OpenAI globally;
change it to patch the import location used by the module under test by patching
"apps.ai.agent.nodes.openai.OpenAI" instead (keep the existing
mocker.patch("os.getenv", return_value="fake-key") and return the patched
object), so the mock applies only to the OpenAI class used in tests for
functions/classes in apps.ai.agent.nodes.
In `@backend/tests/apps/slack/management/commands/owasp_match_channels_test.py`:
- Around line 87-90: The test currently patches EntityChannel.objects as
mock_ec_qs but doesn't explicitly set the return value for the
filter(...).exists() chain, relying on Mock truthiness; update the mock to
explicitly set mock_ec_qs.filter.return_value.exists.return_value = True (or
False as appropriate for the "exists" path you want to cover) so the command's
dry-run branch is exercised; keep the existing mock_get_or_create setup
(mock_get_or_create.return_value = (None, True)) and ensure you set the exists()
value before invoking the command in the test.
backend/tests/apps/slack/management/commands/owasp_match_channels_test.py
Show resolved
Hide resolved
arkid15r
requested changes
Jan 29, 2026
backend/tests/apps/owasp/management/commands/owasp_sync_board_candidates_test.py
Outdated
Show resolved
Hide resolved
backend/tests/apps/owasp/api/internal/views/permissions_test.py
Outdated
Show resolved
Hide resolved
HarshitVerma109
force-pushed
the
test-coverage-improvement
branch
from
6171665 to
e8bc2e0
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/tests/apps/owasp/api/internal/views/project_health_metrics_test.py`:
- Around line 120-152: The test name
test_generate_project_health_metrics_pdf_no_metrics is misleading because the
mock for ProjectHealthMetrics.get_latest_health_metrics returns a truthy mock
(so metrics exist) and the Http404 is triggered by generate_latest_metrics_pdf
returning None; rename the test to
test_generate_project_health_metrics_pdf_generation_fails or change the mock to
return None from ProjectHealthMetrics.get_latest_health_metrics.first() to truly
test the "no metrics" path; locate references to
generate_project_health_metrics_pdf,
ProjectHealthMetrics.get_latest_health_metrics, and generate_latest_metrics_pdf
in the test to apply the rename or adjust the mocked return value accordingly.
🧹 Nitpick comments (5)
backend/tests/apps/github/models/comment_test.py (1)
71-76: Remove redundant initial body assignment.The
bodyparameter on line 72 is immediately overwritten on line 74, making the initial assignment unnecessary.♻️ Suggested simplification
def test_str_representation(self): - comment = Comment(body="A very long comment body that should be truncated", author=None) - long_body = "A" * 60 - comment.body = long_body + comment = Comment(body="A" * 60, author=None) assert str(comment).startswith("None - AAAAA") assert len(str(comment)) <= 60 # approxbackend/tests/apps/common/middlewares/block_null_characters_test.py (2)
39-47: Consider verifying response message for consistency.These tests only assert the status code, whereas
test_null_in_path_blocksandtest_null_in_body_blocksalso verify the full JSON response body. For consistency and to catch regressions in error messages, consider adding the same message assertion here.♻️ Suggested enhancement for consistency
def test_null_in_query_params_blocks(self, middleware, factory): request = factory.get("/clean/path", {"q": "bad\x00value"}) response = middleware(request) assert response.status_code == HTTPStatus.BAD_REQUEST + assert json.loads(response.content) == { + "message": "Request contains null characters in URL or parameters " + "which are not allowed.", + "errors": {}, + } def test_null_in_post_data_blocks(self, middleware, factory): request = factory.post("/clean/path", {"data": "bad\x00value"}) response = middleware(request) assert response.status_code == HTTPStatus.BAD_REQUEST + assert json.loads(response.content) == { + "message": "Request contains null characters in URL or parameters " + "which are not allowed.", + "errors": {}, + }
62-69: Consider adding response body assertion for unicode null test as well.For consistency with
test_null_in_body_blocks, this test should also verify the JSON response message.♻️ Suggested enhancement
def test_unicode_null_in_body_blocks(self, middleware, factory): request = factory.post( "/clean/path", data=b'{"key": "bad\\u0000value"}', content_type="application/json", ) response = middleware(request) assert response.status_code == HTTPStatus.BAD_REQUEST + assert json.loads(response.content) == { + "message": "Request contains null characters in body which are not allowed.", + "errors": {}, + }backend/tests/apps/owasp/management/commands/owasp_sync_board_candidates_test.py (1)
69-71: Remove redundant mock.This patch is immediately overridden by the patch at lines 94-97 with a
side_effect, making this initial patch completely unused.♻️ Proposed fix
def test_sync_year_candidates_success(self, command, mocker): - mocker.patch( - "apps.owasp.management.commands.owasp_sync_board_candidates.get_repository_file_content" - ) - mock_board = Mock() mock_board.id = 100backend/tests/apps/owasp/admin/mixins_test.py (1)
45-52: Make HTML assertions less brittle.Hard-coding single-quote markup can make tests fail if attribute quoting/order changes. Prefer checking URL substrings (or parsing HTML) to keep tests stable.
♻️ Suggested tweak
- assert "href='https://owasp.org/test-project'" in url - assert "target='_blank'" in url + assert "https://owasp.org/test-project" in url + assert "target" in url and "_blank" in url @@ - assert "href='https://github.com/owasp/repo-1'" in urls - assert "href='https://github.com/owasp/repo-2'" in urls + assert "https://github.com/owasp/repo-1" in urls + assert "https://github.com/owasp/repo-2" in urls @@ - assert "href='https://github.com/owasp/main-repo'" in urls + assert "https://github.com/owasp/main-repo" in urlsAlso applies to: 61-77, 79-92
backend/tests/apps/owasp/api/internal/views/project_health_metrics_test.py
Outdated
Show resolved
Hide resolved
|
Collaborator
|
Thanks for improving BE test coverage! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3631 +/- ##
==========================================
+ Coverage 85.56% 87.72% +2.15%
==========================================
Files 463 463
Lines 14306 14306
Branches 1903 1903
==========================================
+ Hits 12241 12550 +309
+ Misses 1686 1344 -342
- Partials 379 412 +33
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
This was referenced Jan 30, 2026
This was referenced Feb 1, 2026
This was referenced Feb 7, 2026
4 tasks
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
2 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.
Proposed change
Resolves #3607
This PR increases the backend test coverage from 83% to 87% by adding comprehensive unit tests for previously untested or low-coverage files.
Checklist
make check-testlocally: all warnings addressed, tests passed