Test: add unit tests for ActivityLogService including validation and …#156
Conversation
📝 WalkthroughWalkthroughNew unit test class Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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/ActivityLogServiceTest.java (2)
41-48: Consider verifying the saved ActivityLog contents using ArgumentCaptor.The test only verifies that
save()is called, but doesn't assert that theActivityLogpassed tosave()contains the correctActivityType, description, user, and record. Using anArgumentCaptorwould strengthen this test.💡 Suggested enhancement
+import org.mockito.ArgumentCaptor;`@Test` void log_shouldSaveActivityLog_whenValidInput() { + // Arrange + ArgumentCaptor<ActivityLog> captor = ArgumentCaptor.forClass(ActivityLog.class); + // Act service.log(ActivityType.CASE_CREATED, "Created case", user, record); // Assert - verify(repository, times(1)).save(any(ActivityLog.class)); + verify(repository, times(1)).save(captor.capture()); + ActivityLog savedLog = captor.getValue(); + assertEquals(ActivityType.CASE_CREATED, savedLog.getAction()); + assertEquals("Created case", savedLog.getDescription()); + assertEquals(user, savedLog.getUser()); + assertEquals(record, savedLog.getMedicalRecord()); }🤖 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/ActivityLogServiceTest.java` around lines 41 - 48, The test log_shouldSaveActivityLog_whenValidInput currently only asserts repository.save was called; capture the ActivityLog passed to save using an ArgumentCaptor<ActivityLog> and assert its fields (getType/getDescription/getUser/getRecord or matching getters) equal ActivityType.CASE_CREATED, "Created case", the test's user and record; update the test to call verify(repository).save(captor.capture()) then assert on captor.getValue() and keep the existing times(1) verification if desired.
50-68: Consider verifying no side effects occur when validation fails.Adding
verifyNoInteractions(repository)at the end would confirm thatsave()is never called when any input is null, ensuring the validation guard clause prevents any unintended side effects.💡 Suggested enhancement
assertThrows(BusinessRuleException.class, () -> service.log(ActivityType.CASE_CREATED, "desc", user, null) ); + + verifyNoInteractions(repository); }🤖 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/ActivityLogServiceTest.java` around lines 50 - 68, The test log_shouldThrowException_whenNullInput currently asserts exceptions for null inputs but doesn't assert there are no side effects; after the four assertThrows calls add a verification that the mocked repository had no interactions (e.g., call verifyNoInteractions(repository)) to ensure repository.save() was never invoked when validation fails in ActivityLogServiceTest and the service.log(...) guard clauses.
🤖 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/ActivityLogServiceTest.java`:
- Around line 41-48: The test log_shouldSaveActivityLog_whenValidInput currently
only asserts repository.save was called; capture the ActivityLog passed to save
using an ArgumentCaptor<ActivityLog> and assert its fields
(getType/getDescription/getUser/getRecord or matching getters) equal
ActivityType.CASE_CREATED, "Created case", the test's user and record; update
the test to call verify(repository).save(captor.capture()) then assert on
captor.getValue() and keep the existing times(1) verification if desired.
- Around line 50-68: The test log_shouldThrowException_whenNullInput currently
asserts exceptions for null inputs but doesn't assert there are no side effects;
after the four assertThrows calls add a verification that the mocked repository
had no interactions (e.g., call verifyNoInteractions(repository)) to ensure
repository.save() was never invoked when validation fails in
ActivityLogServiceTest and the service.log(...) guard clauses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6cebdfe1-d24b-4655-b353-c9838e66cabe
📒 Files selected for processing (1)
src/test/java/org/example/vet1177/services/ActivityLogServiceTest.java
…filtering logic
Summary by CodeRabbit