-
Notifications
You must be signed in to change notification settings - Fork 0
Feat: Add unit tests for Code Audit Agent #35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds error-path unit tests for fetch_repo_metrics in the CodeAuditAgent: covers HTTP 404s, network request errors, unsupported hosts, and invalid GitHub/GitLab URL formats, asserting zeroed metrics and "N/A" releases. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/services/agents/tests/test_code_audit_agent.py (1)
106-113: Add missingjsonparameter to Response mocks.Lines 108, 109, and 112 are missing the
jsonparameter in the Response mocks, while the similar test at lines 24-31 correctly includesjson=[]. The implementation calls.json()as a fallback in theparse_link_headerhelper, which could fail without valid JSON content.Apply this diff to add the missing parameters:
# Mock GitHub API calls for fetch_repo_metrics - respx.get(f"https://api.github.com/repos/{owner}/{repo}/commits?per_page=1").mock(return_value=Response(200, headers={'link': '<https://api.github.com/repositories/1296269/commits?per_page=1&page=2>; rel="next", <https://api.github.com/repositories/1296269/commits?per_page=1&page=10>; rel="last"'})) - respx.get(f"https://api.github.com/repos/{owner}/{repo}/contributors?per_page=1").mock(return_value=Response(200, headers={'link': '<https://api.github.com/repositories/1296269/contributors?per_page=1&page=2>; rel="next", <https://api.github.com/repositories/1296269/contributors?per_page=1&page=5>; rel="last"'})) + respx.get(f"https://api.github.com/repos/{owner}/{repo}/commits?per_page=1").mock(return_value=Response(200, headers={'link': '<https://api.github.com/repositories/1296269/commits?per_page=1&page=2>; rel="next", <https://api.github.com/repositories/1296269/commits?per_page=1&page=10>; rel="last"'}, json=[])) + respx.get(f"https://api.github.com/repos/{owner}/{repo}/contributors?per_page=1").mock(return_value=Response(200, headers={'link': '<https://api.github.com/repositories/1296269/contributors?per_page=1&page=2>; rel="next", <https://api.github.com/repositories/1296269/contributors?per_page=1&page=5>; rel="last"'}, json=[])) respx.get(f"https://api.github.com/repos/{owner}/{repo}/releases/latest").mock(return_value=Response(200, json={'tag_name': 'v1.0.0'})) respx.get(f"https://api.github.com/search/issues?q=repo%3A{owner}%2F{repo}%2Btype%3Aissue&per_page=1").mock(return_value=Response(200, json={'total_count': 20})) - respx.get(f"https://api.github.com/repos/{owner}/{repo}/pulls?state=all&per_page=1").mock(return_value=Response(200, headers={'link': '<https://api.github.com/repositories/1296269/pulls?state=all&per_page=1&page=2>; rel="next", <https://api.github.com/repositories/1296269/pulls?state=all&per_page=1&page=15>; rel="last"'})) + respx.get(f"https://api.github.com/repos/{owner}/{repo}/pulls?state=all&per_page=1").mock(return_value=Response(200, headers={'link': '<https://api.github.com/repositories/1296269/pulls?state=all&per_page=1&page=2>; rel="next", <https://api.github.com/repositories/1296269/pulls?state=all&per_page=1&page=15>; rel="last"'}, json=[]))
♻️ Duplicate comments (1)
backend/app/services/agents/tests/test_code_audit_agent.py (1)
196-200: Minor: Request object uses incorrect URL.Same issue as the GitHub network error test - the
Requestobjects userepo_urlinstead of actual API endpoint URLs. This doesn't affect functionality but isn't fully realistic.
🧹 Nitpick comments (3)
backend/app/services/agents/tests/test_code_audit_agent.py (3)
6-9: Consider using pytest's caplog fixture instead of module-level logging configuration.Module-level
basicConfigcan interfere with logging configuration in other test modules and makes tests less isolated. pytest'scaplogfixture provides better test isolation.Apply this diff to use caplog instead:
-import logging - -# Suppress logging during tests to avoid clutter -logging.basicConfig(level=logging.CRITICAL)Then in each test function that needs logging suppression:
async def test_fetch_github_repo_metrics_http_error(code_audit_agent, caplog): with caplog.at_level(logging.CRITICAL): # test codeAlternatively, create a fixture to suppress logging:
@pytest.fixture(autouse=True) def suppress_logging(caplog): caplog.set_level(logging.CRITICAL)
152-156: Minor: Request object uses incorrect URL.The
Requestobjects in theRequestErrormocks userepo_url(e.g.,"https://github.com/owner/repo") instead of the actual API endpoint URLs. While this doesn't affect test functionality since the request parameter isn't used in assertions, it's not fully realistic.For better realism, consider using the actual endpoint URLs or omit the request parameter entirely:
- respx.get(f"https://api.github.com/repos/{owner}/{repo}/commits?per_page=1").mock(side_effect=RequestError("Network error", request=Request("GET", repo_url))) + respx.get(f"https://api.github.com/repos/{owner}/{repo}/commits?per_page=1").mock(side_effect=RequestError("Network error", request=Request("GET", f"https://api.github.com/repos/{owner}/{repo}/commits?per_page=1")))Or simply:
- respx.get(f"https://api.github.com/repos/{owner}/{repo}/commits?per_page=1").mock(side_effect=RequestError("Network error", request=Request("GET", repo_url))) + respx.get(f"https://api.github.com/repos/{owner}/{repo}/commits?per_page=1").mock(side_effect=RequestError("Network error"))
121-245: Optional: Consider parametrizing error handling tests.The error handling tests (lines 121-245) share significant structural similarity. While the current implementation is clear and maintainable, parametrization could reduce duplication.
Example parametrized approach:
@pytest.mark.parametrize("platform,base_url,error_type", [ ("github", "https://api.github.com/repos/owner/repo", "http"), ("github", "https://api.github.com/repos/owner/repo", "network"), ("gitlab", "https://gitlab.com/api/v4/projects/owner%2Frepo", "http"), ("gitlab", "https://gitlab.com/api/v4/projects/owner%2Frepo", "network"), ]) @pytest.mark.asyncio async def test_fetch_repo_metrics_errors(code_audit_agent, platform, base_url, error_type): # Common test logic passHowever, the current approach is perfectly acceptable and arguably more readable for error scenarios. This is purely a maintainability suggestion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/app/services/agents/tests/__pycache__/test_code_audit_agent.cpython-313-pytest-8.4.2.pycis excluded by!**/*.pyc
📒 Files selected for processing (1)
backend/app/services/agents/tests/test_code_audit_agent.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/services/agents/tests/test_code_audit_agent.py (1)
backend/app/services/agents/code_audit_agent.py (5)
CodeAuditAgent(44-290)CodeMetrics(23-31)AuditSummary(33-38)CodeAuditResult(40-42)fetch_repo_metrics(177-220)
🔇 Additional comments (5)
backend/app/services/agents/tests/test_code_audit_agent.py (5)
121-143: Well-structured error handling test.This test correctly verifies that HTTP 404 errors result in graceful degradation to default/zeroed metrics rather than propagating exceptions. The mock setup comprehensively covers all GitHub API endpoints.
168-188: LGTM!This test correctly verifies GitLab API 404 error handling with comprehensive endpoint coverage and appropriate assertions for default values.
212-222: Good coverage for unsupported repository platforms.This test correctly verifies that unsupported URLs (Bitbucket in this case) gracefully return default metrics without attempting API calls.
224-234: Good coverage for malformed GitHub URLs.This test verifies that invalid GitHub URL formats (missing repository name) are handled gracefully without API calls.
236-245: Good coverage for malformed GitLab URLs.This test verifies that invalid GitLab URL formats are handled gracefully, maintaining consistency with the GitHub invalid format test.
Overview: Developed comprehensive unit tests for the
CodeAuditAgentto ensure robust functionality and reliable metric extraction.Changes
fetch_repo_metricsandanalyze_code_activity.backend/app/services/agents/code_audit_agent.pyand its new test file.Summary by CodeRabbit