feat: improve test coverage to 90%#3697
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds many new backend tests across API, OWASP, AI, GitHub/Slack, and core modules; adjusts list ordering logic in issue/milestone/release endpoints; removes inline comments in pagination implementation; and adds one cspell dictionary entry. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@backend/tests/apps/core/models/prompt_test.py`:
- Around line 61-70: The test
test_get_text_not_exists_no_warning_when_no_api_key uses the string "None" which
is truthy; update the patched settings.OPEN_AI_SECRET_KEY in that test to a real
falsy value (e.g., None or an empty string) so the code path that checks
OPEN_AI_SECRET_KEY treats it as unset; locate the patch of
apps.core.models.prompt.settings in the test and change
mock_settings.OPEN_AI_SECRET_KEY = "None" to mock_settings.OPEN_AI_SECRET_KEY =
None (or "").
In `@backend/tests/apps/owasp/admin/entity_member_test.py`:
- Around line 19-28: The test currently asserts mock_request.assert_not_called()
which is meaningless and misses verifying that approve_members calls
self.message_user; replace that assertion by setting self.admin.message_user =
MagicMock() before calling approve_members and then assert
self.admin.message_user.assert_called_once_with(mock_request, ANY) (or
assert_called_once_with(mock_request, f"Approved
{mock_queryset.update.return_value} members") to match the exact message), and
keep verifying mock_queryset.update.assert_called_once_with(is_active=True,
is_reviewed=True); remove mock_request.assert_not_called().
In `@backend/tests/apps/owasp/api/internal/queries/snapshot_test.py`:
- Around line 28-37: The patch targets in this test file are pointing at the
model definition instead of where Snapshot is imported into the query module;
update all patch calls to target
apps.owasp.api.internal.queries.snapshot.Snapshot.objects (e.g. change
patch("apps.owasp.models.snapshot.Snapshot.objects.get") to
patch("apps.owasp.api.internal.queries.snapshot.Snapshot.objects.get") and
similarly for .filter) so the mocked Snapshot used by the query's snapshot
method is intercepted; apply the same change in test_snapshot_not_exists and for
the .filter patches in test_snapshots_with_positive_limit and
test_snapshots_limit_clamped_to_max, keeping mock names like
mock_get/mock_filter and asserting calls against Snapshot.objects as before.
In `@backend/tests/apps/owasp/models/committee_test.py`:
- Around line 108-112: The test assumes Committee.nest_key is a property but the
implementation exposes it as a method; update the test in test_nest_key_complex
to call committee.nest_key() instead of accessing committee.nest_key so it
invokes the method and compares the returned value ("web-security"); reference
the test function test_nest_key_complex and the Committee.nest_key method when
making the change.
- Around line 103-107: The test treats nest_key as a property but the model
defines it as a method; update the test to call the method by changing the
assertion to use committee.nest_key() (referencing the Committee class and its
nest_key method), or alternatively add `@property` to the Committee.nest_key
definition in the model so committee.nest_key can be accessed without
parentheses—pick one consistent approach and update either the test
(committee.nest_key()) or the model (decorate nest_key with `@property`).
In `@backend/tests/apps/owasp/models/entity_member_test.py`:
- Around line 13-22: The test named test_str_with_member_login and its docstring
claim to test the string representation but never call str() or __str__ on
EntityMember; either rename the test to something like
test_role_and_member_name_attributes to match the existing assertions (referring
to EntityMember, member_name, role, get_role_display) or add an assertion for
the string representation (call str(entity_member) or entity_member.__str__()
and assert it equals the expected string) and update the docstring accordingly.
- Around line 24-32: The test test_str_without_member_uses_name currently never
calls str() despite its name; update it to either (A) assert the string
representation by calling str(entity_member) and expecting the member_name
(e.g., assert str(entity_member) == "John Doe") so it truly verifies
EntityMember.__str__, or (B) if you only want to verify fields, rename the test
to something like test_member_name_and_role_display_without_member and keep the
existing assertions on entity_member.member_name and
entity_member.get_role_display(); modify the test accordingly and keep
references to EntityMember and test_str_without_member_uses_name to locate the
code.
In `@backend/tests/apps/slack/management/commands/slack_sync_messages_test.py`:
- Around line 255-299: The SlackApiError mock's response needs to behave like a
real dict-like response (support both bracket access and .get()) so tests
hitting Command._sync_user_messages and downstream _create_message see the same
shape as production; update the test's rate_limit_error.response to be a
dict-like object (or a MagicMock) that implements __getitem__ and get (returning
"ratelimited" for "error") and has a headers attribute {"Retry-After": "1"} so
both e.response["error"] and e.response.get("error") work correctly.
🧹 Nitpick comments (17)
backend/tests/apps/owasp/models/project_test.py (5)
140-152: Mocking targets wrong class - test may not exercise actual code.The mock patches
RepositoryBasedEntityModel.entity_leadersbut theProjectmodel likely inherits and overrides this property. IfProject.entity_leaderscallssuper().entity_leadersand then slices, this test would work, but the assertion only verifies the returned length is 5 without verifying that the slicing logic[:MAX_LEADERS_COUNT]is actually being invoked from theProjectclass.Consider verifying the mock was called and that the result is indeed the first 5 items from the mocked list to ensure the slicing logic is tested:
project = Project() result = project.entity_leaders - assert len(result) == 5 # MAX_LEADERS_COUNT + assert len(result) == 5 # MAX_LEADERS_COUNT + assert result == mock_leaders[:5]
206-218: Consider asserting filter arguments for more robust testing.The test verifies
filterwas called but doesn't check the actual filter arguments. This would make the test more resilient to implementation changes.💡 Suggested improvement
project = Project() _ = project.issues - mock_issue.objects.filter.assert_called_once() + mock_issue.objects.filter.assert_called_once_with(repository__in=["repo1"])
270-282: Same pattern astest_issues- consider verifying filter arguments.For consistency and robustness, verify the filter is called with the expected
repository__inargument.
260-268: Weak assertion - verify full URL or mock interaction.The mock returns a specific URL but the assertion only checks for a substring. Consider verifying the complete URL or that
get_absolute_urlwas called with the expected argument.💡 Suggested improvement
project = Project(key="www-project-juice-shop") - assert "juice-shop" in project.nest_url + assert project.nest_url == "https://nest.owasp.org/projects/juice-shop"
441-464: Fragile mocking pattern using__bases__[0].Using
Project.__bases__[0].saveis brittle - if the inheritance hierarchy changes, this test will break unexpectedly. Consider patching the parent class directly by name:💡 Suggested improvement
- mocker.patch.object(Project.__bases__[0], "save") + mocker.patch("apps.owasp.models.project.RepositoryBasedEntityModel.save")backend/tests/apps/owasp/models/mixins/project_test.py (1)
165-171: Weak assertion for name generation from key.The assertion only checks that "juice" appears in the result. Consider verifying the exact expected transformation (e.g., title-casing, prefix stripping).
💡 Suggested improvement
result = ProjectIndexMixin.idx_name.fget(mock_project) - assert "juice" in result.lower() + # Verify the exact transformation from key to name + assert result == "Juice Shop" # or whatever the expected output isbackend/tests/apps/owasp/admin/entity_channel_test.py (1)
61-77: Fragile mock pattern forDoesNotExist.Setting
mock_conversation.DoesNotExist = Exceptionmay not correctly simulate the realConversation.DoesNotExistexception behavior. If the production code usesexcept Conversation.DoesNotExist, this mock works only becauseExceptionis a superclass. However, this pattern can mask issues if the code catches a more specific exception type.A more robust approach is to use the actual model's exception or define a custom exception class:
♻️ Suggested improvement
- mock_conversation.DoesNotExist = Exception - mock_conversation.objects.get.side_effect = mock_conversation.DoesNotExist + class MockDoesNotExist(Exception): + pass + mock_conversation.DoesNotExist = MockDoesNotExist + mock_conversation.objects.get.side_effect = MockDoesNotExist("Not found")backend/tests/apps/owasp/models/event_test.py (4)
158-170: Acceptable test, but consider verifying filter arguments.The test confirms
upcoming_events()returns the expected queryset. For stronger coverage, consider asserting the filter arguments (e.g.,start_date__gtefor filtering future events) to ensure the filtering logic is correct.
176-198: Consider a more specific assertion for the new event result.Line 196 asserts
result is not None, but a stronger assertion would verify the result is anEventinstance to ensure the method returns the correct type:- assert result is not None + assert isinstance(result, Event)
243-260: Weak assertion for fallback behavior test.The assertion on line 260 (
assert event.latitude is not None) doesn't adequately verify the fallback behavior. Consider:
- Verifying both
latitudeandlongitudeare set correctly.- Asserting that
get_location_coordinateswas called with the expected fallback value (context string).♻️ Suggested improvement
with patch("apps.owasp.models.event.get_location_coordinates") as mock_get_coords: mock_get_coords.return_value = mock_location event.generate_geo_location() - assert event.latitude is not None + assert event.latitude == 40.7128 + assert event.longitude == -74.0060 + # Verify fallback to get_context was used (not suggested_location) + mock_get_coords.assert_called_once()
286-330: Prompt mock setup could be more explicit.In
test_generate_suggested_location_handles_none_result(line 296) andtest_generate_suggested_location_handles_exception(line 319), thePromptis patched butget_owasp_event_suggested_locationisn't explicitly configured. While this works due to Mock's default truthy behavior, explicit setup improves test clarity and prevents surprises if the implementation changes:mock_prompt.get_owasp_event_suggested_location.return_value = "prompt"backend/tests/apps/owasp/api/internal/queries/stats_test.py (1)
11-36: Add a brief comment documenting the rounding behavior.The test sets mock return values (275, 342, 15234, 98, 5500) but asserts rounded values (270, 340, 15000, 90, 5000). While this correctly tests the rounding logic in
stats_overview, a short docstring note or inline comment would clarify that these assertions intentionally verify rounding behavior.backend/tests/apps/owasp/api/internal/queries/snapshot_test.py (1)
73-82: Consider asserting the actual slice value to verify MAX_LIMIT clamping.The test name suggests it verifies limit clamping to
MAX_LIMIT, but the assertion only checks the return value matches the mock. To truly verify clamping behavior, you could assert the slice argument passed to__getitem__:mock_filter.return_value.order_by.return_value.__getitem__.assert_called_once() # Check the slice argument matches the expected clamped valuebackend/tests/apps/owasp/api/internal/queries/project_test.py (2)
75-87: Inconsistent patch path compared to other tests in this file.The patch at line 79 targets
apps.owasp.models.project.Project.objects.filter, but tests foris_project_leader(lines 170-173) correctly patch at the query module path. For consistency and to ensure proper isolation, patch where the object is used:patch("apps.owasp.api.internal.queries.project.Project.objects.filter")The same applies to
test_search_projects_with_valid_queryat line 111.
128-134: Consider using the actual constant instead of a magic number.The test uses
"a" * 101assumingMAX_SEARCH_QUERY_LENGTHis 100. If this constant changes, the test may silently become invalid. Consider importing and using the constant:♻️ Suggested improvement
+from apps.owasp.api.internal.queries.project import MAX_SEARCH_QUERY_LENGTH + def test_search_projects_query_too_long(self): """Test search_projects returns empty for query > MAX_SEARCH_QUERY_LENGTH.""" query = ProjectQuery() - long_query = "a" * 101 + long_query = "a" * (MAX_SEARCH_QUERY_LENGTH + 1) result = query.__class__.__dict__["search_projects"](query, query=long_query) assert result == []backend/tests/apps/api/rest/v0/milestone_test.py (1)
157-172: Consider adding an assertion for the state filter.The test exercises the state filter path but doesn't verify that the filter was actually applied with the correct parameter. Other filter tests in this file assert the
filtercall arguments.📝 Suggested improvement
def test_list_milestones_with_state_filter(self, mocker): """Test listing milestones with state filter.""" mock_qs = MagicMock() mock_filtered = MagicMock() mock_qs.select_related.return_value = mock_qs mock_qs.filter.return_value = mock_filtered mock_filtered.filter.return_value = mock_filtered mock_filtered.order_by.return_value = [] mocker.patch( "apps.api.rest.v0.milestone.MilestoneModel.objects", mock_qs, ) request = MagicMock() filters = MilestoneFilter(state="open") list_milestones(request, filters, None) + + mock_qs.filter.assert_called_with(state="open")backend/tests/apps/api/rest/v0/release_test.py (1)
103-118: Consider adding an assertion for the tag_name filter.Similar to the milestone state filter test, this test exercises the tag filter path but doesn't verify the filter was applied with the correct parameter.
📝 Suggested improvement
def test_list_release_with_tag_filter(self, mocker): """Test listing releases with tag_name filter.""" mock_qs = MagicMock() mock_filtered = MagicMock() mock_qs.exclude.return_value.select_related.return_value = mock_qs mock_qs.filter.return_value = mock_filtered mock_filtered.filter.return_value = mock_filtered mock_filtered.order_by.return_value = [] mocker.patch( "apps.api.rest.v0.release.ReleaseModel.objects", mock_qs, ) request = MagicMock() filters = ReleaseFilter(tag_name="v1.0.0") list_release(request, filters, None) + + mock_filtered.filter.assert_called_with(tag_name="v1.0.0")
backend/tests/apps/slack/management/commands/slack_sync_messages_test.py
Show resolved
Hide resolved
|
@arkid15r , please review it and let me know if any changes are required. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 25 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="backend/tests/apps/api/rest/v0/milestone_test.py">
<violation number="1" location="backend/tests/apps/api/rest/v0/milestone_test.py:116">
P2: Test has no assertions - it calls `list_milestones` with a state filter but never verifies that the state filter is actually applied. Add an assertion like `mock_filtered.filter.assert_called_with(state="open")` to verify the filtering behavior.</violation>
</file>
<file name="backend/tests/apps/api/rest/v0/issue_test.py">
<violation number="1" location="backend/tests/apps/api/rest/v0/issue_test.py:65">
P3: This test doesn’t assert that the state filter is applied, so it passes even if list_issues stops filtering by state. Add an assertion on the queryset filter call to ensure the behavior is actually verified.</violation>
</file>
<file name="backend/tests/apps/api/rest/v0/release_test.py">
<violation number="1" location="backend/tests/apps/api/rest/v0/release_test.py:62">
P2: Test has no assertions - `test_list_release_with_tag_filter` calls the function but never verifies that the `tag_name` filter is actually applied. Add an assertion to verify the filter behavior, similar to other filter tests in this class.</violation>
</file>
<file name="backend/tests/apps/owasp/models/project_test.py">
<violation number="1" location="backend/tests/apps/owasp/models/project_test.py:448">
P2: Using `Project.__bases__[0]` is fragile and unclear. If the inheritance order of `Project` changes, this test will silently patch the wrong class. Import and patch `BulkSaveModel` explicitly for clarity and maintainability.</violation>
</file>
<file name="backend/tests/apps/owasp/models/event_test.py">
<violation number="1" location="backend/tests/apps/owasp/models/event_test.py:261">
P2: Weak test assertion: should verify both `latitude` and `longitude` values using `math.isclose()` like the similar test above. This test doesn't verify the actual coordinate values or that longitude was set correctly.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Hi @arkid15r, should I go ahead and fix this? |
Yes please -- https://owasp.slack.com/archives/C07JLLG2GFQ/p1770077515910809 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/tests/apps/api/rest/v0/milestone_test.py`:
- Around line 186-188: The implementation of list_milestones currently always
appends "-updated_at" as a secondary sort (returning
milestones.order_by(ordering or "-created_at", "-updated_at")), which produces
redundant ordering when the primary ordering is "updated_at" or "-updated_at";
update list_milestones to detect whether the primary ordering field is
updated_at (compare ordering stripped of leading '-' to "updated_at") and only
append the "-updated_at" secondary when the primary is a different field
(otherwise omit it or use an alternative tie-breaker such as "pk" when needed)
so the call to order_by does not redundantly include both "updated_at" and
"-updated_at".
🧹 Nitpick comments (12)
backend/tests/apps/owasp/models/project_test.py (4)
140-152: Consider importing MAX_LEADERS_COUNT constant for maintainability.The test hardcodes the expected count as
5. IfMAX_LEADERS_COUNTchanges in the future, this test would silently pass with incorrect expectations. Import the constant from the model to keep tests in sync with the implementation.Suggested improvement
from apps.owasp.models.project import Project +from apps.owasp.models.project import MAX_LEADERS_COUNT ... def test_entity_leaders(self, mocker): """Test entity_leaders returns up to MAX_LEADERS_COUNT leaders.""" - mock_leaders = [Mock() for _ in range(7)] + mock_leaders = [Mock() for _ in range(MAX_LEADERS_COUNT + 2)] mocker.patch( "apps.owasp.models.common.RepositoryBasedEntityModel.entity_leaders", new_callable=mocker.PropertyMock, return_value=mock_leaders, ) project = Project() result = project.entity_leaders - assert len(result) == 5 + assert len(result) == MAX_LEADERS_COUNT
177-186: Consider adding test for non-compliant case.This test only covers the case where all repositories are funding-compliant. Adding a test where
exists()returnsTrue(non-compliant repositories exist) would improve branch coverage.Suggested additional test
def test_is_funding_requirements_not_compliant(self, mocker): """Test returns False when non-compliant repositories exist.""" mock_repos = Mock() mock_repos.filter.return_value.exists.return_value = True mocker.patch.object( Project, "repositories", new_callable=mocker.PropertyMock, return_value=mock_repos ) project = Project() assert not project.is_funding_requirements_compliant
260-268: Assertion could be more precise.The test mocks
get_absolute_urlbut only checks that"juice-shop"is in the result. A stronger assertion would verify the exact returned value or check thatget_absolute_urlwas called with the correct argument.Suggested improvement
def test_nest_url(self, mocker): """Test nest_url returns full URL.""" - mocker.patch( + mock_get_url = mocker.patch( "apps.owasp.models.project.get_absolute_url", return_value="https://nest.owasp.org/projects/juice-shop", ) project = Project(key="www-project-juice-shop") - assert "juice-shop" in project.nest_url + assert project.nest_url == "https://nest.owasp.org/projects/juice-shop" + mock_get_url.assert_called_once_with("/projects/juice-shop")
441-464: Missing test cases for save method branches.The
savemethod has the conditionif self.is_active and not self.summary and (prompt := ...). Current tests cover:
is_active=True,summary="", prompt exists → generates summaryis_active=True,summaryexists → skipsMissing coverage for:
is_active=False→ should skip summary generation- Prompt returns
None→ should skip summary generationSuggested additional tests
def test_save_skips_summary_when_inactive(self, mocker): """Test save skips summary generation when project is inactive.""" mock_generate = mocker.patch.object(Project, "generate_summary") mocker.patch.object(BulkSaveModel, "save") project = Project(is_active=False, summary="") project.save() mock_generate.assert_not_called() def test_save_skips_summary_when_no_prompt(self, mocker): """Test save skips summary generation when no prompt available.""" mocker.patch( "apps.owasp.models.project.Prompt.get_owasp_project_summary", return_value=None, ) mock_generate = mocker.patch.object(Project, "generate_summary") mocker.patch.object(BulkSaveModel, "save") project = Project(is_active=True, summary="") project.save() mock_generate.assert_not_called()backend/tests/apps/ai/agent/agent_test.py (3)
23-25: Consider strengthening the assertions.The
is not Nonechecks will pass for any truthy value. Since the production code assignsself.nodes = AgentNodes(), a more precise assertion would verify the mock instance.💡 Suggested improvement
mock_nodes.assert_called_once() - assert agent.nodes is not None - assert agent.graph is not None + assert agent.nodes is mock_nodes.return_value + assert agent.graph is mock_graph_instance.compile.return_value
49-49: Consider verifying the invoke arguments.The test verifies
invokewas called but not with what arguments. Asserting the initial state structure would validate the input to the workflow.💡 Suggested improvement
- mock_compiled_graph.invoke.assert_called_once() + call_args = mock_compiled_graph.invoke.call_args[0][0] + assert call_args["query"] == "Test query" + assert call_args["iteration"] == 0 + assert call_args["feedback"] is None + assert call_args["history"] == []
83-84: Minor inconsistency:ENDmock not captured.
STARTis captured and used in assertions (line 92), butENDis patched without capturing its reference. IfENDis used in the conditional edges (which it likely is), consider capturing it for consistent verification.💡 Suggested improvement
mock_start = mocker.patch(f"{self.target_module}.START") - mocker.patch(f"{self.target_module}.END") + mock_end = mocker.patch(f"{self.target_module}.END")Then you can verify
mock_endis passed toadd_conditional_edgesif needed.backend/tests/apps/api/rest/v0/issue_test.py (3)
83-85: Consider usingassert_called_once_withfor stricter verification.Using
assert_called_withonly verifies the most recent call's arguments, not that the method was called exactly once. If the implementation accidentally calls.filter()multiple times, this assertion would still pass.♻️ Suggested improvement
- mock_qs.filter.assert_called_with(repository__organization__login__iexact="OWASP") + mock_qs.filter.assert_called_once_with(repository__organization__login__iexact="OWASP")Apply similar changes to lines 104 and 121 for
assert_called_withusages.
50-137: Consider adding a test for combined filters.The tests cover individual filters (organization, repository, state) separately, but there's no test exercising all filters together. Adding a combined test would verify the filter chaining behavior works correctly when multiple filters are applied simultaneously.
♻️ Example test for combined filters
def test_list_issues_with_combined_filters(self, mocker): """Test listing issues with organization, repository, and state filters combined.""" mock_qs = MagicMock() mock_filtered = MagicMock() mock_qs.select_related.return_value = mock_qs mock_qs.filter.return_value = mock_filtered mock_filtered.filter.return_value = mock_filtered mock_filtered.order_by.return_value = [] mocker.patch( "apps.api.rest.v0.issue.IssueModel.objects", mock_qs, ) request = MagicMock() filters = IssueFilter(organization="OWASP", repository="Nest", state="open") list_issues(request, filters, None) # Verify all filters were applied mock_qs.filter.assert_called_with(repository__organization__login__iexact="OWASP") assert mock_filtered.filter.call_count == 2 # repository and state filters
123-137: Test correctly captures the implementation behavior.The test is valid. The implementation at
apps/api/rest/v0/issue.py:93callsorder_by(ordering or "-created_at", "-updated_at"), which always appends a secondary sort on"-updated_at"regardless of the primary sort field. Whenordering="updated_at"is passed, this results inorder_by("updated_at", "-updated_at"), which the test correctly verifies.However, this secondary sort is functionally redundant because a field cannot have ties with itself—a secondary sort on the same column (even with opposite direction) has no effect. Consider optimizing the implementation to skip the secondary sort when the primary sort is already on an
updated_atvariant.backend/tests/apps/owasp/models/event_test.py (2)
77-104: Add direct assertions for from_dict/save on existing-event path.
This makes the test explicitly verify the core side effects.Proposed test strengthening
result = Event.update_data(category, data) assert result == mock_event + mock_event.from_dict.assert_called_once_with(category, data) + mock_event.save.assert_called_once()
159-172: Consider asserting filter/order_by arguments for upcoming_events.
Right now the test only checks that.filter()was called, not the query contract.Proposed test strengthening
def test_upcoming_events(self): """Test upcoming_events returns future events.""" - with patch.object(Event, "objects") as mock_objects: + frozen_now = date(2025, 1, 1) + with ( + patch.object(Event, "objects") as mock_objects, + patch("apps.owasp.models.event.timezone.now", return_value=frozen_now), + ): mock_qs = Mock() mock_objects.filter.return_value.exclude.return_value.order_by.return_value = mock_qs result = Event.upcoming_events() assert result == mock_qs - mock_objects.filter.assert_called_once() + mock_objects.filter.assert_called_once_with(start_date__gt=frozen_now) + mock_objects.filter.return_value.exclude.assert_called_once() + mock_objects.filter.return_value.exclude.return_value.order_by.assert_called_once_with( + "start_date" + )
|
@arkid15r, please review it and let me know if any changes are required. |
|
@cubic-dev-ai review this pull request |
@HarshitVerma109 I have started the AI code review. It will take a few minutes to complete. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3697 +/- ##
==========================================
+ Coverage 87.47% 90.26% +2.79%
==========================================
Files 463 463
Lines 14408 14420 +12
Branches 1931 1934 +3
==========================================
+ Hits 12603 13016 +413
+ Misses 1376 987 -389
+ Partials 429 417 -12
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 19 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
@arkid15r, please review it and let me know if any changes are required. |
|
2 participants
Proposed change
Resolves #613
Improved backend test coverage to 91.46% by adding comprehensive tests
Checklist
make check-testlocally: all warnings addressed, tests passed