test: add unit tests for PetService#164
Conversation
📝 WalkthroughWalkthroughA new JUnit 5 test suite for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/test/java/org/example/vet1177/services/PetServiceTest.java (2)
35-36:RETURNS_DEEP_STUBSis unnecessary here.All
petRepositoryinteractions in this test class are direct method calls (findById,save,delete,flush), not chained fluent API calls. Deep stubs are typically for patterns likerepo.findById().map().orElseThrow(). Using them unnecessarily can obscure missing stubs and return unexpected default values silently.Suggested fix
- `@Mock`(answer = org.mockito.Answers.RETURNS_DEEP_STUBS) - private PetRepository petRepository; + `@Mock` + private PetRepository petRepository;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/vet1177/services/PetServiceTest.java` around lines 35 - 36, The mock declaration in PetServiceTest uses unnecessary deep stubs: change the annotation on petRepository from `@Mock`(answer = org.mockito.Answers.RETURNS_DEEP_STUBS) to a plain `@Mock` so interactions with PetRepository (findById, save, delete, flush) use standard Mockito behavior; update the PetServiceTest class to remove the answer attribute on the PetRepository mock and run the tests to ensure no missing stubs or behavior changes.
217-223: Add explicit stub forexistsByPetIdAndClinicIdto clarify test intent.This test relies on Mockito's default return value (
false) formedicalRecordRepository.existsByPetIdAndClinicId(). Since the test name explicitly describes "vetWithNoJournalAccess," stubbing this call to returnfalsewould make the test self-documenting and resilient to default behavior changes.Suggested fix
`@Test` void getPetById_vetWithNoJournalAccess_shouldThrowForbiddenException() { when(petRepository.findById(petId)).thenReturn(Optional.of(pet)); when(petPolicy.canView(vet, pet)).thenReturn(false); + when(medicalRecordRepository.existsByPetIdAndClinicId(petId, clinic.getId())).thenReturn(false); + assertThatThrownBy(() -> petService.getPetById(petId, vet)) .isInstanceOf(ForbiddenException.class); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/vet1177/services/PetServiceTest.java` around lines 217 - 223, Add an explicit Mockito stub for medicalRecordRepository.existsByPetIdAndClinicId(...) in the test getPetById_vetWithNoJournalAccess_shouldThrowForbiddenException to return false so the test intent is clear and not reliant on Mockito defaults; locate the test method getPetById_vetWithNoJournalAccess_shouldThrowForbiddenException and add a when(...).thenReturn(false) for medicalRecordRepository.existsByPetIdAndClinicId(petId, vet.getClinicId()) before invoking petService.getPetById(petId, vet), keeping the existing stubs for petRepository.findById(...) and petPolicy.canView(...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/test/java/org/example/vet1177/services/PetServiceTest.java`:
- Around line 35-36: The mock declaration in PetServiceTest uses unnecessary
deep stubs: change the annotation on petRepository from `@Mock`(answer =
org.mockito.Answers.RETURNS_DEEP_STUBS) to a plain `@Mock` so interactions with
PetRepository (findById, save, delete, flush) use standard Mockito behavior;
update the PetServiceTest class to remove the answer attribute on the
PetRepository mock and run the tests to ensure no missing stubs or behavior
changes.
- Around line 217-223: Add an explicit Mockito stub for
medicalRecordRepository.existsByPetIdAndClinicId(...) in the test
getPetById_vetWithNoJournalAccess_shouldThrowForbiddenException to return false
so the test intent is clear and not reliant on Mockito defaults; locate the test
method getPetById_vetWithNoJournalAccess_shouldThrowForbiddenException and add a
when(...).thenReturn(false) for
medicalRecordRepository.existsByPetIdAndClinicId(petId, vet.getClinicId())
before invoking petService.getPetById(petId, vet), keeping the existing stubs
for petRepository.findById(...) and petPolicy.canView(...).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4b4ec61a-e089-4f0d-aff1-87dcfba51c85
📒 Files selected for processing (1)
src/test/java/org/example/vet1177/services/PetServiceTest.java
Closes #144
Täcker createPet, getPetById, getPetsByOwner, updatePet och deletePet inklusive behörighetskontroll, felhantering och edge cases som veterinär med/utan klinik och journalåtkomst.
Summary by CodeRabbit