From a2feca6ccae899d4267087d0acc30a69ac5cdb40 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 10 Dec 2025 20:19:10 +0000 Subject: [PATCH] Fix format flag crash and add comprehensive tests - Remove NoOptDefVal which caused the format flag to fail with space-separated values - Add 5 comprehensive unit tests for format flag: space syntax, short form, equals syntax, default value, and case-insensitive - Update README with detailed format flag documentation and examples - Format flag now works with all syntaxes: --format webp, -f webp, --format=webp - Default value (webp) is preserved and shown in help text Co-authored-by: Belphemur <197810+Belphemur@users.noreply.github.com> --- README.md | 20 +- cmd/cbzoptimizer/commands/optimize_command.go | 3 +- .../commands/optimize_command_test.go | 237 ++++++++++++++++++ cmd/cbzoptimizer/commands/watch_command.go | 5 +- 4 files changed, 259 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index a882293..e01b24c 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,22 @@ Optimize all CBZ/CBR files in a folder recursively: cbzconverter optimize [folder] --quality 85 --parallelism 2 --override --format webp --split ``` +The format flag can be specified in multiple ways: + +```sh +# Using space-separated syntax +cbzconverter optimize [folder] --format webp + +# Using short form with space +cbzconverter optimize [folder] -f webp + +# Using equals syntax +cbzconverter optimize [folder] --format=webp + +# Format is case-insensitive +cbzconverter optimize [folder] --format WEBP +``` + With timeout to avoid hanging on problematic chapters: ```sh @@ -74,7 +90,9 @@ docker run -v /path/to/comics:/comics ghcr.io/belphemur/cbzoptimizer:latest watc - `--parallelism`, `-n`: Number of chapters to convert in parallel. Default is 2. - `--override`, `-o`: Override the original files. For CBZ files, overwrites the original. For CBR files, deletes the original CBR and creates a new CBZ. Default is false. - `--split`, `-s`: Split long pages into smaller chunks. Default is false. -- `--format`, `-f`: Format to convert the images to (e.g., webp). Default is webp. +- `--format`, `-f`: Format to convert the images to (currently supports: webp). Default is webp. + - Can be specified as: `--format webp`, `-f webp`, or `--format=webp` + - Case-insensitive: `webp`, `WEBP`, and `WebP` are all valid - `--timeout`, `-t`: Maximum time allowed for converting a single chapter (e.g., 30s, 5m, 1h). 0 means no timeout. Default is 0. - `--log`, `-l`: Set log level; can be 'panic', 'fatal', 'error', 'warn', 'info', 'debug', or 'trace'. Default is info. diff --git a/cmd/cbzoptimizer/commands/optimize_command.go b/cmd/cbzoptimizer/commands/optimize_command.go index 9149f3a..82a483a 100644 --- a/cmd/cbzoptimizer/commands/optimize_command.go +++ b/cmd/cbzoptimizer/commands/optimize_command.go @@ -33,11 +33,10 @@ func init() { command.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files") command.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks") command.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter (e.g., 30s, 5m, 1h). 0 means no timeout") - command.PersistentFlags().VarP( + command.Flags().VarP( formatFlag, "format", "f", fmt.Sprintf("Format to convert the images to: %s", constant.ListAll())) - command.PersistentFlags().Lookup("format").NoOptDefVal = constant.DefaultConversion.String() AddCommand(command) } diff --git a/cmd/cbzoptimizer/commands/optimize_command_test.go b/cmd/cbzoptimizer/commands/optimize_command_test.go index 92f6484..c758e37 100644 --- a/cmd/cbzoptimizer/commands/optimize_command_test.go +++ b/cmd/cbzoptimizer/commands/optimize_command_test.go @@ -15,6 +15,7 @@ import ( "github.com/belphemur/CBZOptimizer/v2/pkg/converter" "github.com/belphemur/CBZOptimizer/v2/pkg/converter/constant" "github.com/spf13/cobra" + "github.com/thediveo/enumflag/v2" ) // MockConverter is a mock implementation of the Converter interface @@ -172,3 +173,239 @@ func TestConvertCbzCommand(t *testing.T) { // Log summary t.Logf("Found %d converted files", len(convertedFiles)) } + +// TestFormatFlagWithSpace tests that the format flag works with space-separated values +func TestFormatFlagWithSpace(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "test_format_space") + if err != nil { + t.Fatalf("Failed to create temp directory: %v", err) + } + defer os.RemoveAll(tempDir) + + // Mock the converter.Get function + originalGet := converter.Get + converter.Get = func(format constant.ConversionFormat) (converter.Converter, error) { + return &MockConverter{}, nil + } + defer func() { converter.Get = originalGet }() + + // Set up the command + cmd := &cobra.Command{ + Use: "optimize", + } + cmd.Flags().Uint8P("quality", "q", 85, "Quality for conversion (0-100)") + cmd.Flags().IntP("parallelism", "n", 1, "Number of chapters to convert in parallel") + cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files") + cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks") + cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter") + + // Reset converterType to default before test + converterType = constant.WebP + formatFlag := enumflag.New(&converterType, "format", constant.CommandValue, enumflag.EnumCaseInsensitive) + cmd.Flags().VarP(formatFlag, "format", "f", "Format to convert the images to") + + // Test with space-separated format flag (--format webp) + cmd.ParseFlags([]string{"--format", "webp"}) + + // Execute the command + err = ConvertCbzCommand(cmd, []string{tempDir}) + if err != nil { + t.Fatalf("Command execution failed with --format webp: %v", err) + } + + // Verify the format was set correctly + if converterType != constant.WebP { + t.Errorf("Expected format to be WebP, got %v", converterType) + } +} + +// TestFormatFlagWithShortForm tests that the short form of format flag works with space-separated values +func TestFormatFlagWithShortForm(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "test_format_short") + if err != nil { + t.Fatalf("Failed to create temp directory: %v", err) + } + defer os.RemoveAll(tempDir) + + // Mock the converter.Get function + originalGet := converter.Get + converter.Get = func(format constant.ConversionFormat) (converter.Converter, error) { + return &MockConverter{}, nil + } + defer func() { converter.Get = originalGet }() + + // Set up the command + cmd := &cobra.Command{ + Use: "optimize", + } + cmd.Flags().Uint8P("quality", "q", 85, "Quality for conversion (0-100)") + cmd.Flags().IntP("parallelism", "n", 1, "Number of chapters to convert in parallel") + cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files") + cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks") + cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter") + + // Reset converterType to default before test + converterType = constant.WebP + formatFlag := enumflag.New(&converterType, "format", constant.CommandValue, enumflag.EnumCaseInsensitive) + cmd.Flags().VarP(formatFlag, "format", "f", "Format to convert the images to") + + // Test with short form and space (-f webp) + cmd.ParseFlags([]string{"-f", "webp"}) + + // Execute the command + err = ConvertCbzCommand(cmd, []string{tempDir}) + if err != nil { + t.Fatalf("Command execution failed with -f webp: %v", err) + } + + // Verify the format was set correctly + if converterType != constant.WebP { + t.Errorf("Expected format to be WebP, got %v", converterType) + } +} + +// TestFormatFlagWithEquals tests that the format flag works with equals syntax +func TestFormatFlagWithEquals(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "test_format_equals") + if err != nil { + t.Fatalf("Failed to create temp directory: %v", err) + } + defer os.RemoveAll(tempDir) + + // Mock the converter.Get function + originalGet := converter.Get + converter.Get = func(format constant.ConversionFormat) (converter.Converter, error) { + return &MockConverter{}, nil + } + defer func() { converter.Get = originalGet }() + + // Set up the command + cmd := &cobra.Command{ + Use: "optimize", + } + cmd.Flags().Uint8P("quality", "q", 85, "Quality for conversion (0-100)") + cmd.Flags().IntP("parallelism", "n", 1, "Number of chapters to convert in parallel") + cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files") + cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks") + cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter") + + // Reset converterType to default before test + converterType = constant.WebP + formatFlag := enumflag.New(&converterType, "format", constant.CommandValue, enumflag.EnumCaseInsensitive) + cmd.Flags().VarP(formatFlag, "format", "f", "Format to convert the images to") + + // Test with equals syntax (--format=webp) + cmd.ParseFlags([]string{"--format=webp"}) + + // Execute the command + err = ConvertCbzCommand(cmd, []string{tempDir}) + if err != nil { + t.Fatalf("Command execution failed with --format=webp: %v", err) + } + + // Verify the format was set correctly + if converterType != constant.WebP { + t.Errorf("Expected format to be WebP, got %v", converterType) + } +} + +// TestFormatFlagDefaultValue tests that the default format is used when flag is not provided +func TestFormatFlagDefaultValue(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "test_format_default") + if err != nil { + t.Fatalf("Failed to create temp directory: %v", err) + } + defer os.RemoveAll(tempDir) + + // Mock the converter.Get function + originalGet := converter.Get + converter.Get = func(format constant.ConversionFormat) (converter.Converter, error) { + return &MockConverter{}, nil + } + defer func() { converter.Get = originalGet }() + + // Set up the command + cmd := &cobra.Command{ + Use: "optimize", + } + cmd.Flags().Uint8P("quality", "q", 85, "Quality for conversion (0-100)") + cmd.Flags().IntP("parallelism", "n", 1, "Number of chapters to convert in parallel") + cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files") + cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks") + cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter") + + // Reset converterType to default before test + converterType = constant.DefaultConversion + formatFlag := enumflag.New(&converterType, "format", constant.CommandValue, enumflag.EnumCaseInsensitive) + cmd.Flags().VarP(formatFlag, "format", "f", "Format to convert the images to") + + // Don't set format flag - should use default + cmd.ParseFlags([]string{}) + + // Execute the command + err = ConvertCbzCommand(cmd, []string{tempDir}) + if err != nil { + t.Fatalf("Command execution failed with default format: %v", err) + } + + // Verify the default format is used + if converterType != constant.DefaultConversion { + t.Errorf("Expected format to be default (%v), got %v", constant.DefaultConversion, converterType) + } +} + +// TestFormatFlagCaseInsensitive tests that the format flag is case-insensitive +func TestFormatFlagCaseInsensitive(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "test_format_case") + if err != nil { + t.Fatalf("Failed to create temp directory: %v", err) + } + defer os.RemoveAll(tempDir) + + // Mock the converter.Get function + originalGet := converter.Get + converter.Get = func(format constant.ConversionFormat) (converter.Converter, error) { + return &MockConverter{}, nil + } + defer func() { converter.Get = originalGet }() + + testCases := []string{"webp", "WEBP", "WebP", "WeBp"} + + for _, formatValue := range testCases { + t.Run(formatValue, func(t *testing.T) { + // Set up the command + cmd := &cobra.Command{ + Use: "optimize", + } + cmd.Flags().Uint8P("quality", "q", 85, "Quality for conversion (0-100)") + cmd.Flags().IntP("parallelism", "n", 1, "Number of chapters to convert in parallel") + cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files") + cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks") + cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter") + + // Reset converterType to default before test + converterType = constant.WebP + formatFlag := enumflag.New(&converterType, "format", constant.CommandValue, enumflag.EnumCaseInsensitive) + cmd.Flags().VarP(formatFlag, "format", "f", "Format to convert the images to") + + // Test with different case variations + cmd.ParseFlags([]string{"--format", formatValue}) + + // Execute the command + err = ConvertCbzCommand(cmd, []string{tempDir}) + if err != nil { + t.Fatalf("Command execution failed with format '%s': %v", formatValue, err) + } + + // Verify the format was set correctly + if converterType != constant.WebP { + t.Errorf("Expected format to be WebP for input '%s', got %v", formatValue, converterType) + } + }) + } +} diff --git a/cmd/cbzoptimizer/commands/watch_command.go b/cmd/cbzoptimizer/commands/watch_command.go index 5a9105b..8dfcdc4 100644 --- a/cmd/cbzoptimizer/commands/watch_command.go +++ b/cmd/cbzoptimizer/commands/watch_command.go @@ -42,12 +42,11 @@ func init() { command.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter (e.g., 30s, 5m, 1h). 0 means no timeout") _ = viper.BindPFlag("timeout", command.Flags().Lookup("timeout")) - command.PersistentFlags().VarP( + command.Flags().VarP( formatFlag, "format", "f", fmt.Sprintf("Format to convert the images to: %s", constant.ListAll())) - command.PersistentFlags().Lookup("format").NoOptDefVal = constant.DefaultConversion.String() - _ = viper.BindPFlag("format", command.PersistentFlags().Lookup("format")) + _ = viper.BindPFlag("format", command.Flags().Lookup("format")) AddCommand(command) }