feat: improve backend test coverage to 96%#3840
Conversation
Summary by CodeRabbit
WalkthroughAdds per-call retry_count and retry/backoff handling for Slack API rate limits in the Slack message sync command and introduces a large suite of new and expanded backend tests across REST endpoints, OWASP models/mixins/managers, GraphQL nodes/queries, management commands, Slack modules, and utilities. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 47 files
Confidence score: 4/5
- Overall risk looks low because the issues are confined to test coverage/structure rather than runtime code changes.
- Most notable gap:
backend/tests/apps/api/rest/v0/member_test.pydoesn’t assertmock_filters.filter()was called, so thelist_membersfilter behavior isn’t actually verified. backend/tests/apps/owasp/video_test.pyoverwrites the render-error setup, meaning that error path isn’t exercised as intended.- Pay close attention to
backend/tests/apps/api/rest/v0/member_test.py,backend/tests/apps/owasp/video_test.py,backend/tests/apps/owasp/models/snapshot_test.py,backend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.py- test assertions/structure gaps and duplication.
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/owasp/models/snapshot_test.py">
<violation number="1" location="backend/tests/apps/owasp/models/snapshot_test.py:84">
P2: This test bypasses Snapshot.save by patching it to a no-op and manually setting the key, so it never verifies the actual save behavior (including the `now()` call inside the model). The test can pass even if save is broken.</violation>
</file>
<file name="backend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.py">
<violation number="1" location="backend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.py:116">
P2: The `QS` helper class is defined identically 3 times in this file. Consider extracting it to the class level or as a fixture to avoid duplication.</violation>
</file>
<file name="backend/tests/apps/api/rest/v0/member_test.py">
<violation number="1" location="backend/tests/apps/api/rest/v0/member_test.py:69">
P2: Test is incomplete - missing assertion that `mock_filters.filter()` was called. The actual `list_members` function calls `filters.filter(queryset)` but this test doesn't verify that behavior. Add `mock_filters.filter.assert_called_once_with(mock_queryset)` to ensure the filter is actually applied. The same issue exists in `test_list_members_with_ordering`.</violation>
</file>
<file name="backend/tests/apps/owasp/video_test.py">
<violation number="1" location="backend/tests/apps/owasp/video_test.py:132">
P3: This line overrides the earlier `write_pdf.side_effect = Exception("Render Error")`, so the render-error path is never tested. Either remove this override or split the test to cover both errors; as written, the first setup is dead and the test only covers the page error.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
backend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.py
Outdated
Show resolved
Hide resolved
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/tests/apps/github/management/commands/github_enrich_issues_test.py (1)
164-196:⚠️ Potential issue | 🟡 MinorTest name is misleading and doesn't verify the intended behavior.
The function
test_handle_with_chunked_savedoesn't actually test chunked saving at the command level—the command callsbulk_save()once regardless of item count. The real value of using 1001 items was to exercise Django ORM's internal BATCH_SIZE=1000 boundary condition (bulk_create would split into 2 batches), which is lost with only 50 items. If the goal is speed, consider either keeping 1001 items, using a smaller BATCH_SIZE in the test, or renaming the test to reflect that it's testing basic multi-issue processing rather than chunking.
🤖 Fix all issues with AI agents
In `@backend/tests/apps/api/rest/v0/committee_test.py`:
- Around line 73-90: The function name get_chapter is inconsistent with its
behavior (it operates on CommitteeModel) and with tests calling get_chapter in
committee_test.py; rename the source function get_chapter to get_committee in
the committee module and update its docstring to "Get committee", then update
any imports/usages (including tests, e.g., TestGetCommittee) to reference
get_committee so names reflect the CommitteeModel operations and eliminate the
copy-paste mismatch.
In `@backend/tests/apps/api/rest/v0/organization_test.py`:
- Around line 50-62: The test test_list_organization_no_ordering doesn't assert
that the default ordering is applied; update the test to verify that
OrganizationModel.objects.filter(...).order_by was called with the default
"-created_at" when ordering=None. Specifically, in the test referencing
OrganizationModel and list_organization, add an assertion that the mocked
chain's order_by was called with "-created_at" (e.g., assert
mock_org_model.objects.filter.return_value.order_by.assert_called_with("-created_at")),
while keeping the existing assertions on filter and returned queryset.
In
`@backend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.py`:
- Around line 141-185: The test test_handle_no_release_or_license currently only
asserts mock_bulk_save.called; update it to assert the no-release/no-license
branch effects on the Project object instead: after command.handle(offset=0)
verify on the mock_project that attributes reflecting release/licensing were not
set or were cleared (e.g., ensure mock_project.released_at is None or not
assigned and mock_project.licenses is empty/has length 0) and that no
license-related fields were populated; keep the existing use of
Project.bulk_save/mock_active_projects but replace the weak mock_bulk_save-only
assertion with these explicit checks against mock_project to prove the branch
executed.
In `@backend/tests/apps/owasp/models/common_test.py`:
- Around line 544-556: The test test_get_urls_with_domain_value_error claims to
exercise the ValueError branch in get_urls but urlparse("https://[::1]:8080")
does not raise, so update the test to actually trigger that branch by mocking
urlparse used in apps.owasp.models.common to raise ValueError (e.g., patch
common.urlparse with side_effect=ValueError) while keeping
get_repository_file_content returning the mixed content, then assert that
get_urls(domain="example.com") still returns the expected https://example.com
entry; alternatively, if you prefer not to mock, rename the test to reflect it
validates domain filtering with mixed valid/invalid-looking URLs rather than
ValueError handling.
In `@backend/tests/apps/owasp/models/snapshot_test.py`:
- Around line 84-104: The tests currently don't exercise Snapshot.save() because
they replace it with a no-op and perform the key logic inline; change the tests
to call Snapshot.save() so the model's logic runs: remove the patch that
replaces Snapshot.save and instead patch the parent Django save method
(models.Model.save) to avoid DB I/O, then instantiate a Snapshot (e.g., via
Snapshot.__new__(Snapshot) or a proper constructor), set snapshot.key="" for the
generate case or snapshot.key="2024-12" for the preserve case, patch
timezone.now() as needed, call snapshot.save(), and assert the key was
set/preserved; reference Snapshot.save and models.Model.save to locate the code
to patch.
In `@backend/tests/apps/owasp/video_test.py`:
- Around line 129-132: Remove the dead setup that assigns
mock_html.return_value.write_pdf.side_effect = Exception("Render Error") because
it is immediately cleared later; locate the test's mocked objects (mock_html and
mock_pdfium) and delete the line that sets write_pdf.side_effect to that
Exception so the later explicit behavior
(mock_html.return_value.write_pdf.side_effect = None and mock_pdf setup) is the
effective configuration.
In `@backend/tests/apps/slack/management/commands/owasp_match_channels_test.py`:
- Around line 123-127: The test uses weak assertions that pass when no matches
are found; update the assertions around find_fuzzy_matches to require a positive
match and validate its content: call cmd.find_fuzzy_matches("Test Project",
conversations, threshold=50), assert that matches is non-empty (e.g.,
len(matches) == 1 or len(matches) > 0) and then assert matches[0][0].name ==
"project-test" (and optionally assert the match score meets the threshold),
ensuring the check is not guarded by a conditional that allows silent success.
In `@backend/tests/apps/slack/management/commands/slack_sync_messages_test.py`:
- Around line 594-626: The test misses mocking the Django model used by
Command._fetch_messages so Message.bulk_save is invoked for real; patch the
Message symbol used in the Command module (or the module path under test) to a
MagicMock and set Message.bulk_save to a no-op/mock before calling
Command._fetch_messages in test_rate_limit_handling so the retry-success branch
can call Message.bulk_save safely (mirror how other tests in this file mock
Message).
🧹 Nitpick comments (41)
backend/tests/apps/owasp/api/internal/queries/post_test.py (1)
18-28: Weak test assertions and unnecessary mock setup.Two issues:
- The explicit
__getitem__assignment on Line 23 is unnecessary —Mockobjects already support slicing out of the box.- The test only asserts
mock_recent.calledbut never checks the return value ofquery.recent_posts(limit=5)or verifies the slice argument. This means the test would still pass even if the slicing logic were removed or broken.Consider asserting the actual result and verifying the slice:
♻️ Proposed improvement
def test_recent_posts_valid_limit(self): """Test recent_posts with valid limit.""" mock_posts = [Mock(), Mock()] with patch("apps.owasp.models.post.Post.recent_posts") as mock_recent: - mock_recent.return_value.__getitem__ = Mock(return_value=mock_posts) + mock_recent.return_value.__getitem__ = Mock(return_value=mock_posts) query = PostQuery() - query.recent_posts(limit=5) + result = query.recent_posts(limit=5) - assert mock_recent.called + mock_recent.assert_called_once() + assert result == mock_postsbackend/tests/apps/api/rest/v0/label_test.py (2)
35-46: Tests don't verify thatfilters.filter(LabelModel.objects.all())is called.The
list_labelfunction callsfilters.filter(LabelModel.objects.all()), but none of the three tests assert thatmock_filters.filterwas invoked withmock_label_model.objects.all(). This means the filter path is untested—only the ordering branch is exercised.Consider adding assertions like:
mock_label_model.objects.all.assert_called_once() mock_filters.filter.assert_called_once_with(mock_label_model.objects.all())
48-76:test_list_labels_with_orderingandtest_list_labels_with_desc_orderingare nearly identical — consider parameterizing.The two methods differ only in the ordering string. A
@pytest.mark.parametrizeover the ordering values would remove duplication and make it trivial to add more cases.♻️ Suggested refactor
- `@patch`("apps.api.rest.v0.label.LabelModel") - def test_list_labels_with_ordering(self, mock_label_model): - """Test list labels with ordering.""" - mock_request = MagicMock() - mock_filters = MagicMock() - mock_queryset = MagicMock() - mock_ordered_queryset = MagicMock() - mock_filters.filter.return_value = mock_queryset - mock_queryset.order_by.return_value = mock_ordered_queryset - - result = list_label(mock_request, mock_filters, ordering="nest_created_at") - - mock_queryset.order_by.assert_called_with("nest_created_at") - assert result == mock_ordered_queryset - - `@patch`("apps.api.rest.v0.label.LabelModel") - def test_list_labels_with_desc_ordering(self, mock_label_model): - """Test list labels with descending ordering.""" - mock_request = MagicMock() - mock_filters = MagicMock() - mock_queryset = MagicMock() - mock_ordered_queryset = MagicMock() - mock_filters.filter.return_value = mock_queryset - mock_queryset.order_by.return_value = mock_ordered_queryset - - result = list_label(mock_request, mock_filters, ordering="-nest_updated_at") - - mock_queryset.order_by.assert_called_with("-nest_updated_at") - assert result == mock_ordered_queryset + `@pytest.mark.parametrize`( + "ordering", + ["nest_created_at", "-nest_created_at", "nest_updated_at", "-nest_updated_at"], + ) + `@patch`("apps.api.rest.v0.label.LabelModel") + def test_list_labels_with_ordering(self, mock_label_model, ordering): + """Test list labels with ordering.""" + mock_request = MagicMock() + mock_filters = MagicMock() + mock_queryset = MagicMock() + mock_ordered_queryset = MagicMock() + mock_filters.filter.return_value = mock_queryset + mock_queryset.order_by.return_value = mock_ordered_queryset + + result = list_label(mock_request, mock_filters, ordering=ordering) + + mock_queryset.order_by.assert_called_with(ordering) + assert result == mock_ordered_querysetbackend/tests/apps/owasp/api/internal/nodes/chapter_test.py (2)
100-128: Consider adding a partial-coordinates edge case forgeo_location.The resolver condition is
root.latitude is not None and root.longitude is not None, so a case where only one coordinate isNonealso returnsNone. Adding a test for that branch (e.g.,latitude=40.7, longitude=None) would strengthen coverage of the short-circuit logic.🧪 Suggested additional test
+ def test_geo_location_resolver_with_partial_coordinates(self): + """Test geo_location resolver returns None when only one coordinate is set.""" + from unittest.mock import Mock + + mock_chapter = Mock() + mock_chapter.latitude = 40.7128 + mock_chapter.longitude = None + + field = self._get_field_by_name("geo_location", ChapterNode) + result = field.base_resolver.wrapped_func(None, mock_chapter) + + assert result is None
90-90: Hoistfrom unittest.mock import Mockto the module level.
Mockis imported identically inside six test methods. A single top-level import reduces noise and is the conventional pattern.♻️ Proposed refactor
Add at the top of the file:
from unittest.mock import MockThen remove all per-method
from unittest.mock import Mocklines.Also applies to: 102-102, 119-119, 132-132, 144-144, 156-156
backend/tests/apps/owasp/management/commands/owasp_aggregate_entity_contributions_test.py (1)
489-492: Weak assertion:filteris called but the key argument is never verified.
assert_called()only confirms the method was invoked, not thatkey="www-project-test"was actually forwarded to the filter. The same applies to the existingtest_handle_with_specific_keytest for chapters, but since you're adding a new test, consider tightening the assertion.💡 Suggested improvement
- mock_project_model.objects.filter.assert_called() + # Verify the key filter was actually applied + filter_kwargs = mock_project_model.objects.filter.call_args + assert "key" in filter_kwargs.kwargs or any( + "key" in str(c) for c in filter_kwargs + ), "Expected filter to be called with key argument"Alternatively, if the production code calls
.filter(key=key)directly, you can use:mock_project_model.objects.filter.assert_any_call(key="www-project-test")backend/tests/apps/owasp/models/managers/managers_test.py (1)
36-40: The Q object assertion is too weak — it doesn't verify the actual filter conditions.The test only checks
isinstance(q_obj, Q)and that one positional arg was passed. It doesn't verify the filter isQ(latitude__isnull=True) | Q(longitude__isnull=True). This test would still pass if someone changed the Q filter to any arbitrary condition (e.g.,Q(name="foo")).♻️ Proposed fix to assert the Q object content
args, _ = mock_qs.filter.call_args assert len(args) == 1 q_obj = args[0] assert isinstance(q_obj, Q) + expected_q = Q(latitude__isnull=True) | Q(longitude__isnull=True) + assert q_obj == expected_q assert result == mock_qs.filter.return_valuebackend/tests/apps/api/rest/v0/member_test.py (1)
57-69: Assert thatfilters.filteris called with the ordered queryset.Both
test_list_members_no_orderingandtest_list_members_with_orderingverify thatorder_byis called correctly and that the final result matches, but neither asserts thatmock_filters.filterwas actually invoked with the ordered queryset. Sincemock_filters.filter.return_valueis the samemock_queryset, theresult == mock_querysetcheck passes even iffilters.filteris never called or called with the wrong argument.Proposed fix (apply similarly to `test_list_members_with_ordering`)
mock_user_model.objects.order_by.assert_called_with("-created_at") + mock_filters.filter.assert_called_once_with(mock_queryset) assert result == mock_querysetbackend/tests/apps/api/rest/v0/organization_test.py (1)
64-76: Missingfilterassertion for consistency.
test_list_organization_no_orderingasserts thefiltercall but notorder_by; this test assertsorder_bybut notfilter. Both tests should verify both calls for consistent and thorough coverage.Proposed fix
mock_org_model.objects.filter.return_value.order_by.assert_called_with("updated_at") + mock_org_model.objects.filter.assert_called_with(is_owasp_related_organization=True) assert result == mock_querysetbackend/tests/apps/owasp/scraper_test.py (1)
323-334:caplogis used but the log message is never asserted.
caplog.at_level(logging.WARNING)is set up (andlogger.exceptionlogs at ERROR, which is captured), but unliketest_verify_url_logs_warning(line 142), this test never checkscaplog.text. Either assert on the log content for consistency or drop thecaplogfixture if only the return value matters.Option A: Assert the log message
with caplog.at_level(logging.WARNING): result = scraper.verify_url("https://example.org") assert result is None + assert "Request failed" in caplog.textOption B: Drop caplog if log assertion isn't needed
- def test_verify_url_request_exception(self, mock_session, mock_response, caplog): + def test_verify_url_request_exception(self, mock_session, mock_response): """Test verify_url handles request exceptions during verification.""" mock_session.get.return_value = mock_response scraper = OwaspScraper("https://test.org") mock_session.get.reset_mock() mock_session.get.side_effect = requests.exceptions.RequestException("Connection error") - with caplog.at_level(logging.WARNING): - result = scraper.verify_url("https://example.org") + result = scraper.verify_url("https://example.org") assert result is Nonebackend/tests/apps/slack/events/event_test.py (2)
191-219: Assertions could be more specific to catch regressions.Both
test_get_direct_messageandtest_get_ephemeral_messageonly assertisinstance(result, list). Sincerender_blockswith a non-None template and rendered text should produce at least one block, consider assertinglen(result) > 0and verifying the block structure (similar to what you do intest_render_blocks_with_text_section). This would make these tests more meaningful for catching regressions beyond just "returns a list."
242-259: Unusedmockerfixture parameter.
mockeris declared in bothtest_render_blocks_with_dividerandtest_render_blocks_with_text_sectionbut never used —MagicMock()is used directly instead. You can drop it from the signatures.Proposed fix
- def test_render_blocks_with_divider(self, mocker, event_instance): + def test_render_blocks_with_divider(self, event_instance):- def test_render_blocks_with_text_section(self, mocker, event_instance): + def test_render_blocks_with_text_section(self, event_instance):backend/tests/apps/slack/models/message_test.py (2)
191-228: Consider asserting exact expected output rather than only absence checks.Several
cleaned_texttests only verify that unwanted tokens are absent (e.g.,"👋" not in result," " not in result) without asserting the exact cleaned string. This makes the tests pass even if the cleaning logic produces unexpected artifacts. Pinning the expected output (e.g.,assert result == "Hello World") would catch regressions more reliably.Example for whitespace normalization test
def test_cleaned_text_normalizes_whitespace(self): """Test cleaned_text normalizes multiple whitespaces.""" message = Message(raw_data={"text": "Hello World"}) result = message.cleaned_text - assert " " not in result + assert result == "Hello World"
266-282:assert_called_once()without argument verification weakens the test.
mock_filter.assert_called_once()confirms the filter was invoked but not with what arguments. Sincelatest_replyis supposed to filter byconversation=self.conversationandparent_message=self, verifying the kwargs would guard against accidental changes to the query.Proposed stronger assertion
result = message.latest_reply assert result == mock_reply - mock_filter.assert_called_once() + mock_filter.assert_called_once_with( + conversation=mock_conversation, + parent_message=message, + )backend/tests/apps/core/utils/index_test.py (1)
46-91: Good coverage ofdeep_camelize— consider one falsy-value edge case.The source implementation uses
if not obj: return obj, which means falsy non-empty values like0andFalseshort-circuit before reaching the scalar return at the bottom. Yourtest_deep_camelize_non_dict_non_listonly tests truthy scalars ("string",123). Adding a check likeassert deep_camelize(0) == 0would cover that branch more precisely.This is a minor gap and won't affect coverage numbers, so feel free to skip.
Optional: add falsy scalar assertion
def test_deep_camelize_non_dict_non_list(self): """Test that non-dict/list values are returned as-is.""" assert deep_camelize("string") == "string" assert deep_camelize(123) == 123 + assert deep_camelize(0) == 0 + assert deep_camelize(False) is Falsebackend/tests/apps/api/rest/v0/structured_search_test.py (4)
142-154:side_effecton the class constructor doesn't match the real error path.In the source,
QueryParserErroris raised byparser.parse(query), not by theQueryParser(...)constructor. Settingmock_parser_class.side_effectmakes the constructor itself raise the exception. This still exercises theexcept QueryParserErrorbranch (since both calls are inside thetry), so the test passes, but it doesn't reflect a realistic scenario.Mock
.parse()instead for a more faithful test:Suggested fix
qs = make_queryset() with patch("apps.api.rest.v0.structured_search.QueryParser") as mock_parser_class: - mock_parser_class.side_effect = QueryParserError("Test error") + mock_parser = MagicMock() + mock_parser_class.return_value = mock_parser + mock_parser.parse.side_effect = QueryParserError("Test error") result = apply_structured_search(qs, "name:test", FIELD_SCHEMA)
99-109: Test only asserts the negative; add a positive assertion.The test verifies
name__icontainsis absent but never checks what lookup is used. With"lookup": "exact", theSTRING_LOOKUP_MAPPINGshould produce a barename(orname__exact) lookup. A positive assertion would actually validate the exact-match path rather than just ruling out one alternative.Suggested addition
qs.filter.assert_called_once() args, _ = qs.filter.call_args assert "name__icontains" not in str(args[0]) + assert "name" in str(args[0])
80-87: Filter assertions after skipped conditions could be stronger.In
test_invalid_number_value_is_skipped,test_condition_field_not_in_schema_is_skipped, andtest_number_field_with_none_value_is_skipped, the tests confirmfilterwas called but the Q object was essentially empty. Consider asserting the Q object directly (e.g.,args[0] == Q()) to make the intent explicit—"filter was called with no actual predicates."Also applies to: 157-168, 190-203
144-146: Minor:patchandQueryParserErrorimported inside individual test functions.
MagicMockandpatchare both fromunittest.mock, yetpatchis imported locally in several functions whileMagicMockis at the top of the file. Movingfrom unittest.mock import patchto the top-level import block (line 3) would reduce duplication. Same applies to theQueryParserErrorimport on line 146 if it's used in multiple tests.Also applies to: 159-159, 173-173, 192-192
backend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.py (1)
116-118:QShelper class and mock-repository setup are duplicated three times.Consider extracting the
QSclass to module level (or a fixture) and the common mock-repository construction into a@pytest.fixtureto reduce duplication. This would make adding future test scenarios cheaper and less error-prone.Also applies to: 164-166
backend/tests/apps/owasp/management/commands/owasp_update_leaders_test.py (1)
72-72: Prefer strictis Truefor the filter kwarg assertion.The
member__isnullfilter kwarg should be exactlyTrue(a boolean). Using a truthy assertion would also pass for1,"yes", or any other truthy value, weakening the test's precision.Suggested fix
- assert filter_call[1]["member__isnull"] + assert filter_call[1]["member__isnull"] is Truebackend/tests/apps/slack/utils_test.py (1)
293-305: Shallow assertion — consider verifying the returned data.Same as the sponsors test:
assert result is not Nonedoesn't confirm the mock data was actually returned. Consider asserting on the value or verifying the slice call.backend/tests/apps/api/rest/v0/repository_test.py (2)
94-104: Missing assertion on.get()call arguments.The test verifies
select_related("organization")but doesn't assert that.get()was called with the expected filter kwargs (organization__login__iexact="OWASP",name__iexact="Nest"). Without this, the test wouldn't catch regressions in the query filter logic.Proposed fix
mock_repo_model.objects.select_related.assert_called_with("organization") + mock_repo_model.objects.select_related.return_value.get.assert_called_once_with( + organization__login__iexact="OWASP", + name__iexact="Nest", + ) assert result == mock_repo
55-69: Missingselect_relatedassertion in list tests.Both list tests set up
mock_repo_model.objects.select_related.return_valuebut neither asserts thatselect_related("organization")was called. Theget_repositorytest does this (line 103); it would be consistent to verify it here too.Proposed addition for test_list_repository_without_filter
+ mock_repo_model.objects.select_related.assert_called_with("organization") mock_queryset.order_by.assert_called_with("-created_at", "-updated_at")backend/tests/apps/owasp/admin/mixins_test.py (2)
170-191:@patchdecorator + pytestmockerfixture argument ordering is correct but verify the assertions capture values, not just keys.The test correctly validates that
filteris called with the expectedidand thatquerysetandinitialappear in the kwargs forwarded tosuper(). For stronger confidence you could also assert the values of those kwargs (e.g.,call_kwargs["queryset"]is the filtered queryset mock andcall_kwargs["initial"]equalsmock_conversation_ct.id), but this is fine for a coverage-oriented PR.🔧 Optional: tighten value assertions
call_kwargs = mock_parent.call_args[1] assert "queryset" in call_kwargs assert "initial" in call_kwargs + assert call_kwargs["queryset"] == mock_content_type.objects.filter.return_value + assert call_kwargs["initial"] == mock_conversation_ct.id
162-168: Use explicit parent class name instead of__bases__[0].Currently
EntityChannelInline.__bases__[0]correctly resolves toGenericTabularInline, but this pattern is fragile. If another base class or mixin is added beforeGenericTabularInlinein the inheritance list,__bases__[0]will silently patch the wrong class. Replace with explicit reference:patch.object(GenericTabularInline, "formfield_for_dbfield")to make the intent clear and the test resilient to future changes.backend/tests/apps/owasp/management/commands/owasp_create_member_snapshot_test.py (4)
505-838: Heavy mock setup duplication across allTestHandleMethodtests.Every test in this class repeats a near-identical ~30-line block for mocking
User,MemberSnapshot,Commit,PullRequest,Issue,MemberProfile, and thegenerate_*helpers. Consider extracting the common setup into shared fixtures or a helper method, then having each test override only the specifics it cares about.For example, you could create a
base_mocksfixture that yields a namespace/dict of all the pre-configured mocks, letting individual tests tweak just the parts they need:♻️ Sketch of a shared fixture approach
`@pytest.fixture` def base_mocks(self, command, mocker): target = self.target_module mocks = types.SimpleNamespace() mocks.user_cls = mocker.patch(f"{target}.User") mocks.user_instance = mock.MagicMock() mocks.user_cls.objects.get.return_value = mocks.user_instance mocks.snapshot_cls = mocker.patch(f"{target}.MemberSnapshot") mocks.snapshot_cls.objects.filter.return_value.first.return_value = None mocks.snapshot = mock.MagicMock( id=1, total_contributions=0, commits_count=0, pull_requests_count=0, issues_count=0, messages_count=0, ) mocks.snapshot_cls.objects.create.return_value = mocks.snapshot for name in ("Commit", "PullRequest", "Issue"): cls = mocker.patch(f"{target}.{name}") cls.objects.filter.return_value.count.return_value = 0 mocks.profile_cls = mocker.patch(f"{target}.MemberProfile") mocks.profile_cls.DoesNotExist = Exception mocks.profile_cls.objects.get.side_effect = mocks.profile_cls.DoesNotExist for helper in ( "generate_heatmap_data", "generate_entity_contributions", "generate_repository_contributions", "generate_communication_heatmap_data", ): mocker.patch.object(command, helper, return_value={}) return mocks
841-964: Same duplication pattern inTestGenerateEntityContributionsChapter.All four tests instantiate
Command()directly and repeat the same mock setup for user, commits/prs/issues querysets, and theChapter/ContentType/EntityMemberpatch block. A shared fixture and helper would clean this up significantly — same recommendation as forTestHandleMethod.Additionally, note that the existing test
test_generate_entity_contributions_filters_by_date_rangeinTestOwaspCreateMemberSnapshotCommand(Line 131) follows the same pattern for the "project" entity type. Consolidating these into a parameterized test or shared helpers could further reduce duplication.
966-1019: Test name and docstring don't match the actual scenario tested.The method is named
test_generate_entity_contributions_pr_outside_date_rangeand the docstring says "created_at = None or outside range", but the test only exercisescreated_at = None(Line 976). Similarly for the issue variant below (Line 1031). These are two distinct edge cases —Noneskips date comparison entirely, while an out-of-range date evaluates the comparison and fails. Consider either:
- Renaming to
..._pr_created_at_none, or- Parameterizing to cover both
Noneand an actual out-of-range date.
612-661: Mocked querysets return empty iterators despite non-zero counts, weakening assertions.
count()returns 5/3/2 but__iter__yields nothing (Lines 633, 639, 645), sosnapshot.commits.add()is called with zero arguments. The assertionassert_called()(Line 659) passes vacuously. If the intent is to verify that contributions are actually linked to the snapshot, provide mock objects in the iterators and useassert_called_with(...)or at leastassert_called_once()to strengthen coverage.backend/tests/apps/slack/management/commands/owasp_match_channels_test.py (1)
129-213: Consider extracting shared mock setup into a fixture or helper.The four
test_handle_*methods repeat nearly identical boilerplate for patchingConversation.objects,Chapter.objects,Committee.objects,Project.objects,EntityChannel.objects, andContentType.objects.get_for_model. A shared pytest fixture (or a helper method) would reduce duplication and make each test focus on its unique setup.backend/apps/slack/management/commands/slack_sync_messages.py (1)
454-456: Useself.stdout.writeinstead ofprint().Lines 456 and 549 use bare
print()for user-facing output. This bypasses Django's command output infrastructure and makes the output invisible to tests that mockself.stdout. Based on learnings, in Django management commands, preferself.stdout.write(...)overprint(...)for user-facing stdout output to align with Django conventions and improve testability.♻️ Proposed fix
- print("Fetching message replies...") + self.stdout.write("Fetching message replies...")And similarly at line 549:
- print( - f"Saving {replies_count} repl{pluralize(replies_count, 'y,ies')} for {message.url}" - ) + self.stdout.write( + f"Saving {replies_count} repl{pluralize(replies_count, 'y,ies')} for {message.url}" + )backend/tests/apps/slack/management/commands/slack_sync_messages_test.py (2)
682-724: Redundant firstWorkspacepatch on line 685.Line 685 patches
Workspacewith an anonymousMagicMock, but line 695 immediately re-patches it withmock_workspace. The first patch and themock_querysetsetup on lines 689-691 only work becausemock_querysetis later wired tomock_workspaceon line 694. The initial patch at line 685 is unused.♻️ Proposed simplification — remove the redundant first patch
def test_date_filtering_and_pagination(self, mocker): """Test date filtering logic: skip future, process current, break on past.""" mocker.patch(f"{self.target_module}.os.environ.get", return_value="token") - mocker.patch(f"{self.target_module}.Workspace") mock_conversation_cls = mocker.patch(f"{self.target_module}.Conversation") mock_conversation_cls.objects.get_or_create.return_value = (MagicMock(), True) mock_msg_model = mocker.patch(f"{self.target_module}.Message") mock_queryset = MagicMock() mock_queryset.exists.return_value = True mock_queryset.__iter__.return_value = [MagicMock()] mock_workspace = MagicMock() mock_workspace.objects.all.return_value = mock_queryset mocker.patch(f"{self.target_module}.Workspace", mock_workspace)
699-701: Add inline comments mapping timestamps to expected dates for readability.These epoch values are opaque. Brief inline comments would make the test self-documenting and help future maintainers verify the date-filtering assertions without an external converter.
♻️ Proposed fix
- ts_future = "1704196800" - ts_current = "1704110400" - ts_past = "1704024000" + ts_future = "1704196800" # 2024-01-02 08:00 UTC (after end_at) + ts_current = "1704110400" # 2024-01-01 08:00 UTC (within range) + ts_past = "1704024000" # 2023-12-31 08:00 UTC (before start_at)backend/tests/apps/owasp/api/internal/queries/chapter_test.py (1)
64-70: Unusedmock_infofixture parameter.
mock_infois accepted but never referenced in the test body. This is also the case for the existingtest_recent_chapters_queryabove, but since you're adding new code, consider cleaning it up here.Proposed fix
- def test_recent_chapters_with_invalid_limit(self, mock_info): + def test_recent_chapters_with_invalid_limit(self):backend/tests/apps/owasp/api/internal/queries/event_test.py (1)
18-28: Test doesn't assert the return value ofupcoming_events.
query.upcoming_events(limit=5)is called but its result is discarded. The test only checks thatmock_upcoming.calledis truthy. Consider capturing and asserting the result to verify the slicing behavior actually produces the expected output.Suggested improvement
with patch("apps.owasp.models.event.Event.upcoming_events") as mock_upcoming: mock_upcoming.return_value.__getitem__ = Mock(return_value=mock_events) query = EventQuery() - query.upcoming_events(limit=5) + result = query.upcoming_events(limit=5) - assert mock_upcoming.called + assert mock_upcoming.called + assert result == mock_eventsbackend/tests/apps/owasp/api/internal/nodes/project_test.py (2)
153-164: MoveMagicMockimport to the module level.
from unittest.mock import MagicMockis repeated inside every test method in this class (10 times). It should be a single import at the top of the file alongside the other imports.Proposed fix
Add to the top-level imports (e.g., after line 2):
+from unittest.mock import MagicMock +Then remove all inline
from unittest.mock import MagicMockstatements within the test methods.
146-151: Consider extracting_get_resolverintoGraphQLNodeBaseTest.This helper is duplicated identically in
TestProjectNodeResolvers,TestProjectHealthMetricsNodeResolvers, andTestSnapshotNodeResolvers. SinceGraphQLNodeBaseTestalready provides_get_field_by_name, it would be natural to add a_get_resolvermethod there to avoid repetition.backend/tests/apps/owasp/models/project_health_metrics_test.py (3)
126-239: Consider parametrizing the requirement property tests to reduce duplication.All 10 tests follow the exact same with/without pattern differing only in the attribute name and expected value. A single parametrized test pair would cover all five requirement properties with far less boilerplate:
♻️ Suggested refactor
- def test_age_days_requirement_with_requirements(self): - """Test age_days_requirement returns value when requirements exist.""" - mock_requirements = MagicMock() - mock_requirements.age_days = 30 - ... - def test_age_days_requirement_without_requirements(self): - ... - # (repeated for each field) + `@pytest.mark.parametrize`( + ("requirement_attr", "mock_value", "property_name"), + [ + ("age_days", 30, "age_days_requirement"), + ("last_commit_days", 14, "last_commit_days_requirement"), + ("last_pull_request_days", 21, "last_pull_request_days_requirement"), + ("last_release_days", 90, "last_release_days_requirement"), + ("owasp_page_last_update_days", 60, "owasp_page_last_update_days_requirement"), + ], + ) + def test_requirement_with_requirements(self, requirement_attr, mock_value, property_name): + mock_requirements = MagicMock() + setattr(mock_requirements, requirement_attr, mock_value) + metrics = ProjectHealthMetrics() + with patch.object( + ProjectHealthMetrics, + "project_requirements", + new_callable=lambda: property(lambda _: mock_requirements), + ): + assert getattr(metrics, property_name) == mock_value + + `@pytest.mark.parametrize`( + "property_name", + [ + "age_days_requirement", + "last_commit_days_requirement", + "last_pull_request_days_requirement", + "last_release_days_requirement", + "owasp_page_last_update_days_requirement", + ], + ) + def test_requirement_without_requirements(self, property_name): + metrics = ProjectHealthMetrics() + with patch.object( + ProjectHealthMetrics, + "project_requirements", + new_callable=lambda: property(lambda _: None), + ): + assert getattr(metrics, property_name) == 0
274-274: Redundant import —Projectis already imported at the top of the file (line 7).def test_project_requirements_returns_requirements(self, mock_requirements_model): """Test project_requirements returns requirements for project level.""" - from apps.owasp.models.project import Project - mock_requirements = MagicMock()
257-265: Assertingmock_filter.call_count == 2is fragile.This assertion couples the test to the number of internal
.filter()calls in the implementation rather than the observable outcome. If the implementation is refactored (e.g., chaining is reorganized), this test breaks without any behavior change. Consider asserting on the returned queryset and verifying key filter arguments instead.
backend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.py
Show resolved
Hide resolved
backend/tests/apps/slack/management/commands/owasp_match_channels_test.py
Outdated
Show resolved
Hide resolved
backend/tests/apps/slack/management/commands/slack_sync_messages_test.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/tests/apps/owasp/models/project_health_metrics_test.py`:
- Around line 273-288: Remove the redundant local import of Project inside the
test_project_requirements_returns_requirements test: the Project class is
already imported at module scope, so delete the line "from
apps.owasp.models.project import Project" in that test and keep using the
existing Project symbol; ensure the test still constructs
Project(level="FLAGSHIP") and runs the same assertions.
🧹 Nitpick comments (8)
backend/tests/apps/owasp/api/internal/nodes/chapter_test.py (1)
92-92: Consider hoistingfrom unittest.mock import Mockto module level.
Mockis imported locally in every test method (and in pre-existing tests too). Moving it to a single top-level import would reduce repetition across all tests in this file.Proposed diff
"""Test cases for ChapterNode.""" import math +from unittest.mock import Mock from apps.owasp.api.internal.nodes.chapter import ChapterNode from tests.apps.common.graphql_node_base_test import GraphQLNodeBaseTestThen remove the
from unittest.mock import Mocklines inside each test method.Also applies to: 104-104, 121-121, 134-134, 146-146, 158-158
backend/tests/apps/api/rest/v0/committee_test.py (1)
108-116: Consider asserting the filter call in the not-found test.The not-found test verifies the response status but doesn't assert that
filterwas called with the expected arguments (key__iexact="www-committee-NonExistent"). The other two test methods do assert on the filter call, so this is a minor consistency gap.♻️ Suggested addition
result = get_committee(mock_request, "NonExistent") + mock_committee_model.active_committees.filter.assert_called_with( + is_active=True, key__iexact="www-committee-NonExistent" + ) assert result.status_code == HTTPStatus.NOT_FOUNDbackend/tests/apps/owasp/models/project_health_metrics_test.py (2)
127-240: Consider parametrizing these repetitive requirement tests.All 10 tests follow an identical pattern — only the attribute name and value differ. This can be collapsed into two parametrized tests (
_with_requirements/_without_requirements), greatly reducing boilerplate.♻️ Proposed refactor using parametrize
+ `@pytest.mark.parametrize`( + ("requirement_attr", "expected_value"), + [ + ("age_days", 30), + ("last_commit_days", 14), + ("last_pull_request_days", 21), + ("last_release_days", 90), + ("owasp_page_last_update_days", 60), + ], + ) + def test_requirement_with_requirements(self, requirement_attr, expected_value): + """Test *_requirement property returns value when requirements exist.""" + mock_requirements = MagicMock() + setattr(mock_requirements, requirement_attr, expected_value) + + metrics = ProjectHealthMetrics() + with patch.object( + ProjectHealthMetrics, + "project_requirements", + new_callable=lambda: property(lambda _: mock_requirements), + ): + assert getattr(metrics, f"{requirement_attr}_requirement") == expected_value + + `@pytest.mark.parametrize`( + "requirement_attr", + [ + "age_days", + "last_commit_days", + "last_pull_request_days", + "last_release_days", + "owasp_page_last_update_days", + ], + ) + def test_requirement_without_requirements(self, requirement_attr): + """Test *_requirement property returns 0 when no requirements.""" + metrics = ProjectHealthMetrics() + with patch.object( + ProjectHealthMetrics, + "project_requirements", + new_callable=lambda: property(lambda _: None), + ): + assert getattr(metrics, f"{requirement_attr}_requirement") == 0This replaces all 10 individual test methods while preserving the exact same coverage.
258-266: Assertion oncall_countalone is fragile — consider verifying filter arguments.
assert mock_filter.call_count == 2will pass for any two.filter()calls regardless of arguments. If the production code changes which filters are applied, this test won't catch the regression. Consider asserting the actual call args (or at minimum checkingmock_filter.call_args_list) to make the test more meaningful.backend/tests/apps/slack/management/commands/slack_sync_messages_test.py (2)
683-725: RedundantWorkspacepatch — the second one silently overrides the first.Line 686 patches
Workspace, then line 696 patches it again with a different mock. Only the second patch takes effect. Remove the first to avoid confusion.♻️ Suggested fix
def test_date_filtering_and_pagination(self, mocker): """Test date filtering logic: skip future, process current, break on past.""" mocker.patch(f"{self.target_module}.os.environ.get", return_value="token") - mocker.patch(f"{self.target_module}.Workspace") mock_conversation_cls = mocker.patch(f"{self.target_module}.Conversation") mock_conversation_cls.objects.get_or_create.return_value = (MagicMock(), True) mock_msg_model = mocker.patch(f"{self.target_module}.Message")
603-614: Consider extracting a helper to build theSlackApiErrorrate-limit mock.The same pattern — construct
SlackApiError, override.responsewith aMagicMock, wire up__getitem__,.get, and.headers— is duplicated in at least four tests. A small factory function (e.g.,_make_rate_limit_error(retry_after="1")) at module or class level would reduce boilerplate and make the tests easier to maintain.Also applies to: 635-646, 255-266, 294-305
backend/tests/apps/owasp/models/common_test.py (2)
454-474: Consider consolidating into a single parametrized test.Both tests exercise the same
if not prompt: returnearly-exit path ingenerate_summary. They could be merged into one@pytest.mark.parametrizetest with("", None)as inputs, reducing duplication.♻️ Suggested consolidation
- def test_generate_summary_empty_prompt(self): - """Test generate_summary returns early when prompt is empty.""" - model = EntityModel() - model.summary = "existing" - mock_open_ai = MagicMock() - - model.generate_summary(prompt="", open_ai=mock_open_ai) - - mock_open_ai.set_input.assert_not_called() - assert model.summary == "existing" - - def test_generate_summary_none_prompt(self): - """Test generate_summary returns early when prompt is None.""" - model = EntityModel() - model.summary = "existing" - mock_open_ai = MagicMock() - - model.generate_summary(prompt=None, open_ai=mock_open_ai) - - mock_open_ai.set_input.assert_not_called() - assert model.summary == "existing" + `@pytest.mark.parametrize`("prompt", ["", None]) + def test_generate_summary_falsy_prompt(self, prompt): + """Test generate_summary returns early when prompt is falsy.""" + model = EntityModel() + model.summary = "existing" + mock_open_ai = MagicMock() + + model.generate_summary(prompt=prompt, open_ai=mock_open_ai) + + mock_open_ai.set_input.assert_not_called() + assert model.summary == "existing"
476-502: Consider asserting ontagsas well.
from_githubalso callsself.parse_tags(...)with the metadata tags["tag1", "tag2"]. Addingassert model.tags == ["tag1", "tag2"]would strengthen the test by verifying the full side-effect surface of the method under test.
|
@cubic-dev-ai review this pull request |
@HarshitVerma109 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
3 issues found across 48 files
Confidence score: 4/5
- This looks safe to merge overall; issues are limited to test quality/documentation rather than runtime behavior.
- The weakest point is the assertion in
backend/tests/apps/slack/management/commands/owasp_match_channels_test.pythat can pass without validating expected matches, which could let regressions slip through. - Pay close attention to
backend/tests/apps/slack/management/commands/owasp_match_channels_test.py,backend/tests/apps/api/rest/v0/structured_search_test.py,backend/tests/apps/owasp/api/internal/queries/post_test.py- ensure assertions and docstrings reflect the intended behaviors.
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/slack/management/commands/owasp_match_channels_test.py">
<violation number="1" location="backend/tests/apps/slack/management/commands/owasp_match_channels_test.py:125">
P2: Weak test assertion could pass without validating the expected behavior. `assert len(matches) <= 1` passes when `matches` is empty, and the conditional `if matches:` block means no validation occurs if the matching fails. Consider using `assert len(matches) == 1` to ensure the valid conversation is actually matched.</violation>
</file>
<file name="backend/tests/apps/api/rest/v0/structured_search_test.py">
<violation number="1" location="backend/tests/apps/api/rest/v0/structured_search_test.py:191">
P2: Misleading docstring: says "triggers TypeError" but the test verifies the field is skipped. Update the docstring to match the actual test behavior.</violation>
</file>
<file name="backend/tests/apps/owasp/api/internal/queries/post_test.py">
<violation number="1" location="backend/tests/apps/owasp/api/internal/queries/post_test.py:26">
P3: This test doesn’t assert the returned value or that the limit slice was applied, so it won’t catch regressions where `recent_posts` ignores `limit` or returns the wrong data. Add assertions on the result and slice call to validate behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
backend/tests/apps/slack/management/commands/owasp_match_channels_test.py
Outdated
Show resolved
Hide resolved
|
@cubic-dev-ai review this pull request |
@HarshitVerma109 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 48 files
Confidence score: 5/5
- Only issue is a minor test cleanup in
backend/tests/apps/owasp/scraper_test.pywherecaplogis unused, so user-facing risk appears low - Severity is moderate but limited to test hygiene, so overall merge risk is low
- Pay close attention to
backend/tests/apps/owasp/scraper_test.py- remove unusedcaplogor add an assertion to validate logged output.
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/owasp/scraper_test.py">
<violation number="1" location="backend/tests/apps/owasp/scraper_test.py:331">
P2: Test uses `caplog` fixture but doesn't verify any log output. Either add an assertion like `assert "Connection error" in caplog.text` or similar, or remove the unused `caplog` fixture and `with caplog.at_level()` context manager if no log verification is intended.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@cubic-dev-ai review this pull request |
@HarshitVerma109 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 48 files
Confidence score: 4/5
- This PR looks safe to merge with minimal risk; the noted issues are test-only mismatches rather than production code defects.
- Most severe item is a failing/misaligned assertion in
backend/tests/apps/owasp/scraper_test.pywhere the expected log message doesn't matchverify_urlbehavior, which could cause test failures. backend/tests/apps/api/rest/v0/structured_search_test.pyhas a misleading test name/docstring that may confuse intent but shouldn’t affect runtime behavior.- Pay close attention to
backend/tests/apps/owasp/scraper_test.py,backend/tests/apps/api/rest/v0/structured_search_test.py- align assertions and naming with actual behavior.
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/owasp/scraper_test.py">
<violation number="1" location="backend/tests/apps/owasp/scraper_test.py:335">
P2: Test assertion doesn't match actual implementation. The `verify_url` method logs `"Request failed"` via `logger.exception()` (ERROR level), not `"Couldn't verify URL"` or `"Connection error"`. The test should capture at ERROR level and assert on the correct message.</violation>
</file>
<file name="backend/tests/apps/api/rest/v0/structured_search_test.py">
<violation number="1" location="backend/tests/apps/api/rest/v0/structured_search_test.py:171">
P2: The test name and docstring are misleading - the schema has `"type": "boolean"` but the name suggests testing a field with no type. Consider renaming to something like `test_boolean_field_uses_no_lookup_suffix` to accurately reflect what's being tested.
(Based on your team's feedback about maintaining consistent and accurate naming.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@cubic-dev-ai review this pull request |
@HarshitVerma109 I have started the AI code review. It will take a few minutes to complete. |
|
@arkid15r , please review it and let me know if any changes are required. |
arkid15r
dismissed stale reviews from cubic-dev-ai[bot] and coderabbitai[bot]
via
a4ee6cf
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3840 +/- ##
==========================================
+ Coverage 90.24% 93.68% +3.44%
==========================================
Files 463 463
Lines 14416 14419 +3
Branches 1939 1939
==========================================
+ Hits 13009 13508 +499
+ Misses 988 535 -453
+ Partials 419 376 -43
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 43 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
Sure, I'd be happy to help with #2297. I'll take a look and start working on it. |
|
|
I was just about to push the changes, but you already pushed them. Thank you! |
|
And I also fixed slow backend test execution times for 5 files in this PR. |
* Run make update * Clean up snapshot generated videos * Update backend/data/nest.dump * feat(ui): revamp corporate supporters carousel (Infinite Marquee + Dark Mode fix) (#3837) * feat(ui): revamp corporate supporters carousel (Infinite Marquee + Dark Mode fix) * fix: resolve failing test case * fix: add fallback text for unnamed sponsors * docs: add docstrings to satisfy coverage requirements * Run make check and fix tests. --------- Co-authored-by: Kate <kate@kgthreads.com> * Fix/redundant typescript assertion (#3834) * Fix Sonar S4325 by narrowing session user fields instead of casting * Fix unused ExtendedSession in mentorship page * fix: redundant-typescript-assertion * Fix stale latest date displayed in Project Health Dashboard metrics (#3842) * Fixed latest date in proejct health dashboard * updated order * Update code * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * feat: improve backend test coverage to 96% (#3840) * feat: improve backend test coverage to 96% * fix comments * fix issues * fix issue * fix cubic-dev-ai comments * Update code * Fix tests --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Fix: merge consecutive RUN instructions in frontend Dockerfile (#3644) * Fix: merge consecutive RUN instructions in frontend Dockerfile * fix: comment Dockerfile note to prevent syntax error * Update code * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Fix 'is_merged' not being available on the Issue (#3843) * Fix 'is_merged' not being available on the Issue * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * CI: Add ansible-lint workflow for Ansible playbooks (#3796) * ci: add ansible-lint workflow Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * Update .github/workflows/lint-ansible.yaml Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * ci: add ansible-lint make target and workflow Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * ci: add ansible-lint pre-commit hook Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * fix: whitespace & version Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * Update Makefile Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * ci: enable ansible-lint scanning and add requirements.yml Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * chore(ansible):align linting and module usage Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * ci(ansible): install collections before deploy playbooks Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * Update code * Update code * Update .github/workflows/run-ci-cd.yaml --------- Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * Fix ElevenLabs API error (#3861) * use default liam voice * bump speed by 0.10 --------- Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Add Ime Iyonsi to MENTORS.md (#3866) * Add mentor profile for Ime Iyonsi Added Ime Iyonsi's mentor profile. * Fix GitHub link for Ime Iyonsi Corrected GitHub link for Ime Iyonsi. * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * Update MENTORS.md * Enabled Strict Mode (#3776) * Enabled Strict Mode * fixed ai review * fix * fixed review * fix * update test * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Resolve case-sensitivity in QueryParser to support Chapters/Members search (#3844) * resolve query parser blocker * use case_sensitive flag in QueryParser * feat: add case_sensitive option to QueryParser and update tests * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Update dependencies (#3874) * Update dependencies * Bump django-ninja version * fix(proxy): pin nginx and certbot images (#3848) * fix(proxy): pin nginx and certbot images Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * fix stable verssions Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> --------- Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Update docker-compose/proxy/compose.yaml * Update backend/pyproject.toml * Update ansible lint configuration (#3880) * Update .github/ansible/.ansible-lint.yaml * Improve frontend test coverage above 80% and add missing test files (#3864) * Imrove test coverage to 80% and added test * Fixed coderabbit review * update code * fixed coderabbit ai * fixed soanrqube warning * fixed review * update * fixed aloglia cache_key (#3825) * fixed aloglia cache_key * change separator val to be semicolon (;) * Update code * add tests + use json filters * add trailing newline * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * fix: remove unused className prop from AnchorTitle component (#3822) * fix: remove unused className prop from AnchorTitle component Fixes #3805 The className prop was defined in AnchorTitleProps but never used in the component implementation. Removing it resolves Sonar rule typescript:S6767 and improves code maintainability. * fix: use className prop instead of removing it - Added className back to AnchorTitleProps interface - Accept className parameter in component - Apply className to root div element - Resolves reviewer feedback on PR #3822 * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> --------- Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Yashraj Pahuja <yashrajpahuja9999@gmail.com> Co-authored-by: Kate <kate@kgthreads.com> Co-authored-by: CodeAritraDhank <aritradhank21@gmail.com> Co-authored-by: Anurag Yadav <143180737+anurag2787@users.noreply.github.com> Co-authored-by: Harshit Verma <harshit1092004@gmail.com> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> Co-authored-by: Shuban Mutagi <shubanmutagi55@gmail.com> Co-authored-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Co-authored-by: emaybu <152900874+emaybu@users.noreply.github.com> Co-authored-by: sai chethana <saichethanavesireddy@gmail.com> Co-authored-by: Rahul Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> Co-authored-by: Lavanya <lavanyayadawad30@gmail.com>
2 participants
Proposed change
Resolves #3811
Improved backend test coverage to 96.08%
Checklist
make check-testlocally: all warnings addressed, tests passed