test(auth, shopping): add unit tests for services#134
Open
varshapandiann wants to merge 1 commit into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds Jasmine unit tests for AuthService (login success and failure, logout and token persistence) and ShoppingListService (getAllShoppingLists and addListItem), with HttpClientTestingModule setup and httpMock.verify() cleanup. ChangesCore Service Unit Tests
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/src/app/shared/services/shopping-list.service.spec.ts (2)
57-59: ⚡ Quick winAssert POST payload to lock the API contract.
Line 57-59 validate URL/method, but not request body. Add a body assertion so the test fails if DTO mapping regresses.
Suggested patch
const req = httpMock.expectOne(`${apiUrl}/1/items`); expect(req.request.method).toBe('POST'); + expect(req.request.body).toEqual(mockItemRequest); req.flush(mockItemResponse);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/shared/services/shopping-list.service.spec.ts` around lines 57 - 59, The test currently checks the URL and method but not the POST body; after calling httpMock.expectOne(`${apiUrl}/1/items`) and before req.flush(mockItemResponse) add an assertion that req.request.body strictly equals the DTO you send in the service call (e.g., the newItem or mockItemRequest object used in the spec) so changes to the request mapping will fail the test; update the spec in shopping-list.service.spec.ts around the existing expectOne/req.sequence to assert req.request.body === expectedPayload.
22-60: ⚡ Quick winAdd error-path specs for both service methods.
Current tests only cover success responses. Add HTTP error cases (e.g., 400/500) for
getAllShoppingLists()andaddListItem()to validate behavior throughcatchError(...)in the service and improve confidence in failure handling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/shared/services/shopping-list.service.spec.ts` around lines 22 - 60, Add unit tests that exercise the error paths for service.getAllShoppingLists() and service.addListItem(): simulate HTTP errors (e.g., 400 and 500) via httpMock.expectOne(...).flush(..., { status, statusText }) and assert the Observables emit the expected error handling behavior (subscribe error callback or transformed value returned by the service's catchError). Locate tests around the existing specs for getAllShoppingLists and addListItem and add one failing-case test for each that verifies the service's catchError logic (e.g., error message, rethrow, or fallback value) for getAllShoppingLists() and for addListItem() using the same request URLs used in the success tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/app/shared/services/auth.service.spec.ts`:
- Around line 12-16: Replace the inline jasmine spy "routerSpy" with the
project's shared Router mock: import the shared Router mock (e.g., mockRouter)
and use it in TestBed.configureTestingModule providers instead of creating
routerSpy, keeping AuthService in providers; remove the local
jasmine.createSpyObj call and update any references to routerSpy in the spec to
use the shared mock instance.
---
Nitpick comments:
In `@frontend/src/app/shared/services/shopping-list.service.spec.ts`:
- Around line 57-59: The test currently checks the URL and method but not the
POST body; after calling httpMock.expectOne(`${apiUrl}/1/items`) and before
req.flush(mockItemResponse) add an assertion that req.request.body strictly
equals the DTO you send in the service call (e.g., the newItem or
mockItemRequest object used in the spec) so changes to the request mapping will
fail the test; update the spec in shopping-list.service.spec.ts around the
existing expectOne/req.sequence to assert req.request.body === expectedPayload.
- Around line 22-60: Add unit tests that exercise the error paths for
service.getAllShoppingLists() and service.addListItem(): simulate HTTP errors
(e.g., 400 and 500) via httpMock.expectOne(...).flush(..., { status, statusText
}) and assert the Observables emit the expected error handling behavior
(subscribe error callback or transformed value returned by the service's
catchError). Locate tests around the existing specs for getAllShoppingLists and
addListItem and add one failing-case test for each that verifies the service's
catchError logic (e.g., error message, rethrow, or fallback value) for
getAllShoppingLists() and for addListItem() using the same request URLs used in
the success tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 030b131d-43b1-4b11-8c3b-d1923bea1a4e
📒 Files selected for processing (2)
frontend/src/app/shared/services/auth.service.spec.tsfrontend/src/app/shared/services/shopping-list.service.spec.ts
varshapandiann
force-pushed
the
feature/issue-34-unit-tests
branch
from
3bfafc4 to
c87de2a
Compare
Collaborator
|
LGTM! @varshapandiann squash commit please! |
test(auth): use shared MockRouter from shared mocks style: fix formatting issues
varshapandiann
force-pushed
the
feature/issue-34-unit-tests
branch
from
c87de2a to
9cde199
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #34
Added unit tests for AuthService and ShoppingListService to achieve 80% code coverage. Verified that all tests pass and follow project standards by running npm run test:ci, npm run lint, and npm run prettier.
Summary by CodeRabbit