Analysis Endpoint Finished / Tests Folder Moved#43
Conversation
…ra tests for 100% coverage
Trosper3
requested review from
DaniSleight,
Pranaya-Kafle,
Copilot and
jerrya-code
There was a problem hiding this comment.
Pull request overview
Adds a live /analyze endpoint that fetches Adzuna job listings and derives skills from the aggregated job descriptions, while updating/expanding tests to match the new flow.
Changes:
- Replaced the previous POST-based analyze endpoint with a GET
/analyze?q=...flow that fetches live jobs and analyzes their descriptions. - Added
analyze_jobs_data()service helper to aggregate descriptions prior to keyword extraction. - Updated and expanded unit/API tests for the new analyze behavior and for missing Adzuna credentials handling.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| app/tests/test_jobs.py | Adds coverage for missing Adzuna credentials path in the jobs service. |
| app/tests/test_analyze_service.py | Adds unit coverage for aggregating/analyzing skills across multiple job descriptions. |
| app/tests/test_analyze.py | Updates endpoint tests to reflect the new live GET analyze flow and removes old POST expectations. |
| app/modules/analyze/service.py | Introduces analyze_jobs_data() to extract skills from fetched jobs payloads. |
| app/modules/analyze/routes.py | Implements GET /analyze to fetch live jobs and return analyzed skills. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with ( | ||
| patch("app.modules.analyze.routes.fetch_jobs_from_adzuna", return_value=mocked_jobs) as mock_fetch, | ||
| patch("app.modules.analyze.routes.analyze_jobs_data", return_value=expected) as mock_analyze_jobs, | ||
| ): | ||
| response = client.get("/analyze", params={"q": "python"}) |
There was a problem hiding this comment.
fetch_jobs_from_adzuna is an async function, but this test patches it with return_value=... (a plain dict). The route does await fetch_jobs_from_adzuna(q), so this patch will cause TypeError: object dict can't be used in 'await' expression. Patch it with an AsyncMock (and assert it was awaited, not just called).
| with pytest.raises(Exception) as exc_info: | ||
| asyncio.run(jobs_service.fetch_jobs_from_adzuna("python")) | ||
|
|
||
| assert getattr(exc_info.value, "status_code", None) == 500 | ||
| assert getattr(exc_info.value, "detail", None) == "Adzuna credentials are not configured / correct" |
There was a problem hiding this comment.
This test expects a specific FastAPI HTTPException shape (status_code/detail), but it catches a broad Exception. Use pytest.raises(HTTPException) (importing it from fastapi) so the test fails correctly if a different exception type is raised.
jerrya-code
left a comment
jerrya-code
left a comment
There was a problem hiding this comment.
Looks good! Thank you!
Added Analysis live endpoint
Moved tests/ to be in app/
Added extra tests for 100% coverage
Closes #39