From f800b10dadc146ea44fb48d7163e472b169cd926 Mon Sep 17 00:00:00 2001 From: gourish25 Date: Fri, 17 Oct 2025 17:43:54 +0530 Subject: [PATCH 1/5] add test case and improved coverage for datasource/file/s3 --- pkg/gofr/datasource/file/s3/file_test.go | 1133 ++++++++++++++++++++++ pkg/gofr/datasource/file/s3/fs_test.go | 365 ++++++- 2 files changed, 1486 insertions(+), 12 deletions(-) create mode 100644 pkg/gofr/datasource/file/s3/file_test.go diff --git a/pkg/gofr/datasource/file/s3/file_test.go b/pkg/gofr/datasource/file/s3/file_test.go new file mode 100644 index 0000000000..fec4a4996e --- /dev/null +++ b/pkg/gofr/datasource/file/s3/file_test.go @@ -0,0 +1,1133 @@ +package s3 + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "io" + "os" + "strings" + "testing" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + "gofr.dev/pkg/gofr/datasource/file" +) + +var ( + ErrGetObject = errors.New("failed to get object from S3") + ErrPutObject = errors.New("failed to put object to S3") + ErrCloseFailed = errors.New("close failed") + ErrReadAllFailed = errors.New("simulated io.ReadAll error") +) + +// Helper function to create a new S3File instance for testing. +func newTestS3File(t *testing.T, ctrl *gomock.Controller, name string, size, offset int64) *S3File { + t.Helper() + return newTestS3FileWithTime(t, ctrl, name, size, offset, time.Now()) +} + +// Helper function to create a new S3File instance for testing with custom time. +func newTestS3FileWithTime(_ *testing.T, ctrl *gomock.Controller, name string, size, offset int64, + lastModified time.Time) *S3File { + mockClient := NewMocks3Client(ctrl) + mockMetrics := NewMockMetrics(ctrl) + mockLogger := NewMockLogger(ctrl) + + // Set up default logger expectations for common operations + mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() + + return &S3File{ + conn: mockClient, + name: name, + offset: offset, + logger: mockLogger, + metrics: mockMetrics, + size: size, + lastModified: lastModified, + } +} + +// Helper to create a successful GetObjectOutput. +func getObjectOutput(content string) *s3.GetObjectOutput { + return &s3.GetObjectOutput{ + Body: io.NopCloser(bytes.NewReader([]byte(content))), + ContentLength: aws.Int64(int64(len(content))), + } +} + +// Helper to create a successful PutObjectOutput. +func putObjectOutput() *s3.PutObjectOutput { + return &s3.PutObjectOutput{} +} + +// Define mock for io.ReadCloser to test Close. +type mockReadCloser struct { + io.Reader + closeErr error +} + +func (m *mockReadCloser) Close() error { + return m.closeErr +} + +// TestClose tests the Close method of S3File. +func TestS3File_Close(t *testing.T) { + testCases := []struct { + name string + body io.ReadCloser + expected error + }{ + { + name: "Success_BodyNil", + body: nil, + expected: nil, + }, + { + name: "Success_BodyNotNil", + body: &mockReadCloser{Reader: bytes.NewReader([]byte("test")), closeErr: nil}, + expected: nil, + }, + { + name: "Failure_CloseError", + body: &mockReadCloser{Reader: bytes.NewReader([]byte("test")), closeErr: ErrCloseFailed}, + expected: ErrCloseFailed, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + f := newTestS3File(t, ctrl, "test-bucket/test-file.txt", 10, 0) + f.body = tc.body + + err := f.Close() + + if !errors.Is(err, tc.expected) { + if tc.expected == nil && err != nil { + t.Errorf("Expected nil error, got %v", err) + } else if tc.expected != nil && err == nil { + t.Errorf("Expected error %v, got nil", tc.expected) + } else if tc.expected != nil && !strings.Contains(err.Error(), tc.expected.Error()) { + t.Errorf("Expected error containing %q, got %q", tc.expected.Error(), err.Error()) + } + } + }) + } +} + +var errS3Test = errors.New("s3 error") + +// TestS3File_Read_Success tests the successful Read operations of S3File. +func TestS3File_Read_Success(t *testing.T) { + bucketName := "test-bucket" + fileName := "test-file.txt" + fullPath := bucketName + "/" + fileName + content := "This is a test file content." + + testCases := []struct { + name string + offset int64 + bufferLen int + mockGetObject func(m *Mocks3ClientMockRecorder) + expectedN int + expectedP string + expectedErr error + }{ + { + name: "Success_ReadFromStart", + offset: 0, + bufferLen: 5, + mockGetObject: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Eq(&s3.GetObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(fileName), + })).Return(getObjectOutput(content), nil) + }, + expectedN: 5, + expectedP: "This ", + expectedErr: nil, + }, + { + name: "Success_ReadFromOffset", + offset: 5, + bufferLen: 4, + mockGetObject: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Eq(&s3.GetObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(fileName), + })).Return(getObjectOutput(content), nil) + }, + expectedN: 4, + expectedP: "is a", + expectedErr: nil, + }, + { + name: "Success_ReadToEOF", + offset: 0, + bufferLen: len(content), + mockGetObject: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Any()).Return(getObjectOutput(content), nil) + }, + expectedN: len(content), + expectedP: content, + expectedErr: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + f := newTestS3File(t, ctrl, fullPath, int64(len(content)), tc.offset) + + m := f.conn.(*Mocks3Client) + + if tc.mockGetObject != nil { + tc.mockGetObject(m.EXPECT()) + } + + // Use the buffer length defined in the test case + p := make([]byte, tc.bufferLen) + + // Reset buffer for clean comparison + for i := range p { + p[i] = 0 + } + + n, err := f.Read(p) + + // Check if the expected error is the one returned (or wrapped) + if !errors.Is(err, tc.expectedErr) { + t.Errorf("Expected error %v, got %v", tc.expectedErr, err) + } + + if tc.expectedErr == nil { + if n != tc.expectedN { + t.Errorf("Expected bytes read %d, got %d", tc.expectedN, n) + } + + // We only check up to n bytes of the expected and actual content + if string(p[:n]) != tc.expectedP[:n] { + t.Errorf("Expected content %q, got %q", tc.expectedP[:n], string(p[:n])) + } + } + }) + } +} + +// TestS3File_Read_Failure tests the failure cases of S3File Read operations. +func TestS3File_Read_Failure(t *testing.T) { + bucketName := "test-bucket" + fileName := "test-file.txt" + fullPath := bucketName + "/" + fileName + content := "This is a test file content." + + testCases := []struct { + name string + offset int64 + bufferLen int + mockGetObject func(m *Mocks3ClientMockRecorder) + expectedN int + expectedP string + expectedErr error + }{ + { + name: "Failure_GetObjectError", + offset: 0, + bufferLen: 10, + mockGetObject: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Any()).Return(nil, errS3Test) + }, + expectedN: 0, + expectedP: "", + expectedErr: errS3Test, + }, + { + name: "Failure_NilResponse", + offset: 0, + bufferLen: 10, + mockGetObject: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{Body: nil}, nil) + }, + expectedN: 0, + expectedP: "", + expectedErr: ErrNilResponse, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + f := newTestS3File(t, ctrl, fullPath, int64(len(content)), tc.offset) + + m := f.conn.(*Mocks3Client) + + if tc.mockGetObject != nil { + tc.mockGetObject(m.EXPECT()) + } + + // Use the buffer length defined in the test case + p := make([]byte, tc.bufferLen) + + // Reset buffer for clean comparison + for i := range p { + p[i] = 0 + } + + n, err := f.Read(p) + + // Check if the expected error is the one returned (or wrapped) + if !errors.Is(err, tc.expectedErr) { + t.Errorf("Expected error %v, got %v", tc.expectedErr, err) + } + + if tc.expectedErr == nil { + if n != tc.expectedN { + t.Errorf("Expected bytes read %d, got %d", tc.expectedN, n) + } + + // We only check up to n bytes of the expected and actual content + if string(p[:n]) != tc.expectedP[:n] { + t.Errorf("Expected content %q, got %q", tc.expectedP[:n], string(p[:n])) + } + } + }) + } +} + +// TestReadAt tests the ReadAt method of S3File. +func TestS3File_ReadAt(t *testing.T) { + bucketName := "test-bucket" + fileName := "test-file.txt" + fullPath := bucketName + "/" + fileName + content := "This is a test file content." + fileSize := int64(len(content)) + + testCases := []struct { + name string + size int64 + readAtOffset int64 + bufferLen int + mockGetObject func(m *Mocks3ClientMockRecorder) + expectedN int + expectedP string + expectedErr error + }{ + { + name: "Success_ReadFromMiddle", + size: fileSize, + readAtOffset: 5, + bufferLen: 4, + mockGetObject: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Any()).Return(getObjectOutput(content), nil) + }, + expectedN: 4, + expectedP: "is a", + expectedErr: nil, + }, + { + name: "Failure_GetObjectError", + size: fileSize, + readAtOffset: 0, + bufferLen: 10, + mockGetObject: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Any()).Return(nil, errS3Test) + }, + expectedN: 0, + expectedP: "", + expectedErr: errS3Test, + }, + { + name: "Failure_NilBody", + size: fileSize, + readAtOffset: 0, + bufferLen: 10, + mockGetObject: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{Body: nil}, nil) + }, + expectedN: 0, + expectedP: "", + expectedErr: io.EOF, + }, + { + name: "Failure_OutOfRange", + size: fileSize, + readAtOffset: 25, + bufferLen: 4, + mockGetObject: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Any()).Return(getObjectOutput(content), nil) + }, + expectedN: 0, + expectedP: "", + expectedErr: ErrOutOfRange, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + f := newTestS3File(t, ctrl, fullPath, tc.size, 10) + m := f.conn.(*Mocks3Client) + + if tc.mockGetObject != nil { + tc.mockGetObject(m.EXPECT()) + } + + p := make([]byte, tc.bufferLen) + n, err := f.ReadAt(p, tc.readAtOffset) + + if !errors.Is(err, tc.expectedErr) { + t.Errorf("Expected error %v, got %v", tc.expectedErr, err) + } + + if tc.expectedErr == nil { + if n != tc.expectedN { + t.Errorf("Expected bytes read %d, got %d", tc.expectedN, n) + } + + if string(p[:n]) != tc.expectedP[:n] { + t.Errorf("Expected content %q, got %q", tc.expectedP[:n], string(p[:n])) + } + + if f.offset != 10 { + t.Errorf("ReadAt modified offset. Expected 10, got %d", f.offset) + } + } + }) + } +} + +// TestS3File_Write_Success tests the successful Write operations of S3File. +func TestS3File_Write_Success(t *testing.T) { + bucketName := "test-bucket" + fileName := "test-file.txt" + fullPath := bucketName + "/" + fileName + initialContent := "Hello, World!" + initialSize := int64(len(initialContent)) + dataToWrite := []byte("GoFr") + + testCases := []struct { + name string + initialOffset int64 + initialSize int64 + dataToWrite []byte + mockExpectations func(m *Mocks3ClientMockRecorder) + expectedN int + expectedOffset int64 + expectedSize int64 + expectedErr error + }{ + { + name: "Success_WriteFromStart_NewFile", + initialOffset: 0, + initialSize: 0, + dataToWrite: dataToWrite, + mockExpectations: func(m *Mocks3ClientMockRecorder) { + m.PutObject(gomock.Any(), gomock.Any()).Return(putObjectOutput(), nil) + }, + expectedN: len(dataToWrite), + expectedOffset: int64(len(dataToWrite)), + expectedSize: int64(len(dataToWrite)), + expectedErr: nil, + }, + { + name: "Success_WriteFromStart_Overwrite", + initialOffset: 0, + initialSize: initialSize, + dataToWrite: dataToWrite, + mockExpectations: func(m *Mocks3ClientMockRecorder) { + m.PutObject(gomock.Any(), gomock.Any()).Return(putObjectOutput(), nil) + }, + expectedN: len(dataToWrite), + expectedOffset: int64(len(dataToWrite)), + expectedSize: int64(len(dataToWrite)), + expectedErr: nil, + }, + { + name: "Success_WriteFromMiddle", + initialOffset: 7, + initialSize: initialSize, + dataToWrite: dataToWrite, + mockExpectations: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Any()).Return(getObjectOutput(initialContent), nil) + m.PutObject(gomock.Any(), gomock.Any()).Return(putObjectOutput(), nil) + }, + expectedN: len(dataToWrite), + expectedOffset: 7 + int64(len(dataToWrite)), + expectedSize: initialSize, + expectedErr: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + f := newTestS3File(t, ctrl, fullPath, tc.initialSize, tc.initialOffset) + + m := f.conn.(*Mocks3Client) + + if tc.mockExpectations != nil { + tc.mockExpectations(m.EXPECT()) + } + + n, err := f.Write(tc.dataToWrite) + + if !errors.Is(err, tc.expectedErr) { + t.Errorf("Expected error %v, got %v", tc.expectedErr, err) + } + + if tc.expectedErr == nil { + if n != tc.expectedN { + t.Errorf("Expected bytes written %d, got %d", tc.expectedN, n) + } + + if f.offset != tc.expectedOffset { + t.Errorf("Expected offset %d, got %d", tc.expectedOffset, f.offset) + } + + if f.size != tc.expectedSize { + t.Errorf("Expected size %d, got %d", tc.expectedSize, f.size) + } + } + }) + } +} + +// TestS3File_Write_Failure tests the failure cases of S3File Write operations. +func TestS3File_Write_Failure(t *testing.T) { + bucketName := "test-bucket" + fileName := "test-file.txt" + fullPath := bucketName + "/" + fileName + initialContent := "Hello, World!" + initialSize := int64(len(initialContent)) + dataToWrite := []byte("GoFr") + + testCases := []struct { + name string + initialOffset int64 + initialSize int64 + dataToWrite []byte + mockExpectations func(m *Mocks3ClientMockRecorder) + expectedN int + expectedOffset int64 + expectedSize int64 + expectedErr error + }{ + { + name: "Failure_GetObjectError", + initialOffset: 5, + initialSize: initialSize, + dataToWrite: dataToWrite, + mockExpectations: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Any()).Return(nil, errS3Test) + }, + expectedN: 0, + expectedOffset: 5, + expectedSize: initialSize, + expectedErr: errS3Test, + }, + { + name: "Failure_PutObjectError", + initialOffset: 0, + initialSize: initialSize, + dataToWrite: dataToWrite, + mockExpectations: func(m *Mocks3ClientMockRecorder) { + m.PutObject(gomock.Any(), gomock.Any()).Return(nil, errS3Test) + }, + expectedN: 0, + expectedOffset: 0, + expectedSize: initialSize, + expectedErr: errS3Test, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + f := newTestS3File(t, ctrl, fullPath, tc.initialSize, tc.initialOffset) + + m := f.conn.(*Mocks3Client) + + if tc.mockExpectations != nil { + tc.mockExpectations(m.EXPECT()) + } + + n, err := f.Write(tc.dataToWrite) + + if !errors.Is(err, tc.expectedErr) { + t.Errorf("Expected error %v, got %v", tc.expectedErr, err) + } + + if tc.expectedErr == nil { + if n != tc.expectedN { + t.Errorf("Expected bytes written %d, got %d", tc.expectedN, n) + } + + if f.offset != tc.expectedOffset { + t.Errorf("Expected offset %d, got %d", tc.expectedOffset, f.offset) + } + + if f.size != tc.expectedSize { + t.Errorf("Expected size %d, got %d", tc.expectedSize, f.size) + } + } + }) + } +} + +// TestWriteAt tests the WriteAt method of S3File. +func TestS3File_WriteAt(t *testing.T) { + bucketName, fileName := "test-bucket", "test-file.txt" + fullPath := bucketName + "/" + fileName + initialContent := "Hello, World!" + initialSize, dataToWrite := int64(len(initialContent)), []byte("GoFr") + + testCases := []struct { + name string + writeAtOffset, initialOffset int64 + dataToWrite []byte + mockExpectations func(m *Mocks3ClientMockRecorder) + expectedN int + expectedOffset, expectedSize int64 + expectedErr error + }{ + { + name: "Success_WriteAtMiddle", + initialOffset: 10, writeAtOffset: 7, + dataToWrite: dataToWrite, + mockExpectations: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Any()).Return(getObjectOutput(initialContent), nil) + + expectedPutBody := []byte("Hello, GoFrd!") + + m.PutObject(gomock.Any(), gomock.Any()).Do(func(_ context.Context, params *s3.PutObjectInput, _ ...func(*s3.Options)) { + actualPutBody := getBodyContent(t, params.Body) + if !bytes.Equal(expectedPutBody, actualPutBody) { + t.Errorf("PutObject Body mismatch. Expected: %q, Got: %q", string(expectedPutBody), string(actualPutBody)) + t.FailNow() + } + }).Return(putObjectOutput(), nil) + }, + expectedN: len(dataToWrite), expectedOffset: 10, expectedSize: initialSize, + expectedErr: nil, + }, + { + name: "Failure_GetObjectError", + initialOffset: 10, writeAtOffset: 5, + dataToWrite: dataToWrite, + mockExpectations: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Any()).Return(nil, ErrGetObject) + }, + expectedN: 0, expectedOffset: 10, expectedSize: initialSize, + expectedErr: ErrGetObject, + }, + { + name: "Failure_PutObjectError", + initialOffset: 10, writeAtOffset: 0, + dataToWrite: dataToWrite, + mockExpectations: func(m *Mocks3ClientMockRecorder) { + m.GetObject(gomock.Any(), gomock.Any()).Return(getObjectOutput(initialContent), nil) + m.PutObject(gomock.Any(), gomock.Any()).Return(nil, ErrPutObject) + }, + expectedN: 0, expectedOffset: 10, expectedSize: initialSize, + expectedErr: ErrPutObject, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + f := newTestS3File(t, ctrl, fullPath, initialSize, tc.initialOffset) + + m := f.conn.(*Mocks3Client) + if tc.mockExpectations != nil { + tc.mockExpectations(m.EXPECT()) + } + + n, err := f.WriteAt(tc.dataToWrite, tc.writeAtOffset) + + if !errors.Is(err, tc.expectedErr) { + t.Errorf("Expected error %v, got %v", tc.expectedErr, err) + } + + if tc.expectedErr == nil { + if n != tc.expectedN { + t.Errorf("Expected bytes written %d, got %d", tc.expectedN, n) + } + + if f.offset != tc.expectedOffset { + t.Errorf("WriteAt modified offset. Expected %d, got %d", tc.expectedOffset, f.offset) + } + + if f.size != tc.expectedSize { + t.Errorf("Expected size %d, got %d", tc.expectedSize, f.size) + } + } + }) + } +} + +// Helper to read the content of the PutObjectInput Body. +func getBodyContent(t *testing.T, body io.Reader) []byte { + t.Helper() + + b, err := io.ReadAll(body) + require.NoError(t, err, "Failed to read PutObject body") + + return b +} + +// TestS3File_Seek_Basic tests the basic Seek operations (SeekStart and SeekCurrent) of S3File. +func TestS3File_Seek_Basic(t *testing.T) { + bucketName := "test-bucket" + fileName := "test-file.txt" + fullPath := bucketName + "/" + fileName + fileSize := int64(20) // Mock file size + + testCases := []struct { + name string + initialOffset int64 + offset int64 + whence int + expectedNewOffset int64 + expectedErr error + }{ + { + name: "SeekStart_Success", + initialOffset: 5, + offset: 10, + whence: io.SeekStart, + expectedNewOffset: 10, + expectedErr: nil, + }, + { + name: "SeekStart_Failure_Negative", + initialOffset: 5, + offset: -1, + whence: io.SeekStart, + expectedNewOffset: 0, + expectedErr: ErrOutOfRange, + }, + { + name: "SeekStart_Failure_TooLarge", + initialOffset: 5, + offset: 21, + whence: io.SeekStart, + expectedNewOffset: 0, + expectedErr: ErrOutOfRange, + }, + { + name: "SeekCurrent_Success", + initialOffset: 5, + offset: 10, + whence: io.SeekCurrent, + expectedNewOffset: 15, + expectedErr: nil, + }, + { + name: "SeekCurrent_Failure_NegativeResult", + initialOffset: 5, + offset: -6, + whence: io.SeekCurrent, + expectedNewOffset: 0, + expectedErr: ErrOutOfRange, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + f := newTestS3File(t, ctrl, fullPath, fileSize, tc.initialOffset) + + newOffset, err := f.Seek(tc.offset, tc.whence) + + if !errors.Is(err, tc.expectedErr) { + t.Errorf("Expected error %v, got %v", tc.expectedErr, err) + } + + if tc.expectedErr == nil { + if newOffset != tc.expectedNewOffset { + t.Errorf("Expected new offset %d, got %d", tc.expectedNewOffset, newOffset) + } + + if f.offset != tc.expectedNewOffset { + t.Errorf("File struct offset was not updated. Expected %d, got %d", tc.expectedNewOffset, f.offset) + } + } + }) + } +} + +// TestS3File_Seek_Advanced tests the advanced Seek operations (SeekEnd and InvalidWhence) of S3File. +func TestS3File_Seek_Advanced(t *testing.T) { + bucketName := "test-bucket" + fileName := "test-file.txt" + fullPath := bucketName + "/" + fileName + fileSize := int64(20) // Mock file size + + testCases := []struct { + name string + initialOffset int64 + offset int64 + whence int + expectedNewOffset int64 + expectedErr error + }{ + { + name: "SeekEnd_Success", + initialOffset: 5, + offset: -5, + whence: io.SeekEnd, + expectedNewOffset: 15, + expectedErr: nil, + }, + { + name: "SeekEnd_Failure_TooLarge", + initialOffset: 5, + offset: 1, + whence: io.SeekEnd, + expectedNewOffset: 0, + expectedErr: ErrOutOfRange, + }, + { + name: "SeekEnd_Success_ToStart", + initialOffset: 5, + offset: -20, + whence: io.SeekEnd, + expectedNewOffset: 0, + expectedErr: nil, + }, + { + name: "Seek_InvalidWhence", + initialOffset: 5, + offset: 0, + whence: 3, // Invalid whence + expectedNewOffset: 0, + expectedErr: os.ErrInvalid, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + f := newTestS3File(t, ctrl, fullPath, fileSize, tc.initialOffset) + + newOffset, err := f.Seek(tc.offset, tc.whence) + + if !errors.Is(err, tc.expectedErr) { + t.Errorf("Expected error %v, got %v", tc.expectedErr, err) + } + + if tc.expectedErr == nil { + if newOffset != tc.expectedNewOffset { + t.Errorf("Expected new offset %d, got %d", tc.expectedNewOffset, newOffset) + } + + if f.offset != tc.expectedNewOffset { + t.Errorf("File struct offset was not updated. Expected %d, got %d", tc.expectedNewOffset, f.offset) + } + } + }) + } +} + +// TestJsonReader_ValidObjects tests reading valid JSON objects from a jsonReader. +func TestJsonReader_ValidObjects(t *testing.T) { + jsonContent := `[ + {"name": "Alice", "age": 30}, + {"name": "Bob", "age": 25} + ]` + reader := bytes.NewReader([]byte(jsonContent)) + decoder := json.NewDecoder(reader) + // Must consume the '[' token + _, _ = decoder.Token() + + jReader := jsonReader{decoder: decoder} + + // Test first object + require.True(t, jReader.Next(), "Expected Next to be true for the first object") + + var data1 struct { + Name string + Age int + } + require.NoError(t, jReader.Scan(&data1), "Scan failed for first object") + assert.Equal(t, "Alice", data1.Name) + assert.Equal(t, 30, data1.Age) + + // Test second object + require.True(t, jReader.Next(), "Expected Next to be true for the second object") + + var data2 struct { + Name string + Age int + } + require.NoError(t, jReader.Scan(&data2), "Scan failed for second object") + assert.Equal(t, "Bob", data2.Name) + assert.Equal(t, 25, data2.Age) +} + +// TestJsonReader_NullAndEnd tests reading null values and end of array from a jsonReader. +func TestJsonReader_NullAndEnd(t *testing.T) { + jsonContent := `[ + {"name": "Alice", "age": 30}, + null + ]` + reader := bytes.NewReader([]byte(jsonContent)) + decoder := json.NewDecoder(reader) + // Must consume the '[' token + _, _ = decoder.Token() + + jReader := jsonReader{decoder: decoder} + + // Skip first object + jReader.Next() + err := jReader.Scan(&struct{}{}) + require.NoError(t, err, "Scan failed for null object") + + // Test null object + require.True(t, jReader.Next(), "Expected Next to be true for the null object") + + var data3 any + require.NoError(t, jReader.Scan(&data3), "Scan failed for null object") + assert.Nil(t, data3) + + // Test end of array + assert.False(t, jReader.Next(), "Expected Next to be false at the end of the array") + + // Test Scan error after array end + var invalidScanTarget struct{} + require.Error(t, jReader.Scan(&invalidScanTarget), "Expected Scan to fail after array end") +} + +// TestS3File_Metadata_Methods tests simple metadata methods. +func TestS3File_Metadata_Methods(t *testing.T) { + bucketName := "test-bucket" + fileName := "path/to/my-file.txt" + fullPath := bucketName + "/" + fileName + testSize := int64(4096) + testTime := time.Date(2023, 10, 10, 12, 0, 0, 0, time.UTC) + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + f := newTestS3FileWithTime(t, ctrl, fullPath, testSize, 0, testTime) + + // Test Name + expectedName := "my-file.txt" + assert.Equal(t, expectedName, f.Name()) + + // Test Size + assert.Equal(t, testSize, f.Size()) + + // Test ModTime + assert.Equal(t, testTime, f.ModTime()) + + // Test IsDir + assert.False(t, f.IsDir()) + + // Test IsDir for a directory-like name + f.name = bucketName + "/path/to/my-dir/" + assert.True(t, f.IsDir()) + + // Test Mode (placeholder) + assert.Equal(t, os.FileMode(0), f.Mode()) +} + +// createMockBody creates a MockReadCloser for testing based on the test case parameters. +func createMockBody(fileBody []byte, bodyReadError error) *MockReadCloser { + if bodyReadError != nil { + return &MockReadCloser{ + Reader: io.NopCloser(errorReader{err: bodyReadError}), + CloseFunc: func() error { + return nil + }, + } + } + + return &MockReadCloser{Reader: bytes.NewReader(fileBody)} +} + +// assertErrorCase validates that ReadAll returns an error as expected. +func assertErrorCase(t *testing.T, err error, reader file.RowReader, _ string) { + t.Helper() + require.Error(t, err, "ReadAll() expected an error, but got nil") + assert.Nil(t, reader, "ReadAll() expected nil reader on error") +} + +// assertSuccessCase validates that ReadAll succeeds and returns the correct reader type. +func assertSuccessCase(t *testing.T, err error, reader file.RowReader, expectedType string) { + t.Helper() + require.NoError(t, err, "ReadAll() unexpected error") + require.NotNil(t, reader, "ReadAll() returned nil reader on success") + + switch expectedType { + case "json-array", "json-object": + assert.IsType(t, &jsonReader{}, reader, "ReadAll() for JSON file expected *jsonReader") + case "text": + assert.IsType(t, &textReader{}, reader, "ReadAll() for text file expected *textReader") + } +} + +func TestS3File_ReadAll(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // A sample error for simulating I/O failures. + readError := ErrReadAllFailed + + // Helper function for creating a new S3File instance for a test + newS3File := func(name string, body io.ReadCloser) *S3File { + f := newTestS3File(t, ctrl, name, 0, 0) + f.body = body + + return f + } + + // --- Test Cases --- + tests := []struct { + name string + fileName string + fileBody []byte + bodyReadError error + expectedType string + }{ + { + name: "JSON Array Success", + fileName: "my-bucket/path/to/data.json", + fileBody: []byte(`[{"id": 1}, {"id": 2}]`), + bodyReadError: nil, + expectedType: "json-array", + }, + { + name: "JSON Object Success", + fileName: "my-bucket/path/to/config.json", + fileBody: []byte(`{"key": "value"}`), + bodyReadError: nil, + expectedType: "json-object", + }, + { + name: "Text/CSV Success", + fileName: "my-bucket/path/to/data.csv", + fileBody: []byte("col1,col2\n1,2"), + bodyReadError: nil, + expectedType: "text", + }, + { + name: "JSON ReadAll Error", + fileName: "my-bucket/fail.json", + fileBody: nil, + bodyReadError: readError, + expectedType: "error", + }, + { + name: "Text/CSV ReadAll Error", + fileName: "my-bucket/fail.txt", + fileBody: nil, + bodyReadError: readError, + expectedType: "error", + }, + { + name: "JSON Invalid Token Error", + fileName: "my-bucket/invalid.json", + fileBody: []byte(`not a json`), + bodyReadError: nil, + expectedType: "json-error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockBody := createMockBody(tt.fileBody, tt.bodyReadError) + f := newS3File(tt.fileName, mockBody) + + reader, err := f.ReadAll() + + if tt.expectedType == "error" || tt.expectedType == "json-error" { + assertErrorCase(t, err, reader, tt.expectedType) + return + } + + assertSuccessCase(t, err, reader, tt.expectedType) + }) + } +} + +// errorReader is a helper to simulate an io.ReadAll failure for testing. +type errorReader struct { + err error +} + +func (er errorReader) Read(_ []byte) (n int, err error) { + return 0, er.err +} + +// MockReadCloser is a minimal mock for the io.ReadCloser field 'f.body'. +type MockReadCloser struct { + io.Reader + CloseFunc func() error +} + +func (m *MockReadCloser) Close() error { + if m.CloseFunc != nil { + return m.CloseFunc() + } + + return nil +} + +func TestFileSystem_Connect(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + successConfig := &Config{ + BucketName: "test-bucket", + Region: "us-east-1", + AccessKeyID: "AKIA_SUCCESS", + SecretAccessKey: "SECRET_SUCCESS", + EndPoint: "http://localhost:9000", + } + + // --- Test Case: Successful Connection --- + t.Run("SuccessCase", func(t *testing.T) { + mockLogger := NewMockLogger(ctrl) + mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mockLogger.EXPECT().Debugf(gomock.Any(), gomock.Any()).AnyTimes() + mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + + fs := &FileSystem{ + config: successConfig, + logger: mockLogger, + } + + // Execute the function under test + fs.Connect() + + assert.NotNil(t, fs.conn, "Connect() failed to initialize S3 client") + }) +} diff --git a/pkg/gofr/datasource/file/s3/fs_test.go b/pkg/gofr/datasource/file/s3/fs_test.go index e719048a48..3fd488db5f 100644 --- a/pkg/gofr/datasource/file/s3/fs_test.go +++ b/pkg/gofr/datasource/file/s3/fs_test.go @@ -118,12 +118,12 @@ func Test_CreateFile(t *testing.T) { } } -func Test_OpenFile(t *testing.T) { +func Test_CreateFile_ErrorCases(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() // Create a mock S3 client - mockS3 := NewMocks3Client(ctrl) // Replace with the actual generated mock for the S3 client. + mockS3 := NewMocks3Client(ctrl) mockLogger := NewMockLogger(ctrl) mockMetrics := NewMockMetrics(ctrl) @@ -150,20 +150,267 @@ func Test_OpenFile(t *testing.T) { metrics: mockMetrics, } - mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + tests := []struct { + name string + createPath string + setupMocks func() + expectError bool + }{ + { + name: "PutObject_Fails", + createPath: "folder/test.txt", + setupMocks: func() { + // Mock ListObjectsV2 to succeed (for parent path check) + mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(&s3.ListObjectsV2Output{ + Contents: []types.Object{ + { + Key: aws.String("folder/"), + Size: aws.Int64(0), + }, + }, + }, nil) + // Mock PutObject to fail + mockS3.EXPECT().PutObject(gomock.Any(), gomock.Any()).Return(nil, errMock) + }, + expectError: true, + }, + { + name: "GetObject_Fails_After_PutObject_Success", + createPath: "folder/test.txt", + setupMocks: func() { + // Mock ListObjectsV2 to succeed (for parent path check) + mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(&s3.ListObjectsV2Output{ + Contents: []types.Object{ + { + Key: aws.String("folder/"), + Size: aws.Int64(0), + }, + }, + }, nil) + // Mock PutObject to succeed + mockS3.EXPECT().PutObject(gomock.Any(), gomock.Any()).Return(&s3.PutObjectOutput{}, nil) + // Mock GetObject to fail + mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(nil, errMock) + }, + expectError: true, + }, + } + + // Don't mock logger expectations - let the actual logger calls happen mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() + mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() - mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{ - Body: io.NopCloser(strings.NewReader("mock file content")), // Mock file content - ContentType: aws.String("text/plain"), // Mock content type - LastModified: aws.Time(time.Now()), // Mock last modified time - ContentLength: aws.Int64(123), // Mock file size in bytes - }, nil).AnyTimes() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.setupMocks() + _, err := fs.Create(tt.createPath) + + if tt.expectError { + require.Error(t, err, "Expected error for case: %s", tt.name) + } else { + require.NoError(t, err, "Unexpected error for case: %s", tt.name) + } + }) + } +} + +func Test_OpenFile(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // Create a mock S3 client + mockS3 := NewMocks3Client(ctrl) // Replace with the actual generated mock for the S3 client. + mockLogger := NewMockLogger(ctrl) + mockMetrics := NewMockMetrics(ctrl) + + // Define the configuration for the S3 package + config := &Config{ + EndPoint: "https://example.com", + BucketName: "test-bucket", + Region: "us-east-1", + AccessKeyID: "dummy-access-key", + SecretAccessKey: "dummy-secret-key", + } + + f := S3File{ + logger: mockLogger, + metrics: mockMetrics, + conn: mockS3, + } + + fs := &FileSystem{ + s3File: f, + conn: mockS3, + logger: mockLogger, + config: config, + metrics: mockMetrics, + } + + tests := []struct { + name string + fileName string + setupMocks func() + expectError bool + expectedError string + }{ + { + name: "Success_OpenFile", + fileName: "abc.json", + setupMocks: func() { + mockLogger.EXPECT().Debug(gomock.Any()).Times(1) // For the deferred stat log + mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{ + Body: io.NopCloser(strings.NewReader("mock file content")), // Mock file content + ContentType: aws.String("text/plain"), // Mock content type + LastModified: aws.Time(time.Now()), // Mock last modified time + ContentLength: aws.Int64(123), // Mock file size in bytes + }, nil).Times(1) + }, + expectError: false, + }, + { + name: "Error_GetObjectFails", + fileName: "nonexistent.json", + setupMocks: func() { + mockLogger.EXPECT().Errorf("failed to retrieve %q: %v", "nonexistent.json", errMock).Times(1) + mockLogger.EXPECT().Debug(gomock.Any()).Times(1) // For the deferred stat log + mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(nil, errMock).Times(1) + }, + expectError: true, + expectedError: "mocked error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Reset mock expectations for each test + ctrl.Finish() + ctrl = gomock.NewController(t) + mockS3 = NewMocks3Client(ctrl) + mockLogger = NewMockLogger(ctrl) + mockMetrics = NewMockMetrics(ctrl) + + // Update the FileSystem with new mocks + fs.conn = mockS3 + fs.logger = mockLogger + fs.metrics = mockMetrics + + // Setup mocks for this test case + tt.setupMocks() + + // Execute the test + _, err := fs.OpenFile(tt.fileName, 0, os.ModePerm) + + if tt.expectError { + require.Error(t, err, "Expected error but got none for case: %s", tt.name) + + if tt.expectedError != "" { + require.Contains(t, err.Error(), tt.expectedError, "Expected error to contain %q, got %q", tt.expectedError, err.Error()) + } + } else { + require.NoError(t, err, "Unexpected error for case: %s", tt.name) + } + }) + } +} + +func Test_Open(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // Create a mock S3 client + mockS3 := NewMocks3Client(ctrl) + mockLogger := NewMockLogger(ctrl) + mockMetrics := NewMockMetrics(ctrl) + + // Define the configuration for the S3 package + config := &Config{ + EndPoint: "https://example.com", + BucketName: "test-bucket", + Region: "us-east-1", + AccessKeyID: "dummy-access-key", + SecretAccessKey: "dummy-secret-key", + } + + f := S3File{ + logger: mockLogger, + metrics: mockMetrics, + conn: mockS3, + } + + fs := &FileSystem{ + s3File: f, + conn: mockS3, + logger: mockLogger, + config: config, + metrics: mockMetrics, + } + + tests := []struct { + name string + fileName string + setupMocks func() + expectError bool + expectedError string + }{ + { + name: "Success_OpenFile", + fileName: "test.json", + setupMocks: func() { + mockLogger.EXPECT().Debug(gomock.Any()).Times(1) // For the deferred stat log + mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{ + Body: io.NopCloser(strings.NewReader("test content")), + ContentType: aws.String("application/json"), + LastModified: aws.Time(time.Now()), + ContentLength: aws.Int64(12), + }, nil).Times(1) + }, + expectError: false, + }, + { + name: "Error_GetObjectFails", + fileName: "missing.json", + setupMocks: func() { + mockLogger.EXPECT().Errorf("failed to retrieve %q: %v", "missing.json", errMock).Times(1) + mockLogger.EXPECT().Debug(gomock.Any()).Times(1) // For the deferred stat log + mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(nil, errMock).Times(1) + }, + expectError: true, + expectedError: "mocked error", + }, + } - _, err := fs.OpenFile("abc.json", 0, os.ModePerm) - require.NoError(t, err, "TEST[%d] Failed. Desc: %v", 0, "Failed to open file") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Reset mock expectations for each test + ctrl.Finish() + ctrl = gomock.NewController(t) + mockS3 = NewMocks3Client(ctrl) + mockLogger = NewMockLogger(ctrl) + mockMetrics = NewMockMetrics(ctrl) + + // Update the FileSystem with new mocks + fs.conn = mockS3 + fs.logger = mockLogger + fs.metrics = mockMetrics + + // Setup mocks for this test case + tt.setupMocks() + + // Execute the test + _, err := fs.Open(tt.fileName) + + if tt.expectError { + require.Error(t, err, "Expected error but got none for case: %s", tt.name) + + if tt.expectedError != "" { + require.Contains(t, err.Error(), tt.expectedError, "Expected error to contain %q, got %q", tt.expectedError, err.Error()) + } + } else { + require.NoError(t, err, "Unexpected error for case: %s", tt.name) + } + }) + } } func Test_MakingDirectories(t *testing.T) { @@ -290,6 +537,53 @@ func Test_RenameDirectory(t *testing.T) { require.NoError(t, err, "TEST[%d] Failed. Desc: %v", 0, "Failed to rename directory") } +func Test_renameDirectory_ErrorCase(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // Create a mock S3 client + mockS3 := NewMocks3Client(ctrl) + mockLogger := NewMockLogger(ctrl) + mockMetrics := NewMockMetrics(ctrl) + + // Define the configuration for the S3 package + config := &Config{ + EndPoint: "https://example.com", + BucketName: "test-bucket", + Region: "us-east-1", + AccessKeyID: "dummy-access-key", + SecretAccessKey: "dummy-secret-key", + } + + f := S3File{ + logger: mockLogger, + metrics: mockMetrics, + conn: mockS3, + } + + fs := &FileSystem{ + s3File: f, + conn: mockS3, + logger: mockLogger, + config: config, + metrics: mockMetrics, + } + + // Mock ListObjectsV2 to fail + mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(nil, errMock) + + // Don't mock logger expectations - let the actual logger calls happen + mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() + + // Execute the test by calling Rename with directory paths (no extension) + err := fs.Rename("old-dir", "new-dir") + + // Verify error is returned + require.Error(t, err, "Expected error when ListObjectsV2 fails in renameDirectory") + require.Contains(t, err.Error(), "mocked error", "Expected error to contain the mocked error") +} + type result struct { Name string Size int64 @@ -656,3 +950,50 @@ func Test_StatDirectory(t *testing.T) { require.NoError(t, err, "TEST[%d] Failed. Desc: %v", 0, "Error getting directory stats") assert.Equal(t, expectedResponse, response, "Mismatch in results for path: %v", expectedResponse.name) } + +func Test_Stat_ErrorCase(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // Create a mock S3 client + mockS3 := NewMocks3Client(ctrl) + mockLogger := NewMockLogger(ctrl) + mockMetrics := NewMockMetrics(ctrl) + + // Define the configuration for the S3 package + config := &Config{ + EndPoint: "https://example.com", + BucketName: "test-bucket", + Region: "us-east-1", + AccessKeyID: "dummy-access-key", + SecretAccessKey: "dummy-secret-key", + } + + f := S3File{ + logger: mockLogger, + metrics: mockMetrics, + conn: mockS3, + } + + fs := &FileSystem{ + s3File: f, + conn: mockS3, + logger: mockLogger, + config: config, + metrics: mockMetrics, + } + + // Mock ListObjectsV2 to fail + mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(nil, errMock) + + // Don't mock logger expectations - let the actual logger calls happen + mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() + + // Execute the test + _, err := fs.Stat("test-file.txt") + + // Verify error is returned + require.Error(t, err, "Expected error when ListObjectsV2 fails") + require.Contains(t, err.Error(), "mocked error", "Expected error to contain the mocked error") +} From d2233f95cf6cf129ae87e3e8abe6ebf00893d5a3 Mon Sep 17 00:00:00 2001 From: gourish25 Date: Thu, 23 Oct 2025 13:00:52 +0530 Subject: [PATCH 2/5] add test case for the file.go --- pkg/gofr/datasource/file/s3/file_test.go | 147 ++++++----------------- 1 file changed, 38 insertions(+), 109 deletions(-) diff --git a/pkg/gofr/datasource/file/s3/file_test.go b/pkg/gofr/datasource/file/s3/file_test.go index fec4a4996e..e5c980fccc 100644 --- a/pkg/gofr/datasource/file/s3/file_test.go +++ b/pkg/gofr/datasource/file/s3/file_test.go @@ -112,14 +112,12 @@ func TestS3File_Close(t *testing.T) { err := f.Close() - if !errors.Is(err, tc.expected) { - if tc.expected == nil && err != nil { - t.Errorf("Expected nil error, got %v", err) - } else if tc.expected != nil && err == nil { - t.Errorf("Expected error %v, got nil", tc.expected) - } else if tc.expected != nil && !strings.Contains(err.Error(), tc.expected.Error()) { - t.Errorf("Expected error containing %q, got %q", tc.expected.Error(), err.Error()) - } + if tc.expected == nil { + assert.NoError(t, err, "Expected no error") + } else { + assert.Error(t, err, "Expected an error") + assert.True(t, errors.Is(err, tc.expected) || strings.Contains(err.Error(), tc.expected.Error()), + "Expected error to be %v or contain %q, got %v", tc.expected, tc.expected.Error(), err) } }) } @@ -208,19 +206,12 @@ func TestS3File_Read_Success(t *testing.T) { n, err := f.Read(p) // Check if the expected error is the one returned (or wrapped) - if !errors.Is(err, tc.expectedErr) { - t.Errorf("Expected error %v, got %v", tc.expectedErr, err) - } + assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { - if n != tc.expectedN { - t.Errorf("Expected bytes read %d, got %d", tc.expectedN, n) - } - + assert.Equal(t, tc.expectedN, n, "Expected bytes read %d, got %d", tc.expectedN, n) // We only check up to n bytes of the expected and actual content - if string(p[:n]) != tc.expectedP[:n] { - t.Errorf("Expected content %q, got %q", tc.expectedP[:n], string(p[:n])) - } + assert.Equal(t, tc.expectedP[:n], string(p[:n]), "Expected content %q, got %q", tc.expectedP[:n], string(p[:n])) } }) } @@ -290,19 +281,12 @@ func TestS3File_Read_Failure(t *testing.T) { n, err := f.Read(p) // Check if the expected error is the one returned (or wrapped) - if !errors.Is(err, tc.expectedErr) { - t.Errorf("Expected error %v, got %v", tc.expectedErr, err) - } + assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { - if n != tc.expectedN { - t.Errorf("Expected bytes read %d, got %d", tc.expectedN, n) - } - + assert.Equal(t, tc.expectedN, n, "Expected bytes read %d, got %d", tc.expectedN, n) // We only check up to n bytes of the expected and actual content - if string(p[:n]) != tc.expectedP[:n] { - t.Errorf("Expected content %q, got %q", tc.expectedP[:n], string(p[:n])) - } + assert.Equal(t, tc.expectedP[:n], string(p[:n]), "Expected content %q, got %q", tc.expectedP[:n], string(p[:n])) } }) } @@ -391,22 +375,12 @@ func TestS3File_ReadAt(t *testing.T) { p := make([]byte, tc.bufferLen) n, err := f.ReadAt(p, tc.readAtOffset) - if !errors.Is(err, tc.expectedErr) { - t.Errorf("Expected error %v, got %v", tc.expectedErr, err) - } + assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { - if n != tc.expectedN { - t.Errorf("Expected bytes read %d, got %d", tc.expectedN, n) - } - - if string(p[:n]) != tc.expectedP[:n] { - t.Errorf("Expected content %q, got %q", tc.expectedP[:n], string(p[:n])) - } - - if f.offset != 10 { - t.Errorf("ReadAt modified offset. Expected 10, got %d", f.offset) - } + assert.Equal(t, tc.expectedN, n, "Expected bytes read %d, got %d", tc.expectedN, n) + assert.Equal(t, tc.expectedP[:n], string(p[:n]), "Expected content %q, got %q", tc.expectedP[:n], string(p[:n])) + assert.Equal(t, int64(10), f.offset, "ReadAt modified offset. Expected 10, got %d", f.offset) } }) } @@ -489,22 +463,12 @@ func TestS3File_Write_Success(t *testing.T) { n, err := f.Write(tc.dataToWrite) - if !errors.Is(err, tc.expectedErr) { - t.Errorf("Expected error %v, got %v", tc.expectedErr, err) - } + assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { - if n != tc.expectedN { - t.Errorf("Expected bytes written %d, got %d", tc.expectedN, n) - } - - if f.offset != tc.expectedOffset { - t.Errorf("Expected offset %d, got %d", tc.expectedOffset, f.offset) - } - - if f.size != tc.expectedSize { - t.Errorf("Expected size %d, got %d", tc.expectedSize, f.size) - } + assert.Equal(t, tc.expectedN, n, "Expected bytes written %d, got %d", tc.expectedN, n) + assert.Equal(t, tc.expectedOffset, f.offset, "Expected offset %d, got %d", tc.expectedOffset, f.offset) + assert.Equal(t, tc.expectedSize, f.size, "Expected size %d, got %d", tc.expectedSize, f.size) } }) } @@ -573,22 +537,12 @@ func TestS3File_Write_Failure(t *testing.T) { n, err := f.Write(tc.dataToWrite) - if !errors.Is(err, tc.expectedErr) { - t.Errorf("Expected error %v, got %v", tc.expectedErr, err) - } + assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { - if n != tc.expectedN { - t.Errorf("Expected bytes written %d, got %d", tc.expectedN, n) - } - - if f.offset != tc.expectedOffset { - t.Errorf("Expected offset %d, got %d", tc.expectedOffset, f.offset) - } - - if f.size != tc.expectedSize { - t.Errorf("Expected size %d, got %d", tc.expectedSize, f.size) - } + assert.Equal(t, tc.expectedN, n, "Expected bytes written %d, got %d", tc.expectedN, n) + assert.Equal(t, tc.expectedOffset, f.offset, "Expected offset %d, got %d", tc.expectedOffset, f.offset) + assert.Equal(t, tc.expectedSize, f.size, "Expected size %d, got %d", tc.expectedSize, f.size) } }) } @@ -667,22 +621,12 @@ func TestS3File_WriteAt(t *testing.T) { n, err := f.WriteAt(tc.dataToWrite, tc.writeAtOffset) - if !errors.Is(err, tc.expectedErr) { - t.Errorf("Expected error %v, got %v", tc.expectedErr, err) - } + assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { - if n != tc.expectedN { - t.Errorf("Expected bytes written %d, got %d", tc.expectedN, n) - } - - if f.offset != tc.expectedOffset { - t.Errorf("WriteAt modified offset. Expected %d, got %d", tc.expectedOffset, f.offset) - } - - if f.size != tc.expectedSize { - t.Errorf("Expected size %d, got %d", tc.expectedSize, f.size) - } + assert.Equal(t, tc.expectedN, n, "Expected bytes written %d, got %d", tc.expectedN, n) + assert.Equal(t, tc.expectedOffset, f.offset, "WriteAt modified offset. Expected %d, got %d", tc.expectedOffset, f.offset) + assert.Equal(t, tc.expectedSize, f.size, "Expected size %d, got %d", tc.expectedSize, f.size) } }) } @@ -764,18 +708,11 @@ func TestS3File_Seek_Basic(t *testing.T) { newOffset, err := f.Seek(tc.offset, tc.whence) - if !errors.Is(err, tc.expectedErr) { - t.Errorf("Expected error %v, got %v", tc.expectedErr, err) - } + assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { - if newOffset != tc.expectedNewOffset { - t.Errorf("Expected new offset %d, got %d", tc.expectedNewOffset, newOffset) - } - - if f.offset != tc.expectedNewOffset { - t.Errorf("File struct offset was not updated. Expected %d, got %d", tc.expectedNewOffset, f.offset) - } + assert.Equal(t, tc.expectedNewOffset, newOffset, "Expected new offset %d, got %d", tc.expectedNewOffset, newOffset) + assert.Equal(t, tc.expectedNewOffset, f.offset, "File struct offset was not updated. Expected %d, got %d", tc.expectedNewOffset, f.offset) } }) } @@ -839,18 +776,11 @@ func TestS3File_Seek_Advanced(t *testing.T) { newOffset, err := f.Seek(tc.offset, tc.whence) - if !errors.Is(err, tc.expectedErr) { - t.Errorf("Expected error %v, got %v", tc.expectedErr, err) - } + assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { - if newOffset != tc.expectedNewOffset { - t.Errorf("Expected new offset %d, got %d", tc.expectedNewOffset, newOffset) - } - - if f.offset != tc.expectedNewOffset { - t.Errorf("File struct offset was not updated. Expected %d, got %d", tc.expectedNewOffset, f.offset) - } + assert.Equal(t, tc.expectedNewOffset, newOffset, "Expected new offset %d, got %d", tc.expectedNewOffset, newOffset) + assert.Equal(t, tc.expectedNewOffset, f.offset, "File struct offset was not updated. Expected %d, got %d", tc.expectedNewOffset, f.offset) } }) } @@ -1068,12 +998,12 @@ func TestS3File_ReadAll(t *testing.T) { reader, err := f.ReadAll() - if tt.expectedType == "error" || tt.expectedType == "json-error" { + switch tt.expectedType { + case "error", "json-error": assertErrorCase(t, err, reader, tt.expectedType) - return + default: + assertSuccessCase(t, err, reader, tt.expectedType) } - - assertSuccessCase(t, err, reader, tt.expectedType) }) } } @@ -1097,7 +1027,6 @@ func (m *MockReadCloser) Close() error { if m.CloseFunc != nil { return m.CloseFunc() } - return nil } From 19f4bd1b5908a76182c2462150c57b7bf490cd0d Mon Sep 17 00:00:00 2001 From: gourish25 Date: Thu, 23 Oct 2025 16:51:29 +0530 Subject: [PATCH 3/5] fix linters for file.go --- pkg/gofr/datasource/file/s3/file_test.go | 25 +++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/pkg/gofr/datasource/file/s3/file_test.go b/pkg/gofr/datasource/file/s3/file_test.go index e5c980fccc..c5fe20f906 100644 --- a/pkg/gofr/datasource/file/s3/file_test.go +++ b/pkg/gofr/datasource/file/s3/file_test.go @@ -115,7 +115,7 @@ func TestS3File_Close(t *testing.T) { if tc.expected == nil { assert.NoError(t, err, "Expected no error") } else { - assert.Error(t, err, "Expected an error") + require.Error(t, err, "Expected an error") assert.True(t, errors.Is(err, tc.expected) || strings.Contains(err.Error(), tc.expected.Error()), "Expected error to be %v or contain %q, got %v", tc.expected, tc.expected.Error(), err) } @@ -206,7 +206,7 @@ func TestS3File_Read_Success(t *testing.T) { n, err := f.Read(p) // Check if the expected error is the one returned (or wrapped) - assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) + require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { assert.Equal(t, tc.expectedN, n, "Expected bytes read %d, got %d", tc.expectedN, n) @@ -281,7 +281,7 @@ func TestS3File_Read_Failure(t *testing.T) { n, err := f.Read(p) // Check if the expected error is the one returned (or wrapped) - assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) + require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { assert.Equal(t, tc.expectedN, n, "Expected bytes read %d, got %d", tc.expectedN, n) @@ -375,7 +375,7 @@ func TestS3File_ReadAt(t *testing.T) { p := make([]byte, tc.bufferLen) n, err := f.ReadAt(p, tc.readAtOffset) - assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) + require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { assert.Equal(t, tc.expectedN, n, "Expected bytes read %d, got %d", tc.expectedN, n) @@ -463,7 +463,7 @@ func TestS3File_Write_Success(t *testing.T) { n, err := f.Write(tc.dataToWrite) - assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) + require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { assert.Equal(t, tc.expectedN, n, "Expected bytes written %d, got %d", tc.expectedN, n) @@ -537,7 +537,7 @@ func TestS3File_Write_Failure(t *testing.T) { n, err := f.Write(tc.dataToWrite) - assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) + require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { assert.Equal(t, tc.expectedN, n, "Expected bytes written %d, got %d", tc.expectedN, n) @@ -621,7 +621,7 @@ func TestS3File_WriteAt(t *testing.T) { n, err := f.WriteAt(tc.dataToWrite, tc.writeAtOffset) - assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) + require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { assert.Equal(t, tc.expectedN, n, "Expected bytes written %d, got %d", tc.expectedN, n) @@ -708,11 +708,12 @@ func TestS3File_Seek_Basic(t *testing.T) { newOffset, err := f.Seek(tc.offset, tc.whence) - assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) + require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { assert.Equal(t, tc.expectedNewOffset, newOffset, "Expected new offset %d, got %d", tc.expectedNewOffset, newOffset) - assert.Equal(t, tc.expectedNewOffset, f.offset, "File struct offset was not updated. Expected %d, got %d", tc.expectedNewOffset, f.offset) + assert.Equal(t, tc.expectedNewOffset, f.offset, + "File struct offset was not updated. Expected %d, got %d", tc.expectedNewOffset, f.offset) } }) } @@ -776,11 +777,12 @@ func TestS3File_Seek_Advanced(t *testing.T) { newOffset, err := f.Seek(tc.offset, tc.whence) - assert.True(t, errors.Is(err, tc.expectedErr), "Expected error %v, got %v", tc.expectedErr, err) + require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) if tc.expectedErr == nil { assert.Equal(t, tc.expectedNewOffset, newOffset, "Expected new offset %d, got %d", tc.expectedNewOffset, newOffset) - assert.Equal(t, tc.expectedNewOffset, f.offset, "File struct offset was not updated. Expected %d, got %d", tc.expectedNewOffset, f.offset) + assert.Equal(t, tc.expectedNewOffset, f.offset, + "File struct offset was not updated. Expected %d, got %d", tc.expectedNewOffset, f.offset) } }) } @@ -1027,6 +1029,7 @@ func (m *MockReadCloser) Close() error { if m.CloseFunc != nil { return m.CloseFunc() } + return nil } From bebd1e54ffc57cba3f608bfd73dbae72eb64f399 Mon Sep 17 00:00:00 2001 From: gourish25 Date: Wed, 29 Oct 2025 17:13:14 +0530 Subject: [PATCH 4/5] refactor the code to remove the redundant initialisation and resolve the pr comments --- pkg/gofr/datasource/file/s3/file_test.go | 629 +++++++------- pkg/gofr/datasource/file/s3/fs_test.go | 992 +++++++---------------- 2 files changed, 595 insertions(+), 1026 deletions(-) diff --git a/pkg/gofr/datasource/file/s3/file_test.go b/pkg/gofr/datasource/file/s3/file_test.go index c5fe20f906..259384e5cc 100644 --- a/pkg/gofr/datasource/file/s3/file_test.go +++ b/pkg/gofr/datasource/file/s3/file_test.go @@ -16,14 +16,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" - "gofr.dev/pkg/gofr/datasource/file" ) var ( - ErrGetObject = errors.New("failed to get object from S3") - ErrPutObject = errors.New("failed to put object to S3") - ErrCloseFailed = errors.New("close failed") - ErrReadAllFailed = errors.New("simulated io.ReadAll error") + errGetObject = errors.New("failed to get object from S3") + errPutObject = errors.New("failed to put object to S3") + errCloseFailed = errors.New("close failed") + errReadAllFailed = errors.New("simulated io.ReadAll error") ) // Helper function to create a new S3File instance for testing. @@ -39,7 +38,6 @@ func newTestS3FileWithTime(_ *testing.T, ctrl *gomock.Controller, name string, s mockMetrics := NewMockMetrics(ctrl) mockLogger := NewMockLogger(ctrl) - // Set up default logger expectations for common operations mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() @@ -78,27 +76,19 @@ func (m *mockReadCloser) Close() error { return m.closeErr } -// TestClose tests the Close method of S3File. -func TestS3File_Close(t *testing.T) { +// TestS3File_Close_Success tests the successful Close operations of S3File. +func TestS3File_Close_Success(t *testing.T) { testCases := []struct { - name string - body io.ReadCloser - expected error + name string + body io.ReadCloser }{ { - name: "Success_BodyNil", - body: nil, - expected: nil, + name: "Success_BodyNil", + body: nil, }, { - name: "Success_BodyNotNil", - body: &mockReadCloser{Reader: bytes.NewReader([]byte("test")), closeErr: nil}, - expected: nil, - }, - { - name: "Failure_CloseError", - body: &mockReadCloser{Reader: bytes.NewReader([]byte("test")), closeErr: ErrCloseFailed}, - expected: ErrCloseFailed, + name: "Success_BodyNotNil", + body: &mockReadCloser{Reader: bytes.NewReader([]byte("test")), closeErr: nil}, }, } @@ -112,17 +102,26 @@ func TestS3File_Close(t *testing.T) { err := f.Close() - if tc.expected == nil { - assert.NoError(t, err, "Expected no error") - } else { - require.Error(t, err, "Expected an error") - assert.True(t, errors.Is(err, tc.expected) || strings.Contains(err.Error(), tc.expected.Error()), - "Expected error to be %v or contain %q, got %v", tc.expected, tc.expected.Error(), err) - } + assert.NoError(t, err, "Expected no error") }) } } +// TestS3File_Close_Failure tests the failure cases of S3File Close operations. +func TestS3File_Close_Failure(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + f := newTestS3File(t, ctrl, "test-bucket/test-file.txt", 10, 0) + f.body = &mockReadCloser{Reader: bytes.NewReader([]byte("test")), closeErr: errCloseFailed} + + err := f.Close() + + require.Error(t, err, "Expected an error") + assert.True(t, errors.Is(err, errCloseFailed) || strings.Contains(err.Error(), errCloseFailed.Error()), + "Expected error to be %v or contain %q, got %v", errCloseFailed, errCloseFailed.Error(), err) +} + var errS3Test = errors.New("s3 error") // TestS3File_Read_Success tests the successful Read operations of S3File. @@ -191,28 +190,19 @@ func TestS3File_Read_Success(t *testing.T) { m := f.conn.(*Mocks3Client) - if tc.mockGetObject != nil { - tc.mockGetObject(m.EXPECT()) - } + tc.mockGetObject(m.EXPECT()) - // Use the buffer length defined in the test case p := make([]byte, tc.bufferLen) - // Reset buffer for clean comparison for i := range p { p[i] = 0 } n, err := f.Read(p) - // Check if the expected error is the one returned (or wrapped) - require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) - - if tc.expectedErr == nil { - assert.Equal(t, tc.expectedN, n, "Expected bytes read %d, got %d", tc.expectedN, n) - // We only check up to n bytes of the expected and actual content - assert.Equal(t, tc.expectedP[:n], string(p[:n]), "Expected content %q, got %q", tc.expectedP[:n], string(p[:n])) - } + require.NoError(t, err, "Expected no error") + assert.Equal(t, tc.expectedN, n, "Expected bytes read %d, got %d", tc.expectedN, n) + assert.Equal(t, tc.expectedP[:n], string(p[:n]), "Expected content %q, got %q", tc.expectedP[:n], string(p[:n])) }) } } @@ -266,34 +256,50 @@ func TestS3File_Read_Failure(t *testing.T) { m := f.conn.(*Mocks3Client) - if tc.mockGetObject != nil { - tc.mockGetObject(m.EXPECT()) - } + tc.mockGetObject(m.EXPECT()) - // Use the buffer length defined in the test case p := make([]byte, tc.bufferLen) - // Reset buffer for clean comparison for i := range p { p[i] = 0 } n, err := f.Read(p) - // Check if the expected error is the one returned (or wrapped) + require.Error(t, err, "Expected an error") require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) - - if tc.expectedErr == nil { - assert.Equal(t, tc.expectedN, n, "Expected bytes read %d, got %d", tc.expectedN, n) - // We only check up to n bytes of the expected and actual content - assert.Equal(t, tc.expectedP[:n], string(p[:n]), "Expected content %q, got %q", tc.expectedP[:n], string(p[:n])) - } + assert.Equal(t, tc.expectedN, n, "Expected bytes read %d, got %d", tc.expectedN, n) }) } } -// TestReadAt tests the ReadAt method of S3File. -func TestS3File_ReadAt(t *testing.T) { +// TestS3File_ReadAt_Success tests the successful ReadAt operations of S3File. +func TestS3File_ReadAt_Success(t *testing.T) { + bucketName := "test-bucket" + fileName := "test-file.txt" + fullPath := bucketName + "/" + fileName + content := "This is a test file content." + fileSize := int64(len(content)) + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + f := newTestS3File(t, ctrl, fullPath, fileSize, 10) + m := f.conn.(*Mocks3Client) + + m.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(getObjectOutput(content), nil) + + p := make([]byte, 4) + n, err := f.ReadAt(p, 5) + + require.NoError(t, err, "Expected no error") + assert.Equal(t, 4, n, "Expected bytes read 4, got %d", n) + assert.Equal(t, "is a", string(p[:n]), "Expected content %q, got %q", "is a", string(p[:n])) + assert.Equal(t, int64(10), f.offset, "ReadAt modified offset. Expected 10, got %d", f.offset) +} + +// TestS3File_ReadAt_Failure tests the failure cases of S3File ReadAt operations. +func TestS3File_ReadAt_Failure(t *testing.T) { bucketName := "test-bucket" fileName := "test-file.txt" fullPath := bucketName + "/" + fileName @@ -302,60 +308,40 @@ func TestS3File_ReadAt(t *testing.T) { testCases := []struct { name string - size int64 readAtOffset int64 bufferLen int mockGetObject func(m *Mocks3ClientMockRecorder) expectedN int - expectedP string expectedErr error }{ - { - name: "Success_ReadFromMiddle", - size: fileSize, - readAtOffset: 5, - bufferLen: 4, - mockGetObject: func(m *Mocks3ClientMockRecorder) { - m.GetObject(gomock.Any(), gomock.Any()).Return(getObjectOutput(content), nil) - }, - expectedN: 4, - expectedP: "is a", - expectedErr: nil, - }, { name: "Failure_GetObjectError", - size: fileSize, readAtOffset: 0, bufferLen: 10, mockGetObject: func(m *Mocks3ClientMockRecorder) { m.GetObject(gomock.Any(), gomock.Any()).Return(nil, errS3Test) }, expectedN: 0, - expectedP: "", expectedErr: errS3Test, }, { name: "Failure_NilBody", - size: fileSize, readAtOffset: 0, bufferLen: 10, mockGetObject: func(m *Mocks3ClientMockRecorder) { m.GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{Body: nil}, nil) }, expectedN: 0, - expectedP: "", expectedErr: io.EOF, }, { name: "Failure_OutOfRange", - size: fileSize, readAtOffset: 25, bufferLen: 4, mockGetObject: func(m *Mocks3ClientMockRecorder) { m.GetObject(gomock.Any(), gomock.Any()).Return(getObjectOutput(content), nil) }, expectedN: 0, - expectedP: "", expectedErr: ErrOutOfRange, }, } @@ -365,23 +351,17 @@ func TestS3File_ReadAt(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - f := newTestS3File(t, ctrl, fullPath, tc.size, 10) + f := newTestS3File(t, ctrl, fullPath, fileSize, 10) m := f.conn.(*Mocks3Client) - if tc.mockGetObject != nil { - tc.mockGetObject(m.EXPECT()) - } + tc.mockGetObject(m.EXPECT()) p := make([]byte, tc.bufferLen) n, err := f.ReadAt(p, tc.readAtOffset) + require.Error(t, err, "Expected an error") require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) - - if tc.expectedErr == nil { - assert.Equal(t, tc.expectedN, n, "Expected bytes read %d, got %d", tc.expectedN, n) - assert.Equal(t, tc.expectedP[:n], string(p[:n]), "Expected content %q, got %q", tc.expectedP[:n], string(p[:n])) - assert.Equal(t, int64(10), f.offset, "ReadAt modified offset. Expected 10, got %d", f.offset) - } + assert.Equal(t, tc.expectedN, n, "Expected bytes read %d, got %d", tc.expectedN, n) }) } } @@ -457,19 +437,14 @@ func TestS3File_Write_Success(t *testing.T) { m := f.conn.(*Mocks3Client) - if tc.mockExpectations != nil { - tc.mockExpectations(m.EXPECT()) - } + tc.mockExpectations(m.EXPECT()) n, err := f.Write(tc.dataToWrite) - require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) - - if tc.expectedErr == nil { - assert.Equal(t, tc.expectedN, n, "Expected bytes written %d, got %d", tc.expectedN, n) - assert.Equal(t, tc.expectedOffset, f.offset, "Expected offset %d, got %d", tc.expectedOffset, f.offset) - assert.Equal(t, tc.expectedSize, f.size, "Expected size %d, got %d", tc.expectedSize, f.size) - } + require.NoError(t, err, "Expected no error") + assert.Equal(t, tc.expectedN, n, "Expected bytes written %d, got %d", tc.expectedN, n) + assert.Equal(t, tc.expectedOffset, f.offset, "Expected offset %d, got %d", tc.expectedOffset, f.offset) + assert.Equal(t, tc.expectedSize, f.size, "Expected size %d, got %d", tc.expectedSize, f.size) }) } } @@ -531,79 +506,91 @@ func TestS3File_Write_Failure(t *testing.T) { m := f.conn.(*Mocks3Client) - if tc.mockExpectations != nil { - tc.mockExpectations(m.EXPECT()) - } + tc.mockExpectations(m.EXPECT()) n, err := f.Write(tc.dataToWrite) + require.Error(t, err, "Expected an error") require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) - - if tc.expectedErr == nil { - assert.Equal(t, tc.expectedN, n, "Expected bytes written %d, got %d", tc.expectedN, n) - assert.Equal(t, tc.expectedOffset, f.offset, "Expected offset %d, got %d", tc.expectedOffset, f.offset) - assert.Equal(t, tc.expectedSize, f.size, "Expected size %d, got %d", tc.expectedSize, f.size) - } + assert.Equal(t, tc.expectedN, n, "Expected bytes written %d, got %d", tc.expectedN, n) + assert.Equal(t, tc.expectedOffset, f.offset, "Expected offset %d, got %d", tc.expectedOffset, f.offset) + assert.Equal(t, tc.expectedSize, f.size, "Expected size %d, got %d", tc.expectedSize, f.size) }) } } -// TestWriteAt tests the WriteAt method of S3File. -func TestS3File_WriteAt(t *testing.T) { +// TestS3File_WriteAt_Success tests the successful WriteAt operations of S3File. +func TestS3File_WriteAt_Success(t *testing.T) { bucketName, fileName := "test-bucket", "test-file.txt" fullPath := bucketName + "/" + fileName initialContent := "Hello, World!" initialSize, dataToWrite := int64(len(initialContent)), []byte("GoFr") - testCases := []struct { - name string - writeAtOffset, initialOffset int64 - dataToWrite []byte - mockExpectations func(m *Mocks3ClientMockRecorder) - expectedN int - expectedOffset, expectedSize int64 - expectedErr error - }{ - { - name: "Success_WriteAtMiddle", - initialOffset: 10, writeAtOffset: 7, - dataToWrite: dataToWrite, - mockExpectations: func(m *Mocks3ClientMockRecorder) { - m.GetObject(gomock.Any(), gomock.Any()).Return(getObjectOutput(initialContent), nil) + ctrl := gomock.NewController(t) + defer ctrl.Finish() - expectedPutBody := []byte("Hello, GoFrd!") + f := newTestS3File(t, ctrl, fullPath, initialSize, 10) + m := f.conn.(*Mocks3Client) - m.PutObject(gomock.Any(), gomock.Any()).Do(func(_ context.Context, params *s3.PutObjectInput, _ ...func(*s3.Options)) { - actualPutBody := getBodyContent(t, params.Body) - if !bytes.Equal(expectedPutBody, actualPutBody) { - t.Errorf("PutObject Body mismatch. Expected: %q, Got: %q", string(expectedPutBody), string(actualPutBody)) - t.FailNow() - } - }).Return(putObjectOutput(), nil) - }, - expectedN: len(dataToWrite), expectedOffset: 10, expectedSize: initialSize, - expectedErr: nil, - }, + m.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(getObjectOutput(initialContent), nil) + + expectedPutBody := []byte("Hello, GoFrd!") + + m.EXPECT().PutObject(gomock.Any(), gomock.Any()).Do(func(_ context.Context, params *s3.PutObjectInput, _ ...func(*s3.Options)) { + actualPutBody := getBodyContent(t, params.Body) + require.True(t, bytes.Equal(expectedPutBody, actualPutBody), + "PutObject Body mismatch. Expected: %q, Got: %q", string(expectedPutBody), string(actualPutBody)) + }).Return(putObjectOutput(), nil) + + n, err := f.WriteAt(dataToWrite, 7) + + require.NoError(t, err, "Expected no error") + assert.Equal(t, len(dataToWrite), n, "Expected bytes written %d, got %d", len(dataToWrite), n) + assert.Equal(t, int64(10), f.offset, "WriteAt modified offset. Expected 10, got %d", f.offset) + assert.Equal(t, initialSize, f.size, "Expected size %d, got %d", initialSize, f.size) +} + +// TestS3File_WriteAt_Failure tests the failure cases of S3File WriteAt operations. +func TestS3File_WriteAt_Failure(t *testing.T) { + bucketName, fileName := "test-bucket", "test-file.txt" + fullPath := bucketName + "/" + fileName + initialContent := "Hello, World!" + initialSize, dataToWrite := int64(len(initialContent)), []byte("GoFr") + + testCases := []struct { + name string + writeAtOffset int64 + initialOffset int64 + mockExpectations func(m *Mocks3ClientMockRecorder) + expectedN int + expectedOffset int64 + expectedSize int64 + expectedErr error + }{ { name: "Failure_GetObjectError", - initialOffset: 10, writeAtOffset: 5, - dataToWrite: dataToWrite, + initialOffset: 10, + writeAtOffset: 5, mockExpectations: func(m *Mocks3ClientMockRecorder) { - m.GetObject(gomock.Any(), gomock.Any()).Return(nil, ErrGetObject) + m.GetObject(gomock.Any(), gomock.Any()).Return(nil, errGetObject) }, - expectedN: 0, expectedOffset: 10, expectedSize: initialSize, - expectedErr: ErrGetObject, + expectedN: 0, + expectedOffset: 10, + expectedSize: initialSize, + expectedErr: errGetObject, }, { name: "Failure_PutObjectError", - initialOffset: 10, writeAtOffset: 0, - dataToWrite: dataToWrite, + initialOffset: 10, + writeAtOffset: 0, mockExpectations: func(m *Mocks3ClientMockRecorder) { m.GetObject(gomock.Any(), gomock.Any()).Return(getObjectOutput(initialContent), nil) - m.PutObject(gomock.Any(), gomock.Any()).Return(nil, ErrPutObject) + m.PutObject(gomock.Any(), gomock.Any()).Return(nil, errPutObject) }, - expectedN: 0, expectedOffset: 10, expectedSize: initialSize, - expectedErr: ErrPutObject, + expectedN: 0, + expectedOffset: 10, + expectedSize: initialSize, + expectedErr: errPutObject, }, } @@ -615,19 +602,15 @@ func TestS3File_WriteAt(t *testing.T) { f := newTestS3File(t, ctrl, fullPath, initialSize, tc.initialOffset) m := f.conn.(*Mocks3Client) - if tc.mockExpectations != nil { - tc.mockExpectations(m.EXPECT()) - } + tc.mockExpectations(m.EXPECT()) - n, err := f.WriteAt(tc.dataToWrite, tc.writeAtOffset) + n, err := f.WriteAt(dataToWrite, tc.writeAtOffset) + require.Error(t, err, "Expected an error") require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) - - if tc.expectedErr == nil { - assert.Equal(t, tc.expectedN, n, "Expected bytes written %d, got %d", tc.expectedN, n) - assert.Equal(t, tc.expectedOffset, f.offset, "WriteAt modified offset. Expected %d, got %d", tc.expectedOffset, f.offset) - assert.Equal(t, tc.expectedSize, f.size, "Expected size %d, got %d", tc.expectedSize, f.size) - } + assert.Equal(t, tc.expectedN, n, "Expected bytes written %d, got %d", tc.expectedN, n) + assert.Equal(t, tc.expectedOffset, f.offset, "WriteAt modified offset. Expected %d, got %d", tc.expectedOffset, f.offset) + assert.Equal(t, tc.expectedSize, f.size, "Expected size %d, got %d", tc.expectedSize, f.size) }) } } @@ -642,12 +625,12 @@ func getBodyContent(t *testing.T, body io.Reader) []byte { return b } -// TestS3File_Seek_Basic tests the basic Seek operations (SeekStart and SeekCurrent) of S3File. -func TestS3File_Seek_Basic(t *testing.T) { +// TestS3File_Seek_Success tests the successful Seek operations of S3File. +func TestS3File_Seek_Success(t *testing.T) { bucketName := "test-bucket" fileName := "test-file.txt" fullPath := bucketName + "/" + fileName - fileSize := int64(20) // Mock file size + fileSize := int64(20) testCases := []struct { name string @@ -655,7 +638,6 @@ func TestS3File_Seek_Basic(t *testing.T) { offset int64 whence int expectedNewOffset int64 - expectedErr error }{ { name: "SeekStart_Success", @@ -663,23 +645,6 @@ func TestS3File_Seek_Basic(t *testing.T) { offset: 10, whence: io.SeekStart, expectedNewOffset: 10, - expectedErr: nil, - }, - { - name: "SeekStart_Failure_Negative", - initialOffset: 5, - offset: -1, - whence: io.SeekStart, - expectedNewOffset: 0, - expectedErr: ErrOutOfRange, - }, - { - name: "SeekStart_Failure_TooLarge", - initialOffset: 5, - offset: 21, - whence: io.SeekStart, - expectedNewOffset: 0, - expectedErr: ErrOutOfRange, }, { name: "SeekCurrent_Success", @@ -687,15 +652,20 @@ func TestS3File_Seek_Basic(t *testing.T) { offset: 10, whence: io.SeekCurrent, expectedNewOffset: 15, - expectedErr: nil, }, { - name: "SeekCurrent_Failure_NegativeResult", + name: "SeekEnd_Success", initialOffset: 5, - offset: -6, - whence: io.SeekCurrent, + offset: -5, + whence: io.SeekEnd, + expectedNewOffset: 15, + }, + { + name: "SeekEnd_Success_ToStart", + initialOffset: 5, + offset: -20, + whence: io.SeekEnd, expectedNewOffset: 0, - expectedErr: ErrOutOfRange, }, } @@ -708,63 +678,62 @@ func TestS3File_Seek_Basic(t *testing.T) { newOffset, err := f.Seek(tc.offset, tc.whence) - require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) - - if tc.expectedErr == nil { - assert.Equal(t, tc.expectedNewOffset, newOffset, "Expected new offset %d, got %d", tc.expectedNewOffset, newOffset) - assert.Equal(t, tc.expectedNewOffset, f.offset, - "File struct offset was not updated. Expected %d, got %d", tc.expectedNewOffset, f.offset) - } + require.NoError(t, err, "Expected no error") + assert.Equal(t, tc.expectedNewOffset, newOffset, "Expected new offset %d, got %d", tc.expectedNewOffset, newOffset) + assert.Equal(t, tc.expectedNewOffset, f.offset, "File struct offset was not updated. "+ + "Expected %d, got %d", tc.expectedNewOffset, f.offset) }) } } -// TestS3File_Seek_Advanced tests the advanced Seek operations (SeekEnd and InvalidWhence) of S3File. -func TestS3File_Seek_Advanced(t *testing.T) { +// TestS3File_Seek_Failure tests the failure cases of S3File Seek operations. +func TestS3File_Seek_Failure(t *testing.T) { bucketName := "test-bucket" fileName := "test-file.txt" fullPath := bucketName + "/" + fileName - fileSize := int64(20) // Mock file size + fileSize := int64(20) testCases := []struct { - name string - initialOffset int64 - offset int64 - whence int - expectedNewOffset int64 - expectedErr error + name string + initialOffset int64 + offset int64 + whence int + expectedErr error }{ { - name: "SeekEnd_Success", - initialOffset: 5, - offset: -5, - whence: io.SeekEnd, - expectedNewOffset: 15, - expectedErr: nil, + name: "SeekStart_Failure_Negative", + initialOffset: 5, + offset: -1, + whence: io.SeekStart, + expectedErr: ErrOutOfRange, }, { - name: "SeekEnd_Failure_TooLarge", - initialOffset: 5, - offset: 1, - whence: io.SeekEnd, - expectedNewOffset: 0, - expectedErr: ErrOutOfRange, + name: "SeekStart_Failure_TooLarge", + initialOffset: 5, + offset: 21, + whence: io.SeekStart, + expectedErr: ErrOutOfRange, }, { - name: "SeekEnd_Success_ToStart", - initialOffset: 5, - offset: -20, - whence: io.SeekEnd, - expectedNewOffset: 0, - expectedErr: nil, + name: "SeekCurrent_Failure_NegativeResult", + initialOffset: 5, + offset: -6, + whence: io.SeekCurrent, + expectedErr: ErrOutOfRange, }, { - name: "Seek_InvalidWhence", - initialOffset: 5, - offset: 0, - whence: 3, // Invalid whence - expectedNewOffset: 0, - expectedErr: os.ErrInvalid, + name: "SeekEnd_Failure_TooLarge", + initialOffset: 5, + offset: 1, + whence: io.SeekEnd, + expectedErr: ErrOutOfRange, + }, + { + name: "Seek_InvalidWhence", + initialOffset: 5, + offset: 0, + whence: 3, // Invalid whence + expectedErr: os.ErrInvalid, }, } @@ -775,15 +744,10 @@ func TestS3File_Seek_Advanced(t *testing.T) { f := newTestS3File(t, ctrl, fullPath, fileSize, tc.initialOffset) - newOffset, err := f.Seek(tc.offset, tc.whence) + _, err := f.Seek(tc.offset, tc.whence) + require.Error(t, err, "Expected an error") require.ErrorIs(t, err, tc.expectedErr, "Expected error %v, got %v", tc.expectedErr, err) - - if tc.expectedErr == nil { - assert.Equal(t, tc.expectedNewOffset, newOffset, "Expected new offset %d, got %d", tc.expectedNewOffset, newOffset) - assert.Equal(t, tc.expectedNewOffset, f.offset, - "File struct offset was not updated. Expected %d, got %d", tc.expectedNewOffset, f.offset) - } }) } } @@ -796,12 +760,10 @@ func TestJsonReader_ValidObjects(t *testing.T) { ]` reader := bytes.NewReader([]byte(jsonContent)) decoder := json.NewDecoder(reader) - // Must consume the '[' token _, _ = decoder.Token() jReader := jsonReader{decoder: decoder} - // Test first object require.True(t, jReader.Next(), "Expected Next to be true for the first object") var data1 struct { @@ -812,7 +774,6 @@ func TestJsonReader_ValidObjects(t *testing.T) { assert.Equal(t, "Alice", data1.Name) assert.Equal(t, 30, data1.Age) - // Test second object require.True(t, jReader.Next(), "Expected Next to be true for the second object") var data2 struct { @@ -832,27 +793,22 @@ func TestJsonReader_NullAndEnd(t *testing.T) { ]` reader := bytes.NewReader([]byte(jsonContent)) decoder := json.NewDecoder(reader) - // Must consume the '[' token _, _ = decoder.Token() jReader := jsonReader{decoder: decoder} - // Skip first object jReader.Next() err := jReader.Scan(&struct{}{}) require.NoError(t, err, "Scan failed for null object") - // Test null object require.True(t, jReader.Next(), "Expected Next to be true for the null object") var data3 any require.NoError(t, jReader.Scan(&data3), "Scan failed for null object") assert.Nil(t, data3) - // Test end of array assert.False(t, jReader.Next(), "Expected Next to be false at the end of the array") - // Test Scan error after array end var invalidScanTarget struct{} require.Error(t, jReader.Scan(&invalidScanTarget), "Expected Scan to fail after array end") } @@ -870,144 +826,135 @@ func TestS3File_Metadata_Methods(t *testing.T) { f := newTestS3FileWithTime(t, ctrl, fullPath, testSize, 0, testTime) - // Test Name expectedName := "my-file.txt" assert.Equal(t, expectedName, f.Name()) - // Test Size assert.Equal(t, testSize, f.Size()) - // Test ModTime assert.Equal(t, testTime, f.ModTime()) - // Test IsDir assert.False(t, f.IsDir()) - // Test IsDir for a directory-like name f.name = bucketName + "/path/to/my-dir/" assert.True(t, f.IsDir()) - // Test Mode (placeholder) assert.Equal(t, os.FileMode(0), f.Mode()) } -// createMockBody creates a MockReadCloser for testing based on the test case parameters. -func createMockBody(fileBody []byte, bodyReadError error) *MockReadCloser { - if bodyReadError != nil { - return &MockReadCloser{ - Reader: io.NopCloser(errorReader{err: bodyReadError}), - CloseFunc: func() error { - return nil - }, - } +// createMockBodyWithError creates a MockReadCloser that returns an error when reading. +func createMockBodyWithError(bodyReadError error) *MockReadCloser { + return &MockReadCloser{ + Reader: io.NopCloser(errorReader{err: bodyReadError}), + CloseFunc: func() error { + return nil + }, } +} - return &MockReadCloser{Reader: bytes.NewReader(fileBody)} +// createMockBodyWithContent creates a MockReadCloser with the provided content. +func createMockBodyWithContent(fileBody []byte) *MockReadCloser { + return &MockReadCloser{ + Reader: bytes.NewReader(fileBody), + CloseFunc: func() error { + return nil + }, + } } -// assertErrorCase validates that ReadAll returns an error as expected. -func assertErrorCase(t *testing.T, err error, reader file.RowReader, _ string) { +// Helper function for creating a new S3File instance for a test. +func newS3FileForReadAll(t *testing.T, ctrl *gomock.Controller, name string, body io.ReadCloser) *S3File { t.Helper() - require.Error(t, err, "ReadAll() expected an error, but got nil") - assert.Nil(t, reader, "ReadAll() expected nil reader on error") + f := newTestS3File(t, ctrl, name, 0, 0) + f.body = body + + return f } -// assertSuccessCase validates that ReadAll succeeds and returns the correct reader type. -func assertSuccessCase(t *testing.T, err error, reader file.RowReader, expectedType string) { - t.Helper() +// TestS3File_ReadAll_JSONArray_Success tests reading JSON array from S3File. +func TestS3File_ReadAll_JSONArray_Success(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockBody := createMockBodyWithContent([]byte(`[{"id": 1}, {"id": 2}]`)) + f := newS3FileForReadAll(t, ctrl, "my-bucket/path/to/data.json", mockBody) + + reader, err := f.ReadAll() + require.NoError(t, err, "ReadAll() unexpected error") require.NotNil(t, reader, "ReadAll() returned nil reader on success") + assert.IsType(t, &jsonReader{}, reader, "ReadAll() for JSON array expected *jsonReader") +} - switch expectedType { - case "json-array", "json-object": - assert.IsType(t, &jsonReader{}, reader, "ReadAll() for JSON file expected *jsonReader") - case "text": - assert.IsType(t, &textReader{}, reader, "ReadAll() for text file expected *textReader") - } +// TestS3File_ReadAll_JSONObject_Success tests reading JSON object from S3File. +func TestS3File_ReadAll_JSONObject_Success(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockBody := createMockBodyWithContent([]byte(`{"key": "value"}`)) + f := newS3FileForReadAll(t, ctrl, "my-bucket/path/to/config.json", mockBody) + + reader, err := f.ReadAll() + + require.NoError(t, err, "ReadAll() unexpected error") + require.NotNil(t, reader, "ReadAll() returned nil reader on success") + assert.IsType(t, &jsonReader{}, reader, "ReadAll() for JSON object expected *jsonReader") } -func TestS3File_ReadAll(t *testing.T) { +// TestS3File_ReadAll_Text_Success tests reading text/CSV from S3File. +func TestS3File_ReadAll_Text_Success(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - // A sample error for simulating I/O failures. - readError := ErrReadAllFailed + mockBody := createMockBodyWithContent([]byte("col1,col2\n1,2")) + f := newS3FileForReadAll(t, ctrl, "my-bucket/path/to/data.csv", mockBody) - // Helper function for creating a new S3File instance for a test - newS3File := func(name string, body io.ReadCloser) *S3File { - f := newTestS3File(t, ctrl, name, 0, 0) - f.body = body + reader, err := f.ReadAll() - return f - } + require.NoError(t, err, "ReadAll() unexpected error") + require.NotNil(t, reader, "ReadAll() returned nil reader on success") + assert.IsType(t, &textReader{}, reader, "ReadAll() for text file expected *textReader") +} - // --- Test Cases --- - tests := []struct { - name string - fileName string - fileBody []byte - bodyReadError error - expectedType string - }{ - { - name: "JSON Array Success", - fileName: "my-bucket/path/to/data.json", - fileBody: []byte(`[{"id": 1}, {"id": 2}]`), - bodyReadError: nil, - expectedType: "json-array", - }, - { - name: "JSON Object Success", - fileName: "my-bucket/path/to/config.json", - fileBody: []byte(`{"key": "value"}`), - bodyReadError: nil, - expectedType: "json-object", - }, - { - name: "Text/CSV Success", - fileName: "my-bucket/path/to/data.csv", - fileBody: []byte("col1,col2\n1,2"), - bodyReadError: nil, - expectedType: "text", - }, - { - name: "JSON ReadAll Error", - fileName: "my-bucket/fail.json", - fileBody: nil, - bodyReadError: readError, - expectedType: "error", - }, - { - name: "Text/CSV ReadAll Error", - fileName: "my-bucket/fail.txt", - fileBody: nil, - bodyReadError: readError, - expectedType: "error", - }, - { - name: "JSON Invalid Token Error", - fileName: "my-bucket/invalid.json", - fileBody: []byte(`not a json`), - bodyReadError: nil, - expectedType: "json-error", - }, - } +// TestS3File_ReadAll_JSON_Error tests ReadAll error for JSON file. +func TestS3File_ReadAll_JSON_Error(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - mockBody := createMockBody(tt.fileBody, tt.bodyReadError) - f := newS3File(tt.fileName, mockBody) + mockBody := createMockBodyWithError(errReadAllFailed) + f := newS3FileForReadAll(t, ctrl, "my-bucket/fail.json", mockBody) - reader, err := f.ReadAll() + reader, err := f.ReadAll() - switch tt.expectedType { - case "error", "json-error": - assertErrorCase(t, err, reader, tt.expectedType) - default: - assertSuccessCase(t, err, reader, tt.expectedType) - } - }) - } + require.Error(t, err, "ReadAll() expected an error, but got nil") + assert.Nil(t, reader, "ReadAll() expected nil reader on error") +} + +// TestS3File_ReadAll_Text_Error tests ReadAll error for text/CSV file. +func TestS3File_ReadAll_Text_Error(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockBody := createMockBodyWithError(errReadAllFailed) + f := newS3FileForReadAll(t, ctrl, "my-bucket/fail.txt", mockBody) + + reader, err := f.ReadAll() + + require.Error(t, err, "ReadAll() expected an error, but got nil") + assert.Nil(t, reader, "ReadAll() expected nil reader on error") +} + +// TestS3File_ReadAll_JSONInvalidToken_Error tests ReadAll error for invalid JSON. +func TestS3File_ReadAll_JSONInvalidToken_Error(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockBody := createMockBodyWithContent([]byte(`not a json`)) + f := newS3FileForReadAll(t, ctrl, "my-bucket/invalid.json", mockBody) + + reader, err := f.ReadAll() + + require.Error(t, err, "ReadAll() expected an error, but got nil") + assert.Nil(t, reader, "ReadAll() expected nil reader on error") } // errorReader is a helper to simulate an io.ReadAll failure for testing. @@ -1026,11 +973,7 @@ type MockReadCloser struct { } func (m *MockReadCloser) Close() error { - if m.CloseFunc != nil { - return m.CloseFunc() - } - - return nil + return m.CloseFunc() } func TestFileSystem_Connect(t *testing.T) { @@ -1045,7 +988,6 @@ func TestFileSystem_Connect(t *testing.T) { EndPoint: "http://localhost:9000", } - // --- Test Case: Successful Connection --- t.Run("SuccessCase", func(t *testing.T) { mockLogger := NewMockLogger(ctrl) mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() @@ -1057,7 +999,6 @@ func TestFileSystem_Connect(t *testing.T) { logger: mockLogger, } - // Execute the function under test fs.Connect() assert.NotNil(t, fs.conn, "Connect() failed to initialize S3 client") diff --git a/pkg/gofr/datasource/file/s3/fs_test.go b/pkg/gofr/datasource/file/s3/fs_test.go index 3fd488db5f..2b3828dd63 100644 --- a/pkg/gofr/datasource/file/s3/fs_test.go +++ b/pkg/gofr/datasource/file/s3/fs_test.go @@ -19,438 +19,163 @@ import ( var errMock = errors.New("mocked error") -func Test_CreateFile(t *testing.T) { - type testCase struct { - name string - createPath string - setupMocks func() - expectError bool - isRoot bool - } - - ctrl := gomock.NewController(t) - defer ctrl.Finish() +// testMocks contains all the mock objects needed for tests. +type testMocks struct { + mockS3 *Mocks3Client + mockLogger *MockLogger + mockMetrics *MockMetrics +} - // Create a mock S3 client - mockS3 := NewMocks3Client(ctrl) // Replace with the actual generated mock for the S3 client. - mockLogger := NewMockLogger(ctrl) - mockMetrics := NewMockMetrics(ctrl) - - tests := []testCase{ - {name: "Create txt file", createPath: "abc.txt", - setupMocks: func() { - mockS3.EXPECT().PutObject(gomock.Any(), gomock.Any()).Return(&s3.PutObjectOutput{}, nil) - mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{ - Body: io.NopCloser(strings.NewReader("test file content")), - ContentLength: aws.Int64(int64(len("test file content"))), - ContentType: aws.String("text/plain"), - LastModified: aws.Time(time.Now()), - }, nil) - }, - expectError: false, isRoot: true}, - {name: "Create file with invalid path", createPath: "abc/abc.txt", - setupMocks: func() { - mockS3.EXPECT(). - ListObjectsV2(gomock.Any(), gomock.Any()). - Return(nil, errMock) - }, - expectError: true, isRoot: false}, - {name: "Create valid file with directory existing", createPath: "abc/efg.txt", - setupMocks: func() { - mockS3.EXPECT(). - ListObjectsV2(gomock.Any(), gomock.Any()). - Return(&s3.ListObjectsV2Output{ - Contents: []types.Object{ - { - Key: aws.String("abc.txt"), - Size: aws.Int64(1), - }, - }, - }, nil) - - mockS3.EXPECT().PutObject(gomock.Any(), gomock.Any()).Return(&s3.PutObjectOutput{}, nil) - mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{ - Body: io.NopCloser(strings.NewReader("test file content")), - ContentLength: aws.Int64(int64(len("test file content"))), - ContentType: aws.String("text/plain"), - LastModified: aws.Time(time.Now()), - }, nil) - }, - expectError: false, isRoot: false}, +// setupTestMocks creates and returns all mock objects needed for testing. +func setupTestMocks(ctrl *gomock.Controller) *testMocks { + return &testMocks{ + mockS3: NewMocks3Client(ctrl), + mockLogger: NewMockLogger(ctrl), + mockMetrics: NewMockMetrics(ctrl), } +} - // Define the configuration for the S3 package - config := &Config{ +// defaultTestConfig returns a default Config for testing. +func defaultTestConfig() *Config { + return &Config{ EndPoint: "https://example.com", BucketName: "test-bucket", Region: "us-east-1", AccessKeyID: "dummy-access-key", SecretAccessKey: "dummy-secret-key", } +} + +// setupTestFileSystem creates and returns a FileSystem with all required dependencies. +func setupTestFileSystem(mocks *testMocks, config *Config) *FileSystem { + if config == nil { + config = defaultTestConfig() + } f := S3File{ - logger: mockLogger, - metrics: mockMetrics, - conn: mockS3, + logger: mocks.mockLogger, + metrics: mocks.mockMetrics, + conn: mocks.mockS3, } - fs := &FileSystem{ + return &FileSystem{ s3File: f, - conn: mockS3, - logger: mockLogger, + conn: mocks.mockS3, + logger: mocks.mockLogger, config: config, - metrics: mockMetrics, + metrics: mocks.mockMetrics, } +} - mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() +func Test_CreateFile_TxtFile_Success(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() - for i, tt := range tests { - tt.setupMocks() - _, err := fs.Create(tt.createPath) + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) - if tt.expectError { - require.Error(t, err, "TEST[%d] Failed. Desc %v", i, "Expected error during file creation") - return - } + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() - require.NoError(t, err, "TEST[%d] Failed. Desc %v", i, "Failed to create file") - } + mocks.mockS3.EXPECT().PutObject(gomock.Any(), gomock.Any()).Return(&s3.PutObjectOutput{}, nil) + mocks.mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{ + Body: io.NopCloser(strings.NewReader("test file content")), + ContentLength: aws.Int64(int64(len("test file content"))), + ContentType: aws.String("text/plain"), + LastModified: aws.Time(time.Now()), + }, nil) + + _, err := fs.Create("abc.txt") + require.NoError(t, err, "Failed to create file") } -func Test_CreateFile_ErrorCases(t *testing.T) { +func Test_CreateFile_WithExistingDirectory_Success(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - // Create a mock S3 client - mockS3 := NewMocks3Client(ctrl) - mockLogger := NewMockLogger(ctrl) - mockMetrics := NewMockMetrics(ctrl) + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) - // Define the configuration for the S3 package - config := &Config{ - EndPoint: "https://example.com", - BucketName: "test-bucket", - Region: "us-east-1", - AccessKeyID: "dummy-access-key", - SecretAccessKey: "dummy-secret-key", - } + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() - f := S3File{ - logger: mockLogger, - metrics: mockMetrics, - conn: mockS3, - } - - fs := &FileSystem{ - s3File: f, - conn: mockS3, - logger: mockLogger, - config: config, - metrics: mockMetrics, - } - - tests := []struct { - name string - createPath string - setupMocks func() - expectError bool - }{ - { - name: "PutObject_Fails", - createPath: "folder/test.txt", - setupMocks: func() { - // Mock ListObjectsV2 to succeed (for parent path check) - mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(&s3.ListObjectsV2Output{ - Contents: []types.Object{ - { - Key: aws.String("folder/"), - Size: aws.Int64(0), - }, - }, - }, nil) - // Mock PutObject to fail - mockS3.EXPECT().PutObject(gomock.Any(), gomock.Any()).Return(nil, errMock) - }, - expectError: true, - }, - { - name: "GetObject_Fails_After_PutObject_Success", - createPath: "folder/test.txt", - setupMocks: func() { - // Mock ListObjectsV2 to succeed (for parent path check) - mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(&s3.ListObjectsV2Output{ - Contents: []types.Object{ - { - Key: aws.String("folder/"), - Size: aws.Int64(0), - }, - }, - }, nil) - // Mock PutObject to succeed - mockS3.EXPECT().PutObject(gomock.Any(), gomock.Any()).Return(&s3.PutObjectOutput{}, nil) - // Mock GetObject to fail - mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(nil, errMock) + mocks.mockS3.EXPECT(). + ListObjectsV2(gomock.Any(), gomock.Any()). + Return(&s3.ListObjectsV2Output{ + Contents: []types.Object{ + { + Key: aws.String("abc.txt"), + Size: aws.Int64(1), + }, }, - expectError: true, - }, - } - - // Don't mock logger expectations - let the actual logger calls happen - mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() - mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + }, nil) + + mocks.mockS3.EXPECT().PutObject(gomock.Any(), gomock.Any()).Return(&s3.PutObjectOutput{}, nil) + mocks.mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{ + Body: io.NopCloser(strings.NewReader("test file content")), + ContentLength: aws.Int64(int64(len("test file content"))), + ContentType: aws.String("text/plain"), + LastModified: aws.Time(time.Now()), + }, nil) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tt.setupMocks() - _, err := fs.Create(tt.createPath) - - if tt.expectError { - require.Error(t, err, "Expected error for case: %s", tt.name) - } else { - require.NoError(t, err, "Unexpected error for case: %s", tt.name) - } - }) - } + _, err := fs.Create("abc/efg.txt") + require.NoError(t, err, "Failed to create file with existing directory") } -func Test_OpenFile(t *testing.T) { +func Test_CreateFile_Error(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - // Create a mock S3 client - mockS3 := NewMocks3Client(ctrl) // Replace with the actual generated mock for the S3 client. - mockLogger := NewMockLogger(ctrl) - mockMetrics := NewMockMetrics(ctrl) + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) - // Define the configuration for the S3 package - config := &Config{ - EndPoint: "https://example.com", - BucketName: "test-bucket", - Region: "us-east-1", - AccessKeyID: "dummy-access-key", - SecretAccessKey: "dummy-secret-key", - } + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() - f := S3File{ - logger: mockLogger, - metrics: mockMetrics, - conn: mockS3, - } - - fs := &FileSystem{ - s3File: f, - conn: mockS3, - logger: mockLogger, - config: config, - metrics: mockMetrics, - } - - tests := []struct { - name string - fileName string - setupMocks func() - expectError bool - expectedError string - }{ - { - name: "Success_OpenFile", - fileName: "abc.json", - setupMocks: func() { - mockLogger.EXPECT().Debug(gomock.Any()).Times(1) // For the deferred stat log - mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{ - Body: io.NopCloser(strings.NewReader("mock file content")), // Mock file content - ContentType: aws.String("text/plain"), // Mock content type - LastModified: aws.Time(time.Now()), // Mock last modified time - ContentLength: aws.Int64(123), // Mock file size in bytes - }, nil).Times(1) - }, - expectError: false, - }, - { - name: "Error_GetObjectFails", - fileName: "nonexistent.json", - setupMocks: func() { - mockLogger.EXPECT().Errorf("failed to retrieve %q: %v", "nonexistent.json", errMock).Times(1) - mockLogger.EXPECT().Debug(gomock.Any()).Times(1) // For the deferred stat log - mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(nil, errMock).Times(1) - }, - expectError: true, - expectedError: "mocked error", - }, - } + mocks.mockS3.EXPECT(). + ListObjectsV2(gomock.Any(), gomock.Any()). + Return(nil, errMock) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Reset mock expectations for each test - ctrl.Finish() - ctrl = gomock.NewController(t) - mockS3 = NewMocks3Client(ctrl) - mockLogger = NewMockLogger(ctrl) - mockMetrics = NewMockMetrics(ctrl) - - // Update the FileSystem with new mocks - fs.conn = mockS3 - fs.logger = mockLogger - fs.metrics = mockMetrics - - // Setup mocks for this test case - tt.setupMocks() - - // Execute the test - _, err := fs.OpenFile(tt.fileName, 0, os.ModePerm) - - if tt.expectError { - require.Error(t, err, "Expected error but got none for case: %s", tt.name) - - if tt.expectedError != "" { - require.Contains(t, err.Error(), tt.expectedError, "Expected error to contain %q, got %q", tt.expectedError, err.Error()) - } - } else { - require.NoError(t, err, "Unexpected error for case: %s", tt.name) - } - }) - } + _, err := fs.Create("abc/abc.txt") + require.Error(t, err, "Expected error during file creation with invalid path") } -func Test_Open(t *testing.T) { +func Test_OpenFile(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - // Create a mock S3 client - mockS3 := NewMocks3Client(ctrl) - mockLogger := NewMockLogger(ctrl) - mockMetrics := NewMockMetrics(ctrl) - - // Define the configuration for the S3 package - config := &Config{ - EndPoint: "https://example.com", - BucketName: "test-bucket", - Region: "us-east-1", - AccessKeyID: "dummy-access-key", - SecretAccessKey: "dummy-secret-key", - } + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) - f := S3File{ - logger: mockLogger, - metrics: mockMetrics, - conn: mockS3, - } + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() - fs := &FileSystem{ - s3File: f, - conn: mockS3, - logger: mockLogger, - config: config, - metrics: mockMetrics, - } - - tests := []struct { - name string - fileName string - setupMocks func() - expectError bool - expectedError string - }{ - { - name: "Success_OpenFile", - fileName: "test.json", - setupMocks: func() { - mockLogger.EXPECT().Debug(gomock.Any()).Times(1) // For the deferred stat log - mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{ - Body: io.NopCloser(strings.NewReader("test content")), - ContentType: aws.String("application/json"), - LastModified: aws.Time(time.Now()), - ContentLength: aws.Int64(12), - }, nil).Times(1) - }, - expectError: false, - }, - { - name: "Error_GetObjectFails", - fileName: "missing.json", - setupMocks: func() { - mockLogger.EXPECT().Errorf("failed to retrieve %q: %v", "missing.json", errMock).Times(1) - mockLogger.EXPECT().Debug(gomock.Any()).Times(1) // For the deferred stat log - mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(nil, errMock).Times(1) - }, - expectError: true, - expectedError: "mocked error", - }, - } + mocks.mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{ + Body: io.NopCloser(strings.NewReader("mock file content")), + ContentType: aws.String("text/plain"), + LastModified: aws.Time(time.Now()), + ContentLength: aws.Int64(123), + }, nil).AnyTimes() - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Reset mock expectations for each test - ctrl.Finish() - ctrl = gomock.NewController(t) - mockS3 = NewMocks3Client(ctrl) - mockLogger = NewMockLogger(ctrl) - mockMetrics = NewMockMetrics(ctrl) - - // Update the FileSystem with new mocks - fs.conn = mockS3 - fs.logger = mockLogger - fs.metrics = mockMetrics - - // Setup mocks for this test case - tt.setupMocks() - - // Execute the test - _, err := fs.Open(tt.fileName) - - if tt.expectError { - require.Error(t, err, "Expected error but got none for case: %s", tt.name) - - if tt.expectedError != "" { - require.Contains(t, err.Error(), tt.expectedError, "Expected error to contain %q, got %q", tt.expectedError, err.Error()) - } - } else { - require.NoError(t, err, "Unexpected error for case: %s", tt.name) - } - }) - } + _, err := fs.OpenFile("abc.json", 0, os.ModePerm) + require.NoError(t, err, "TEST[%d] Failed. Desc: %v", 0, "Failed to open file") } func Test_MakingDirectories(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - // Create a mock S3 client - mockS3 := NewMocks3Client(ctrl) - mockLogger := NewMockLogger(ctrl) - mockMetrics := NewMockMetrics(ctrl) + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) - // Define the configuration for the S3 package - config := &Config{ - EndPoint: "https://example.com", - BucketName: "test-bucket", - Region: "us-east-1", - AccessKeyID: "dummy-access-key", - SecretAccessKey: "dummy-secret-key", - } - - f := S3File{ - logger: mockLogger, - metrics: mockMetrics, - conn: mockS3, - } - - fs := &FileSystem{ - s3File: f, - conn: mockS3, - logger: mockLogger, - config: config, - metrics: mockMetrics, - } + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() - mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() - - mockS3.EXPECT(). + mocks.mockS3.EXPECT(). PutObject(gomock.Any(), gomock.Any()). Return(&s3.PutObjectOutput{}, nil).Times(3) @@ -462,40 +187,17 @@ func Test_RenameDirectory(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - // Create a mock S3 client - mockS3 := NewMocks3Client(ctrl) - mockLogger := NewMockLogger(ctrl) - mockMetrics := NewMockMetrics(ctrl) - - // Define the configuration for the S3 package - config := &Config{ - EndPoint: "https://example.com", - BucketName: "mock-bucket", - Region: "us-east-1", - AccessKeyID: "dummy-access-key", - SecretAccessKey: "dummy-secret-key", - } - - f := S3File{ - logger: mockLogger, - metrics: mockMetrics, - conn: mockS3, - } - - fs := &FileSystem{ - s3File: f, - conn: mockS3, - logger: mockLogger, - config: config, - metrics: mockMetrics, - } + mocks := setupTestMocks(ctrl) + config := defaultTestConfig() + config.BucketName = "mock-bucket" + fs := setupTestFileSystem(mocks, config) - mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() - mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() - mockS3.EXPECT(). + mocks.mockS3.EXPECT(). ListObjectsV2(gomock.Any(), gomock.Any()). Return(&s3.ListObjectsV2Output{ Contents: []types.Object{ @@ -508,15 +210,15 @@ func Test_RenameDirectory(t *testing.T) { }, }, nil).Times(1) - mockS3.EXPECT(). + mocks.mockS3.EXPECT(). CopyObject(gomock.Any(), gomock.Any()). Return(&s3.CopyObjectOutput{}, nil).Times(1) - mockS3.EXPECT(). + mocks.mockS3.EXPECT(). CopyObject(gomock.Any(), gomock.Any()). Return(&s3.CopyObjectOutput{}, nil).Times(1) - mockS3.EXPECT(). + mocks.mockS3.EXPECT(). ListObjectsV2(gomock.Any(), gomock.Any()). Return(&s3.ListObjectsV2Output{ Contents: []types.Object{ @@ -529,7 +231,7 @@ func Test_RenameDirectory(t *testing.T) { }, }, nil).Times(1) - mockS3.EXPECT(). + mocks.mockS3.EXPECT(). DeleteObjects(gomock.Any(), gomock.Any()). Return(&s3.DeleteObjectsOutput{}, nil).Times(1) @@ -537,53 +239,6 @@ func Test_RenameDirectory(t *testing.T) { require.NoError(t, err, "TEST[%d] Failed. Desc: %v", 0, "Failed to rename directory") } -func Test_renameDirectory_ErrorCase(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - // Create a mock S3 client - mockS3 := NewMocks3Client(ctrl) - mockLogger := NewMockLogger(ctrl) - mockMetrics := NewMockMetrics(ctrl) - - // Define the configuration for the S3 package - config := &Config{ - EndPoint: "https://example.com", - BucketName: "test-bucket", - Region: "us-east-1", - AccessKeyID: "dummy-access-key", - SecretAccessKey: "dummy-secret-key", - } - - f := S3File{ - logger: mockLogger, - metrics: mockMetrics, - conn: mockS3, - } - - fs := &FileSystem{ - s3File: f, - conn: mockS3, - logger: mockLogger, - config: config, - metrics: mockMetrics, - } - - // Mock ListObjectsV2 to fail - mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(nil, errMock) - - // Don't mock logger expectations - let the actual logger calls happen - mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() - mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() - - // Execute the test by calling Rename with directory paths (no extension) - err := fs.Rename("old-dir", "new-dir") - - // Verify error is returned - require.Error(t, err, "Expected error when ListObjectsV2 fails in renameDirectory") - require.Contains(t, err.Error(), "mocked error", "Expected error to contain the mocked error") -} - type result struct { Name string Size int64 @@ -594,24 +249,12 @@ func Test_ReadDir(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - mockS3 := NewMocks3Client(ctrl) - mockLogger := NewMockLogger(ctrl) - mockMetrics := NewMockMetrics(ctrl) - - config := &Config{ - EndPoint: "https://example.com", - BucketName: "test-bucket", - Region: "us-east-1", - AccessKeyID: "dummy-access-key", - SecretAccessKey: "dummy-secret-key", - } - - f := S3File{logger: mockLogger, metrics: mockMetrics, conn: mockS3} - fs := &FileSystem{s3File: f, conn: mockS3, logger: mockLogger, config: config, metrics: mockMetrics} + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) - mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() - mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() tests := []struct { name string @@ -627,7 +270,7 @@ func Test_ReadDir(t *testing.T) { {"hij", 0, true}, }, setupMock: func() { - mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(&s3.ListObjectsV2Output{ + mocks.mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(&s3.ListObjectsV2Output{ Contents: []types.Object{ {Key: aws.String("abc/efg/"), Size: aws.Int64(0), LastModified: aws.Time(time.Now())}, {Key: aws.String("abc/efg/file.txt"), Size: aws.Int64(1), LastModified: aws.Time(time.Now())}, @@ -643,7 +286,7 @@ func Test_ReadDir(t *testing.T) { {"efg", 0, true}, }, setupMock: func() { - mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(&s3.ListObjectsV2Output{ + mocks.mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(&s3.ListObjectsV2Output{ Contents: []types.Object{ {Key: aws.String("abc/"), Size: aws.Int64(0), LastModified: aws.Time(time.Now())}, {Key: aws.String("abc/efg/"), Size: aws.Int64(0), LastModified: aws.Time(time.Now())}, @@ -683,133 +326,82 @@ func TestRemove(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - // Create a mock S3 client - mockS3 := NewMocks3Client(ctrl) // Replace with the actual generated mock for the S3 client. - mockLogger := NewMockLogger(ctrl) - mockMetrics := NewMockMetrics(ctrl) + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) - // Define the configuration for the S3 package - config := &Config{ - EndPoint: "https://example.com", - BucketName: "test-bucket", - Region: "us-east-1", - AccessKeyID: "dummy-access-key", - SecretAccessKey: "dummy-secret-key", - } + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()) + mocks.mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() - f := S3File{ - logger: mockLogger, - metrics: mockMetrics, - conn: mockS3, - } + name := "testfile.txt" - fs := &FileSystem{ - s3File: f, - conn: mockS3, - logger: mockLogger, - config: config, - metrics: mockMetrics, - } + mocks.mockS3.EXPECT().DeleteObject(gomock.Any(), gomock.Any()).Return(&s3.DeleteObjectOutput{}, nil).Times(1) - mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any()) - mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() + err := fs.Remove(name) + require.NoError(t, err, "Remove() failed") +} - name := "testfile.txt" +func Test_RenameFile_ToNewName_Success(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() - mockS3.EXPECT().DeleteObject(gomock.Any(), gomock.Any()).Return(&s3.DeleteObjectOutput{}, nil).Times(1) + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) - err := fs.Remove(name) - if err != nil { - t.Fatalf("Remove() failed: %v", err) - } + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() + + mocks.mockS3.EXPECT().CopyObject(gomock.Any(), gomock.Any()).Return(&s3.CopyObjectOutput{}, nil).Times(1) + mocks.mockS3.EXPECT().DeleteObject(gomock.Any(), gomock.Any()).Return(&s3.DeleteObjectOutput{}, nil).Times(1) + + err := fs.Rename("abcd.json", "abc.json") + require.NoError(t, err, "Unexpected error when renaming file to new name") } -func Test_RenameFile(t *testing.T) { +func Test_RenameFile_ToSameName_Success(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - // Create a mock S3 client - mockS3 := NewMocks3Client(ctrl) // Replace with the actual generated mock for the S3 client. - mockLogger := NewMockLogger(ctrl) - mockMetrics := NewMockMetrics(ctrl) + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) - // Define the configuration for the S3 package - config := &Config{ - EndPoint: "https://example.com", - BucketName: "test-bucket", - Region: "us-east-1", - AccessKeyID: "dummy-access-key", - SecretAccessKey: "dummy-secret-key", - } + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() - f := S3File{ - logger: mockLogger, - metrics: mockMetrics, - conn: mockS3, - } + err := fs.Rename("abcd.json", "abcd.json") + require.NoError(t, err, "Unexpected error when renaming file to same name") +} - fs := &FileSystem{ - s3File: f, - conn: mockS3, - logger: mockLogger, - config: config, - metrics: mockMetrics, - } +func Test_RenameFile_WithDifferentExtension_Error(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() - tests := []struct { - name string - initialName string - newName string - expectedError bool - }{ - { - name: "Rename file to new name", - initialName: "abcd.json", - newName: "abc.json", - expectedError: false, - }, - { - name: "Rename file with different extension", - initialName: "abcd.json", - newName: "abcd.txt", - expectedError: true, - }, - { - name: "Rename file to same name", - initialName: "abcd.json", - newName: "abcd.json", - expectedError: false, - }, - { - name: "Rename file to directory path (unsupported)", - initialName: "abcd.json", - newName: "abc/abcd.json", - expectedError: true, - }, - } + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) - mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() - mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() - mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(&s3.ListObjectsV2Output{}, nil).AnyTimes() - mockS3.EXPECT().CopyObject(gomock.Any(), gomock.Any()).Return(&s3.CopyObjectOutput{}, nil).Times(1) - mockS3.EXPECT().DeleteObject(gomock.Any(), gomock.Any()).Return(&s3.DeleteObjectOutput{}, nil).Times(1) + err := fs.Rename("abcd.json", "abcd.txt") + require.Error(t, err, "Expected error when renaming file with different extension") +} - // Iterate through each test case - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := fs.Rename(tt.initialName, tt.newName) +func Test_RenameFile_ToDirectoryPath_Error(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() - if tt.expectedError { - require.Error(t, err, "Expected error but got none for case: %s", tt.name) - } else { - require.NoError(t, err, "Unexpected error for case: %s", tt.name) - } - }) - } + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) + + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() + + err := fs.Rename("abcd.json", "abc/abcd.json") + require.Error(t, err, "Expected error when renaming file to directory path") } func Test_StatFile(t *testing.T) { @@ -830,40 +422,15 @@ func Test_StatFile(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - // Create a mock S3 client - mockS3 := NewMocks3Client(ctrl) // Replace with the actual generated mock for the S3 client. - mockLogger := NewMockLogger(ctrl) - mockMetrics := NewMockMetrics(ctrl) + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) - // Define the configuration for the S3 package - config := &Config{ - EndPoint: "https://example.com", - BucketName: "test-bucket", - Region: "us-east-1", - AccessKeyID: "dummy-access-key", - SecretAccessKey: "dummy-secret-key", - } - - f := S3File{ - logger: mockLogger, - metrics: mockMetrics, - conn: mockS3, - } - - fs := &FileSystem{ - s3File: f, - conn: mockS3, - logger: mockLogger, - config: config, - metrics: mockMetrics, - } - - mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() - mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() - mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(&s3.ListObjectsV2Output{ + mocks.mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(&s3.ListObjectsV2Output{ Contents: []types.Object{ { Key: aws.String("file.txt"), @@ -895,37 +462,23 @@ func Test_StatDirectory(t *testing.T) { } ctrl := gomock.NewController(t) - mockLogger := NewMockLogger(ctrl) - mockMetrics := NewMockMetrics(ctrl) - mockConn := NewMocks3Client(ctrl) - - mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() - mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() - mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() - - cfg := Config{ - "http://localhost:4566", - "gofr-bucket-2", - "us-east-1", - "test", - "test", - } - f := S3File{ - conn: mockConn, - logger: mockLogger, - metrics: mockMetrics, - } + mocks := setupTestMocks(ctrl) - fs := FileSystem{ - s3File: f, - logger: mockLogger, - metrics: mockMetrics, - conn: mockConn, - config: &cfg, + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() + + cfg := &Config{ + EndPoint: "http://localhost:4566", + BucketName: "gofr-bucket-2", + Region: "us-east-1", + AccessKeyID: "test", + SecretAccessKey: "test", } + fs := setupTestFileSystem(mocks, cfg) - mockConn.EXPECT().ListObjectsV2(context.TODO(), &s3.ListObjectsV2Input{ + mocks.mockS3.EXPECT().ListObjectsV2(context.TODO(), &s3.ListObjectsV2Input{ Bucket: aws.String("gofr-bucket-2"), Prefix: aws.String("dir1/dir2"), }).Return(&s3.ListObjectsV2Output{ @@ -951,49 +504,124 @@ func Test_StatDirectory(t *testing.T) { assert.Equal(t, expectedResponse, response, "Mismatch in results for path: %v", expectedResponse.name) } -func Test_Stat_ErrorCase(t *testing.T) { +func Test_CreateFile_PutObjectFails_Error(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - // Create a mock S3 client - mockS3 := NewMocks3Client(ctrl) - mockLogger := NewMockLogger(ctrl) - mockMetrics := NewMockMetrics(ctrl) + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) - // Define the configuration for the S3 package - config := &Config{ - EndPoint: "https://example.com", - BucketName: "test-bucket", - Region: "us-east-1", - AccessKeyID: "dummy-access-key", - SecretAccessKey: "dummy-secret-key", - } + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() - f := S3File{ - logger: mockLogger, - metrics: mockMetrics, - conn: mockS3, - } + mocks.mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(&s3.ListObjectsV2Output{ + Contents: []types.Object{ + { + Key: aws.String("folder/"), + Size: aws.Int64(0), + }, + }, + }, nil) - fs := &FileSystem{ - s3File: f, - conn: mockS3, - logger: mockLogger, - config: config, - metrics: mockMetrics, - } + mocks.mockS3.EXPECT().PutObject(gomock.Any(), gomock.Any()).Return(nil, errMock) - // Mock ListObjectsV2 to fail - mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(nil, errMock) + _, err := fs.Create("folder/test.txt") + require.Error(t, err, "Expected error when PutObject fails") +} - // Don't mock logger expectations - let the actual logger calls happen - mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() - mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() +func Test_CreateFile_GetObjectFails_Error(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() - // Execute the test - _, err := fs.Stat("test-file.txt") + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) + + mocks.mockLogger.EXPECT().Logf(gomock.Any(), gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() - // Verify error is returned + mocks.mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(&s3.ListObjectsV2Output{ + Contents: []types.Object{ + { + Key: aws.String("folder/"), + Size: aws.Int64(0), + }, + }, + }, nil) + + mocks.mockS3.EXPECT().PutObject(gomock.Any(), gomock.Any()).Return(&s3.PutObjectOutput{}, nil) + mocks.mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(nil, errMock) + + _, err := fs.Create("folder/test.txt") + require.Error(t, err, "Expected error when GetObject fails after PutObject success") +} + +func Test_Open_Success(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) + + mocks.mockLogger.EXPECT().Debug(gomock.Any()).Times(1) + mocks.mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(&s3.GetObjectOutput{ + Body: io.NopCloser(strings.NewReader("test content")), + ContentType: aws.String("application/json"), + LastModified: aws.Time(time.Now()), + ContentLength: aws.Int64(12), + }, nil).Times(1) + + _, err := fs.Open("test.json") + require.NoError(t, err, "Unexpected error when opening file") +} + +func Test_Open_GetObjectFails_Error(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) + + mocks.mockLogger.EXPECT().Errorf("failed to retrieve %q: %v", "missing.json", errMock).Times(1) + mocks.mockLogger.EXPECT().Debug(gomock.Any()).Times(1) + mocks.mockS3.EXPECT().GetObject(gomock.Any(), gomock.Any()).Return(nil, errMock).Times(1) + + _, err := fs.Open("missing.json") + require.Error(t, err, "Expected error when GetObject fails") + require.Contains(t, err.Error(), "mocked error", "Expected error to contain mocked error") +} + +func Test_RenameDirectory_ListObjectsV2Fails_Error(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) + + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() + + mocks.mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(nil, errMock) + + err := fs.Rename("old-dir", "new-dir") + require.Error(t, err, "Expected error when ListObjectsV2 fails in renameDirectory") + require.Contains(t, err.Error(), "mocked error", "Expected error to contain mocked error") +} + +func Test_Stat_ListObjectsV2Fails_Error(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mocks := setupTestMocks(ctrl) + fs := setupTestFileSystem(mocks, nil) + + mocks.mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + mocks.mockLogger.EXPECT().Errorf(gomock.Any(), gomock.Any()).AnyTimes() + + mocks.mockS3.EXPECT().ListObjectsV2(gomock.Any(), gomock.Any()).Return(nil, errMock) + + _, err := fs.Stat("test-file.txt") require.Error(t, err, "Expected error when ListObjectsV2 fails") - require.Contains(t, err.Error(), "mocked error", "Expected error to contain the mocked error") + require.Contains(t, err.Error(), "mocked error", "Expected error to contain mocked error") } From 431a9196558d5082c0d22b54483d401c27e02f5a Mon Sep 17 00:00:00 2001 From: gourish25 Date: Wed, 29 Oct 2025 17:17:40 +0530 Subject: [PATCH 5/5] refactor the code to remove the redundant initialisation and resolve the pr comments --- pkg/gofr/datasource/file/s3/file_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/gofr/datasource/file/s3/file_test.go b/pkg/gofr/datasource/file/s3/file_test.go index 259384e5cc..8723b830f4 100644 --- a/pkg/gofr/datasource/file/s3/file_test.go +++ b/pkg/gofr/datasource/file/s3/file_test.go @@ -23,6 +23,7 @@ var ( errPutObject = errors.New("failed to put object to S3") errCloseFailed = errors.New("close failed") errReadAllFailed = errors.New("simulated io.ReadAll error") + errS3Test = errors.New("s3 error") ) // Helper function to create a new S3File instance for testing. @@ -122,8 +123,6 @@ func TestS3File_Close_Failure(t *testing.T) { "Expected error to be %v or contain %q, got %v", errCloseFailed, errCloseFailed.Error(), err) } -var errS3Test = errors.New("s3 error") - // TestS3File_Read_Success tests the successful Read operations of S3File. func TestS3File_Read_Success(t *testing.T) { bucketName := "test-bucket"