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>
This commit is contained in:
copilot-swe-agent[bot]
2025-12-10 20:19:10 +00:00
parent 1fa54e1936
commit a2feca6cca
4 changed files with 259 additions and 6 deletions

View File

@@ -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)
}

View File

@@ -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)
}
})
}
}

View File

@@ -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)
}