From e798a59a434d3db0dcbb225a28753a6a66059931 Mon Sep 17 00:00:00 2001 From: Antoine Aflalo <197810+Belphemur@users.noreply.github.com> Date: Tue, 10 Sep 2024 15:10:40 -0400 Subject: [PATCH] perf: fix any unhandled errors --- cbz/cbz_creator_test.go | 5 +-- cmd/optimize_command_test.go | 3 +- converter/converter_test.go | 11 +++--- utils/errs/errors_defer.go | 9 +++++ utils/errs/errors_defer_test.go | 64 +++++++++++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 8 deletions(-) diff --git a/cbz/cbz_creator_test.go b/cbz/cbz_creator_test.go index d8af32f..9c0907f 100644 --- a/cbz/cbz_creator_test.go +++ b/cbz/cbz_creator_test.go @@ -5,6 +5,7 @@ import ( "bytes" "fmt" "github.com/belphemur/CBZOptimizer/manga" + "github.com/belphemur/CBZOptimizer/utils/errs" "os" "testing" "time" @@ -95,7 +96,7 @@ func TestWriteChapterToCBZ(t *testing.T) { if err != nil { t.Fatalf("Failed to create temporary file: %v", err) } - defer os.Remove(tempFile.Name()) + defer errs.CaptureGeneric(&err, os.Remove, tempFile.Name(), "failed to remove temporary file") // Write the chapter to the .cbz file err = WriteChapterToCBZ(tc.chapter, tempFile.Name()) @@ -108,7 +109,7 @@ func TestWriteChapterToCBZ(t *testing.T) { if err != nil { t.Fatalf("Failed to open CBZ file: %v", err) } - defer r.Close() + defer errs.Capture(&err, r.Close, "failed to close CBZ file") // Collect the names of the files in the archive var filesInArchive []string diff --git a/cmd/optimize_command_test.go b/cmd/optimize_command_test.go index 66b7886..5369ed2 100644 --- a/cmd/optimize_command_test.go +++ b/cmd/optimize_command_test.go @@ -5,6 +5,7 @@ import ( "github.com/belphemur/CBZOptimizer/converter" "github.com/belphemur/CBZOptimizer/converter/constant" "github.com/belphemur/CBZOptimizer/manga" + "github.com/belphemur/CBZOptimizer/utils/errs" "github.com/spf13/cobra" "log" "os" @@ -37,7 +38,7 @@ func TestConvertCbzCommand(t *testing.T) { if err != nil { log.Fatal(err) } - defer os.RemoveAll(tempDir) // Clean up the temp directory when done + defer errs.CaptureGeneric(&err, os.RemoveAll, tempDir, "failed to remove temporary directory") // Locate the testdata directory testdataDir := filepath.Join("../testdata") diff --git a/converter/converter_test.go b/converter/converter_test.go index 33f723a..5a265da 100644 --- a/converter/converter_test.go +++ b/converter/converter_test.go @@ -4,6 +4,7 @@ import ( "bytes" "github.com/belphemur/CBZOptimizer/converter/constant" "github.com/belphemur/CBZOptimizer/manga" + "github.com/belphemur/CBZOptimizer/utils/errs" "golang.org/x/exp/slices" "image" "image/jpeg" @@ -62,7 +63,7 @@ func TestConvertChapter(t *testing.T) { t.Fatalf("failed to create temporary file: %v", err) } - defer os.Remove(temp.Name()) + defer errs.CaptureGeneric(&err, os.Remove, temp.Name(), "failed to remove temporary file") for _, converter := range Available() { converter, err := Get(converter) if err != nil { @@ -121,7 +122,7 @@ func genHugePage(path string) (*manga.Chapter, error) { if err != nil { return nil, err } - defer file.Close() + defer errs.Capture(&err, file.Close, "failed to close file") var pages []*manga.Page for i := 0; i < 1; i++ { // Assuming there are 5 pages for the test @@ -150,7 +151,7 @@ func genSmallPages(path string) (*manga.Chapter, error) { if err != nil { return nil, err } - defer file.Close() + defer errs.Capture(&err, file.Close, "failed to close file") var pages []*manga.Page for i := 0; i < 5; i++ { // Assuming there are 5 pages for the test @@ -179,7 +180,7 @@ func genMixSmallBig(path string) (*manga.Chapter, error) { if err != nil { return nil, err } - defer file.Close() + defer errs.Capture(&err, file.Close, "failed to close file") var pages []*manga.Page for i := 0; i < 5; i++ { // Assuming there are 5 pages for the test @@ -208,7 +209,7 @@ func genMixSmallHuge(path string) (*manga.Chapter, error) { if err != nil { return nil, err } - defer file.Close() + defer errs.Capture(&err, file.Close, "failed to close file") var pages []*manga.Page for i := 0; i < 10; i++ { // Assuming there are 5 pages for the test diff --git a/utils/errs/errors_defer.go b/utils/errs/errors_defer.go index 050bf6d..8bc4445 100644 --- a/utils/errs/errors_defer.go +++ b/utils/errs/errors_defer.go @@ -14,3 +14,12 @@ func Capture(errPtr *error, errFunc func() error, msg string) { } *errPtr = errors.Join(*errPtr, fmt.Errorf("%s: %w", msg, err)) } + +// CaptureGeneric runs errFunc with a generic type K and assigns the error, if any, to *errPtr. +func CaptureGeneric[K any](errPtr *error, errFunc func(value K) error, value K, msg string) { + err := errFunc(value) + if err == nil { + return + } + *errPtr = errors.Join(*errPtr, fmt.Errorf("%s: %w", msg, err)) +} diff --git a/utils/errs/errors_defer_test.go b/utils/errs/errors_defer_test.go index d4b2d83..b5bb59f 100644 --- a/utils/errs/errors_defer_test.go +++ b/utils/errs/errors_defer_test.go @@ -56,3 +56,67 @@ func TestCapture(t *testing.T) { }) } } + +func TestCaptureGeneric(t *testing.T) { + tests := []struct { + name string + initial error + errFunc func(int) error + value int + msg string + expected string + }{ + { + name: "No error from errFunc", + initial: nil, + errFunc: func(value int) error { return nil }, + value: 0, + msg: "test message", + expected: "", + }, + { + name: "Error from errFunc with no initial error", + initial: nil, + errFunc: func(value int) error { return errors.New("error from func") }, + value: 0, + msg: "test message", + expected: "test message: error from func", + }, + { + name: "Error from errFunc with initial error", + initial: errors.New("initial error"), + errFunc: func(value int) error { return errors.New("error from func") }, + value: 0, + msg: "test message", + expected: "initial error\ntest message: error from func", + }, + { + name: "Error from errFunc with initial wrapped error", + initial: fmt.Errorf("wrapped error: %w", errors.New("initial error")), + errFunc: func(value int) error { return errors.New("error from func") }, + value: 0, + msg: "test message", + expected: "wrapped error: initial error\ntest message: error from func", + }, + { + name: "Error from errFunc with initial wrapped error and value", + initial: fmt.Errorf("wrapped error: %w", errors.New("initial error")), + errFunc: func(value int) error { return fmt.Errorf("hello error:%d", value) }, + value: 1, + msg: "test message", + expected: "wrapped error: initial error\ntest message: hello error:1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var err error = tt.initial + CaptureGeneric(&err, tt.errFunc, tt.value, tt.msg) + if err != nil && err.Error() != tt.expected { + t.Errorf("expected %q, got %q", tt.expected, err.Error()) + } else if err == nil && tt.expected != "" { + t.Errorf("expected %q, got nil", tt.expected) + } + }) + } +}