test: add unit tests for document and minio components#91
Conversation
-add DocumentServiceTest for upload, delete, fetch, and error handling -add DocumentControllerTest for file endpoints and HTTP responses -add MinioServiceTest for storage interaction methods -verify cleanup behavior when file operations fail -improve test coverage for document management flow
📝 WalkthroughWalkthroughThree new test classes are added for the document management feature: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
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.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/test/java/org/example/team6backend/document/service/DocumentServiceTest.java (1)
44-53: Assert theDocumentbuilt by the service, not a stubbed replacement.The repository mock returns a manually created
Document, so this test would still pass ifuploadFilestopped settingcontentType,fileSize,fileKey, orincident.Suggested stronger assertions
- Document document = new Document(); - document.setFileName("test.pdf"); - - when(documentRepository.save(any(Document.class))).thenReturn(document); + when(documentRepository.save(any(Document.class))).thenAnswer(invocation -> invocation.getArgument(0)); Document result = documentService.uploadFile(file, incident); assertEquals("test.pdf", result.getFileName()); + assertEquals("application/pdf", result.getContentType()); + assertEquals(100L, result.getFileSize()); + assertSame(incident, result.getIncident()); + assertNotNull(result.getFileKey()); + assertTrue(result.getFileKey().endsWith("_test.pdf")); verify(minioService).uploadFile(anyString(), eq(file)); verify(documentRepository).save(any(Document.class));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/team6backend/document/service/DocumentServiceTest.java` around lines 44 - 53, The test currently asserts the stubbed Document returned by documentRepository.save, so it won't catch if DocumentService.uploadFile fails to populate fields; change it to capture the Document passed into documentRepository.save (use an ArgumentCaptor<Document> or verify with an argument slot), assert on the captured Document's fileName, contentType, fileSize, fileKey and incident, and keep verifying minioService.uploadFile was called; reference DocumentService.uploadFile, documentRepository.save and minioService.uploadFile when locating the test to update.src/test/java/org/example/team6backend/document/service/MinioServiceTest.java (1)
4-12: Assert the MinIO bucket and object, not just “any args”.This test currently passes even if
deleteFile("abc")deletes the wrong object or bucket. CaptureRemoveObjectArgsand assert"test"/"abc".Suggested test tightening
import io.minio.MinioClient; import io.minio.RemoveObjectArgs; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.test.util.ReflectionTestUtils; -import static org.mockito.ArgumentMatchers.any; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.verify; @@ minioService.deleteFile("abc"); - verify(minioClient).removeObject(any(RemoveObjectArgs.class)); + ArgumentCaptor<RemoveObjectArgs> argsCaptor = ArgumentCaptor.forClass(RemoveObjectArgs.class); + verify(minioClient).removeObject(argsCaptor.capture()); + + RemoveObjectArgs args = argsCaptor.getValue(); + assertEquals("test", args.bucket()); + assertEquals("abc", args.object()); }Also applies to: 24-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/team6backend/document/service/MinioServiceTest.java` around lines 4 - 12, The test uses a loose verify(any()) for removeObject; tighten it by capturing the RemoveObjectArgs passed to the MinIO client and asserting the bucket and object values are exactly "test" and "abc". In MinioServiceTest, add an ArgumentCaptor<RemoveObjectArgs> (or use Mockito's capture) when verifying the client remove call for deleteFile("abc"), then assert captured.getBucket() equals "test" and captured.getObject() equals "abc"; apply the same capture/assert pattern for the other similar test case referenced (lines 24-28) to ensure the correct bucket/object are being deleted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/test/java/org/example/team6backend/document/controller/DocumentControllerTest.java`:
- Around line 43-70: Replace the `@WithMockUser` annotations in
DocumentControllerTest tests with a manual security setup that injects a
CustomUserDetails principal into the SecurityContext so
DocumentController.getFile receives a non-null authentication principal; create
a CustomUserDetails wrapping a User (or mock) and set it as the principal of a
UsernamePasswordAuthenticationToken (authenticated) in SecurityContextHolder
before calling mockMvc.perform (mirroring the pattern used in
IncidentControllerTest), and clear/reset the SecurityContext after the test to
avoid cross-test contamination.
In
`@src/test/java/org/example/team6backend/document/service/DocumentServiceTest.java`:
- Around line 85-88: The test stubs minioService.deleteFile("abc") to throw but
never verifies it was invoked, so add an explicit verification after invoking
DocumentService.deleteFile to assert minioService.deleteFile("abc") was called
(e.g., verify(minioService).deleteFile("abc") or verify(minioService,
times(1)).deleteFile("abc")) while keeping the existing assertDoesNotThrow and
verify(documentRepository).delete(document) checks; this ensures
DocumentService.deleteFile actually attempted the MinIO deletion in the failure
path.
- Around line 62-66: The test currently verifies deleteFile was called with any
string; change it to capture the actual file key uploaded and assert deleteFile
was called with that exact key: use an ArgumentCaptor<String> (or Mockito's
capture) to capture the first argument passed to minioService.uploadFile inside
the test for documentService.uploadFile, then when verifying
minioService.deleteFile(...) assert the captured upload key equals the value
passed to deleteFile; reference the test method logic around
documentService.uploadFile(...), minioService.uploadFile(...),
documentRepository.save(...), and minioService.deleteFile(...) to locate where
to add the ArgumentCaptor and equality assertion.
---
Nitpick comments:
In
`@src/test/java/org/example/team6backend/document/service/DocumentServiceTest.java`:
- Around line 44-53: The test currently asserts the stubbed Document returned by
documentRepository.save, so it won't catch if DocumentService.uploadFile fails
to populate fields; change it to capture the Document passed into
documentRepository.save (use an ArgumentCaptor<Document> or verify with an
argument slot), assert on the captured Document's fileName, contentType,
fileSize, fileKey and incident, and keep verifying minioService.uploadFile was
called; reference DocumentService.uploadFile, documentRepository.save and
minioService.uploadFile when locating the test to update.
In
`@src/test/java/org/example/team6backend/document/service/MinioServiceTest.java`:
- Around line 4-12: The test uses a loose verify(any()) for removeObject;
tighten it by capturing the RemoveObjectArgs passed to the MinIO client and
asserting the bucket and object values are exactly "test" and "abc". In
MinioServiceTest, add an ArgumentCaptor<RemoveObjectArgs> (or use Mockito's
capture) when verifying the client remove call for deleteFile("abc"), then
assert captured.getBucket() equals "test" and captured.getObject() equals "abc";
apply the same capture/assert pattern for the other similar test case referenced
(lines 24-28) to ensure the correct bucket/object are being deleted.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 657bec56-ba8c-47ca-a605-77745f45b423
📒 Files selected for processing (3)
src/test/java/org/example/team6backend/document/controller/DocumentControllerTest.javasrc/test/java/org/example/team6backend/document/service/DocumentServiceTest.javasrc/test/java/org/example/team6backend/document/service/MinioServiceTest.java
-add DocumentServiceTest for upload, delete, fetch, and error handling
-add DocumentControllerTest for file endpoints and HTTP responses
-add MinioServiceTest for storage interaction methods
-verify cleanup behavior when file operations fail
-improve test coverage for document management flow
Summary by CodeRabbit