From 82933a08362c195113d7dc5ce59ec80df4dba0dc Mon Sep 17 00:00:00 2001 From: kolaente Date: Sun, 1 Feb 2026 12:38:03 +0100 Subject: [PATCH] test(files): update tests for io.ReadSeeker API - Replace custom testfile structs with bytes.NewReader - Remove readerOnly wrapper and non-seekable reader tests (no longer possible at the type level) - Update S3 unit tests to remove temp file assertions --- pkg/files/files_test.go | 30 ++--------- pkg/files/s3_test.go | 82 +++--------------------------- pkg/models/task_attachment_test.go | 25 +-------- 3 files changed, 12 insertions(+), 125 deletions(-) diff --git a/pkg/files/files_test.go b/pkg/files/files_test.go index 1d883817e..70377fd32 100644 --- a/pkg/files/files_test.go +++ b/pkg/files/files_test.go @@ -17,7 +17,7 @@ package files import ( - "io" + "bytes" "os" "testing" @@ -27,24 +27,6 @@ import ( "github.com/stretchr/testify/require" ) -type testfile struct { - content []byte - done bool -} - -func (t *testfile) Read(p []byte) (n int, err error) { - if t.done { - return 0, io.EOF - } - copy(p, t.content) - t.done = true - return len(p), nil -} - -func (t *testfile) Close() error { - return nil -} - type testauth struct { id int64 } @@ -56,11 +38,8 @@ func (a *testauth) GetID() int64 { func TestCreate(t *testing.T) { t.Run("Normal", func(t *testing.T) { initFixtures(t) - tf := &testfile{ - content: []byte("testfile"), - } ta := &testauth{id: 1} - createdFile, err := Create(tf, "testfile", 100, ta) + createdFile, err := Create(bytes.NewReader([]byte("testfile")), "testfile", 100, ta) require.NoError(t, err) // Check the file was created correctly @@ -74,11 +53,8 @@ func TestCreate(t *testing.T) { }) t.Run("Too Large", func(t *testing.T) { initFixtures(t) - tf := &testfile{ - content: []byte("testfile"), - } ta := &testauth{id: 1} - _, err := Create(tf, "testfile", 99999999999, ta) + _, err := Create(bytes.NewReader([]byte("testfile")), "testfile", 99999999999, ta) require.Error(t, err) assert.True(t, IsErrFileIsTooLarge(err)) }) diff --git a/pkg/files/s3_test.go b/pkg/files/s3_test.go index 7d7869844..3c0c5b3f3 100644 --- a/pkg/files/s3_test.go +++ b/pkg/files/s3_test.go @@ -321,27 +321,14 @@ func (f *fakeS3PutObjectClient) PutObject(_ context.Context, input *s3.PutObject return &s3.PutObjectOutput{}, nil } -type readerOnly struct { - r io.Reader -} - -func (r *readerOnly) Read(p []byte) (int, error) { - return r.r.Read(p) -} - -func TestFileSave_S3_UsesSeekableReaderWithoutTempFile(t *testing.T) { +func TestFileSave_S3_UsesSeekableReader(t *testing.T) { originalClient := s3Client originalBucket := s3Bucket - originalTempDir := config.FilesS3TempDir.GetString() t.Cleanup(func() { s3Client = originalClient s3Bucket = originalBucket - config.FilesS3TempDir.Set(originalTempDir) }) - tempDir := t.TempDir() - config.FilesS3TempDir.Set(tempDir) - client := &fakeS3PutObjectClient{} s3Client = client s3Bucket = "test-bucket" @@ -358,87 +345,36 @@ func TestFileSave_S3_UsesSeekableReaderWithoutTempFile(t *testing.T) { require.NotNil(t, client.lastInput.ContentLength) assert.Equal(t, int64(len(content)), *client.lastInput.ContentLength) assert.IsType(t, &bytes.Reader{}, client.lastInput.Body) - - entries, err := os.ReadDir(tempDir) - require.NoError(t, err) - assert.Empty(t, entries) } -func TestFileSave_S3_BuffersNonSeekableReaderAndCleansUpTempFile(t *testing.T) { +func TestFileSave_S3_ReturnsErrorOnPutObjectFailure(t *testing.T) { originalClient := s3Client originalBucket := s3Bucket - originalTempDir := config.FilesS3TempDir.GetString() t.Cleanup(func() { s3Client = originalClient s3Bucket = originalBucket - config.FilesS3TempDir.Set(originalTempDir) }) - tempDir := t.TempDir() - config.FilesS3TempDir.Set(tempDir) - - client := &fakeS3PutObjectClient{} - s3Client = client - s3Bucket = "test-bucket" - - content := []byte("non-seekable-content") - file := &File{ID: 456, Size: 0} - - err := file.Save(&readerOnly{r: bytes.NewReader(content)}) - require.NoError(t, err) - - require.NotNil(t, client.lastInput) - require.NotNil(t, client.lastInput.ContentLength) - assert.Equal(t, int64(len(content)), *client.lastInput.ContentLength) - assert.IsType(t, &os.File{}, client.lastInput.Body) - - entries, err := os.ReadDir(tempDir) - require.NoError(t, err) - assert.Empty(t, entries) -} - -func TestFileSave_S3_CleansUpTempFileOnPutObjectError(t *testing.T) { - originalClient := s3Client - originalBucket := s3Bucket - originalTempDir := config.FilesS3TempDir.GetString() - t.Cleanup(func() { - s3Client = originalClient - s3Bucket = originalBucket - config.FilesS3TempDir.Set(originalTempDir) - }) - - tempDir := t.TempDir() - config.FilesS3TempDir.Set(tempDir) - client := &fakeS3PutObjectClient{err: errors.New("boom")} s3Client = client s3Bucket = "test-bucket" - content := []byte("non-seekable-content") - file := &File{ID: 789, Size: 0} + content := []byte("test-content") + file := &File{ID: 789, Size: uint64(len(content))} - err := file.Save(&readerOnly{r: bytes.NewReader(content)}) + err := file.Save(bytes.NewReader(content)) require.Error(t, err) assert.Contains(t, err.Error(), "failed to upload file to S3") - - entries, readErr := os.ReadDir(tempDir) - require.NoError(t, readErr) - assert.Empty(t, entries) } -func TestFileSave_S3_UsesBufferedSizeWhenExpectedSizeMismatch(t *testing.T) { +func TestFileSave_S3_LogsWarnOnSizeMismatch(t *testing.T) { originalClient := s3Client originalBucket := s3Bucket - originalTempDir := config.FilesS3TempDir.GetString() t.Cleanup(func() { s3Client = originalClient s3Bucket = originalBucket - config.FilesS3TempDir.Set(originalTempDir) }) - tempDir := t.TempDir() - config.FilesS3TempDir.Set(tempDir) - client := &fakeS3PutObjectClient{} s3Client = client s3Bucket = "test-bucket" @@ -446,14 +382,10 @@ func TestFileSave_S3_UsesBufferedSizeWhenExpectedSizeMismatch(t *testing.T) { content := []byte("mismatch-content") file := &File{ID: 999, Size: uint64(len(content) + 10)} - err := file.Save(&readerOnly{r: bytes.NewReader(content)}) + err := file.Save(bytes.NewReader(content)) require.NoError(t, err) require.NotNil(t, client.lastInput) require.NotNil(t, client.lastInput.ContentLength) assert.Equal(t, int64(len(content)), *client.lastInput.ContentLength) - - entries, readErr := os.ReadDir(tempDir) - require.NoError(t, readErr) - assert.Empty(t, entries) } diff --git a/pkg/models/task_attachment_test.go b/pkg/models/task_attachment_test.go index d42384560..abdffd029 100644 --- a/pkg/models/task_attachment_test.go +++ b/pkg/models/task_attachment_test.go @@ -17,7 +17,7 @@ package models import ( - "io" + "bytes" "os" "path/filepath" "testing" @@ -86,24 +86,6 @@ func TestTaskAttachment_ReadOne(t *testing.T) { }) } -type testfile struct { - content []byte - done bool -} - -func (t *testfile) Read(p []byte) (n int, err error) { - if t.done { - return 0, io.EOF - } - copy(p, t.content) - t.done = true - return len(p), nil -} - -func (t *testfile) Close() error { - return nil -} - func TestTaskAttachment_NewAttachment(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() @@ -114,12 +96,9 @@ func TestTaskAttachment_NewAttachment(t *testing.T) { ta := TaskAttachment{ TaskID: 1, } - tf := &testfile{ - content: []byte("testingstuff"), - } testuser := &user.User{ID: 1} - err := ta.NewAttachment(s, tf, "testfile", 100, testuser) + err := ta.NewAttachment(s, bytes.NewReader([]byte("testingstuff")), "testfile", 100, testuser) require.NoError(t, err) assert.NotEqual(t, 0, ta.FileID) _, err = files.FileStat(ta.File)