Kaynağa Gözat

Harden CLI execution and error reporting

Paul Klumpp 2 hafta önce
ebeveyn
işleme
f1ac4132ef

+ 10 - 4
cmd/wobkey/main.go

@@ -10,14 +10,20 @@ import (
 	"github.com/wobkey/rgb/internal/device"
 )
 
-var rootCmd = &cobra.Command{
-	Use:   "wobkey",
-	Short: "Wobkey RGB CLI — control VIA-compatible keyboard lighting",
+func newRootCommand() *cobra.Command {
+	return &cobra.Command{
+		Use:           "wobkey",
+		Short:         "Wobkey RGB CLI — control VIA-compatible keyboard lighting",
+		SilenceErrors: true,
+		SilenceUsage:  true,
+	}
 }
 
+var rootCmd = newRootCommand()
+
 func Execute() {
 	if err := rootCmd.Execute(); err != nil {
-		fmt.Fprintln(os.Stderr, err)
+		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
 		os.Exit(1)
 	}
 }

+ 31 - 0
cmd/wobkey/main_test.go

@@ -0,0 +1,31 @@
+package main
+
+import (
+	"bytes"
+	"errors"
+	"testing"
+
+	"github.com/spf13/cobra"
+)
+
+func TestRootCommandSuppressesCobraErrorAndUsage(t *testing.T) {
+	cmd := newRootCommand()
+	var output bytes.Buffer
+	cmd.SetOut(&output)
+	cmd.SetErr(&output)
+	cmd.AddCommand(&cobra.Command{
+		Use: "probe",
+		RunE: func(*cobra.Command, []string) error {
+			return errors.New("probe failed")
+		},
+	})
+	cmd.SetArgs([]string{"probe"})
+
+	err := cmd.Execute()
+	if err == nil {
+		t.Fatal("Execute() expected error, got nil")
+	}
+	if output.Len() != 0 {
+		t.Errorf("Cobra output = %q, want no duplicate error or usage", output.String())
+	}
+}

+ 8 - 0
cmd/wobkey/rgb/commands_test.go

@@ -47,6 +47,14 @@ func (f *fakeZoneProtocol) SetColor(channel via.LEDType, hue, saturation uint8)
 	return nil
 }
 
+func (f *fakeZoneProtocol) GetValue(via.LEDType, uint8) ([]byte, error) {
+	return nil, errors.New("unexpected GetValue call")
+}
+
+func (f *fakeZoneProtocol) Close() error {
+	return nil
+}
+
 func TestSelectedChannelOperations(t *testing.T) {
 	defaultZones := []intrgb.Zone{intrgb.ZoneLogo, intrgb.ZoneBacklight, intrgb.ZoneSide}
 	sideZones := []intrgb.Zone{intrgb.ZoneSide}

+ 4 - 5
cmd/wobkey/rgb/effect.go

@@ -2,7 +2,6 @@ package rgb
 
 import (
 	"fmt"
-	"os"
 	"strings"
 
 	"github.com/spf13/cobra"
@@ -23,7 +22,7 @@ func NewEffectCmd() *cobra.Command {
 		Short: "Set RGB effect",
 		Long:  "Set the RGB lighting effect on the connected keyboard.",
 		Args:  cobra.ExactArgs(1),
-		RunE: func(_ *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			zones, err := selectedZones()
 			if err != nil {
 				return err
@@ -37,10 +36,10 @@ func NewEffectCmd() *cobra.Command {
 				for i, zone := range skipped {
 					skippedNames[i] = string(zone)
 				}
-				fmt.Fprintf(os.Stderr, "Warning: effect not supported on zone(s): %s\n", strings.Join(skippedNames, ", "))
+				fmt.Fprintf(cmd.ErrOrStderr(), "Warning: effect not supported on zone(s): %s\n", strings.Join(skippedNames, ", "))
 			}
 
-			proto, _, err := OpenDevice()
+			proto, err := openRGBProtocol()
 			if err != nil {
 				return err
 			}
@@ -52,7 +51,7 @@ func NewEffectCmd() *cobra.Command {
 				}
 			}
 
-			fmt.Printf("Effect set to %q\n", args[0])
+			fmt.Fprintf(cmd.OutOrStdout(), "Effect set to %q\n", args[0])
 			return nil
 		},
 	}

+ 75 - 0
cmd/wobkey/rgb/effect_test.go

@@ -1,7 +1,9 @@
 package rgb
 
 import (
+	"bytes"
 	"reflect"
+	"strings"
 	"testing"
 
 	intrgb "github.com/wobkey/rgb/internal/rgb"
@@ -95,3 +97,76 @@ func TestResolveEffectTargetsStaticCompatibility(t *testing.T) {
 		t.Errorf("resolveEffectTargets() skipped = %v, want none", skipped)
 	}
 }
+
+func executeEffectCommand(t *testing.T, protocol *fakeZoneProtocol, target string, args ...string) (string, string, error) {
+	t.Helper()
+	originalOpen := openRGBProtocol
+	originalZone := targetZone
+	openRGBProtocol = func() (rgbProtocol, error) {
+		return protocol, nil
+	}
+	targetZone = target
+	t.Cleanup(func() {
+		openRGBProtocol = originalOpen
+		targetZone = originalZone
+	})
+
+	var stdout bytes.Buffer
+	var stderr bytes.Buffer
+	cmd := NewEffectCmd()
+	cmd.SetOut(&stdout)
+	cmd.SetErr(&stderr)
+	cmd.SilenceErrors = true
+	cmd.SilenceUsage = true
+	cmd.SetArgs(args)
+	err := cmd.Execute()
+	return stdout.String(), stderr.String(), err
+}
+
+func TestEffectCommandWarnsForSkippedZones(t *testing.T) {
+	protocol := &fakeZoneProtocol{failAt: -1}
+	stdout, stderr, err := executeEffectCommand(t, protocol, "", "rainbow_moving_chevron")
+	if err != nil {
+		t.Fatalf("Execute() unexpected error: %v", err)
+	}
+	if !strings.Contains(stderr, "effect not supported on zone(s): logo, side") {
+		t.Errorf("stderr = %q, want skipped-zone warning", stderr)
+	}
+	if !strings.Contains(stdout, `Effect set to "rainbow_moving_chevron"`) {
+		t.Errorf("stdout = %q, want success output", stdout)
+	}
+	if len(protocol.reports) != 1 {
+		t.Fatalf("reports = %v, want one Backlight report", protocol.reports)
+	}
+	if protocol.reports[0].channel != 3 || protocol.reports[0].param != 2 || protocol.reports[0].value != 17 {
+		t.Errorf("report = %+v, want Backlight Effect 17", protocol.reports[0])
+	}
+}
+
+func TestEffectCommandRejectsBeforeOpeningProtocol(t *testing.T) {
+	protocol := &fakeZoneProtocol{failAt: -1}
+	stdout, _, err := executeEffectCommand(t, protocol, "logo", "rainbow_moving_chevron")
+	if err == nil {
+		t.Fatal("Execute() expected error, got nil")
+	}
+	if len(protocol.reports) != 0 {
+		t.Errorf("reports = %v, want no writes before rejection", protocol.reports)
+	}
+	if stdout != "" {
+		t.Errorf("stdout = %q, want no success output", stdout)
+	}
+}
+
+func TestEffectCommandSuppressesSuccessAfterLaterWriteFailure(t *testing.T) {
+	protocol := &fakeZoneProtocol{failAt: 1}
+	stdout, _, err := executeEffectCommand(t, protocol, "", "breathing")
+	if err == nil {
+		t.Fatal("Execute() expected write error, got nil")
+	}
+	if len(protocol.reports) != 1 {
+		t.Fatalf("reports = %v, want one report before failure", protocol.reports)
+	}
+	if strings.Contains(stdout, "Effect set") {
+		t.Errorf("stdout = %q, want success output suppressed", stdout)
+	}
+}

+ 3 - 3
cmd/wobkey/rgb/info.go

@@ -110,12 +110,12 @@ func NewInfoCmd() *cobra.Command {
 	return &cobra.Command{
 		Use:   "info",
 		Short: "Show current RGB state",
-		RunE: func(_ *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			zones, err := selectedZones()
 			if err != nil {
 				return err
 			}
-			proto, _, err := OpenDevice()
+			proto, err := openRGBProtocol()
 			if err != nil {
 				return err
 			}
@@ -126,7 +126,7 @@ func NewInfoCmd() *cobra.Command {
 			if err != nil {
 				return err
 			}
-			fmt.Println(string(data))
+			fmt.Fprintln(cmd.OutOrStdout(), string(data))
 			return queryErr
 		},
 	}

+ 61 - 0
cmd/wobkey/rgb/info_test.go

@@ -1,9 +1,11 @@
 package rgb
 
 import (
+	"bytes"
 	"encoding/json"
 	"errors"
 	"reflect"
+	"strings"
 	"testing"
 
 	intrgb "github.com/wobkey/rgb/internal/rgb"
@@ -30,6 +32,18 @@ func (f *fakeInfoProtocol) GetValue(channel via.LEDType, param uint8) ([]byte, e
 	return append([]byte(nil), f.values[key]...), nil
 }
 
+func (f *fakeInfoProtocol) SetValue(via.LEDType, uint8, uint8) error {
+	return errors.New("unexpected SetValue call")
+}
+
+func (f *fakeInfoProtocol) SetColor(via.LEDType, uint8, uint8) error {
+	return errors.New("unexpected SetColor call")
+}
+
+func (f *fakeInfoProtocol) Close() error {
+	return nil
+}
+
 func TestInfoSuccessfulRecord(t *testing.T) {
 	protocol := &fakeInfoProtocol{
 		values: map[infoKey][]byte{
@@ -168,3 +182,50 @@ func TestInfoErrorRecord(t *testing.T) {
 		t.Errorf("error zone JSON = %v, want error key", envelope.Zones[1])
 	}
 }
+
+func TestInfoCommandPrintsJSONBeforeReturningError(t *testing.T) {
+	protocol := &fakeInfoProtocol{
+		values: map[infoKey][]byte{
+			{channel: 2, param: 1}: {120},
+			{channel: 2, param: 2}: {4},
+			{channel: 2, param: 3}: {3},
+			{channel: 2, param: 4}: {10, 20},
+			{channel: 3, param: 1}: {90},
+			{channel: 3, param: 2}: {5},
+			{channel: 3, param: 3}: {4},
+			{channel: 3, param: 4}: {30, 40},
+		},
+		errors: map[infoKey]error{
+			{channel: 4, param: 1}: errors.New("side unavailable"),
+		},
+	}
+	originalOpen := openRGBProtocol
+	originalZone := targetZone
+	openRGBProtocol = func() (rgbProtocol, error) {
+		return protocol, nil
+	}
+	targetZone = ""
+	t.Cleanup(func() {
+		openRGBProtocol = originalOpen
+		targetZone = originalZone
+	})
+
+	var stdout bytes.Buffer
+	var stderr bytes.Buffer
+	cmd := NewInfoCmd()
+	cmd.SetOut(&stdout)
+	cmd.SetErr(&stderr)
+	cmd.SilenceErrors = true
+	cmd.SilenceUsage = true
+	cmd.SetArgs(nil)
+	err := cmd.Execute()
+	if err == nil {
+		t.Fatal("Execute() expected query error, got nil")
+	}
+	if !strings.Contains(stdout.String(), `"zones"`) || !strings.Contains(stdout.String(), `"error"`) {
+		t.Errorf("stdout = %q, want JSON containing zones and error", stdout.String())
+	}
+	if stderr.Len() != 0 {
+		t.Errorf("stderr = %q, want no Cobra usage/error noise", stderr.String())
+	}
+}

+ 14 - 0
cmd/wobkey/rgb/rgb.go

@@ -20,6 +20,20 @@ type zoneProtocol interface {
 	SetColor(via.LEDType, uint8, uint8) error
 }
 
+type rgbProtocol interface {
+	zoneProtocol
+	GetValue(via.LEDType, uint8) ([]byte, error)
+	Close() error
+}
+
+var openRGBProtocol = func() (rgbProtocol, error) {
+	proto, _, err := OpenDevice()
+	if err != nil {
+		return nil, err
+	}
+	return proto, nil
+}
+
 func forEachSelectedZone(zones []intrgb.Zone, fn func(intrgb.Zone, via.LEDType) error) error {
 	channels := zoneChannels(zones)
 	for i, zone := range zones {