fix: address PR review comments from copilot

- Remove unreachable --version flag check in local command
- Fix prepareStdinValues to handle all stdin value files
- Add proper error handling for os.Pipe and ReadFrom in tests
- Handle errors in fake helm test helpers

Signed-off-by: yxxhero <aiopsclub@163.com>

Author: yxxhero

cmd/local.go

@@ -64,14 +64,8 @@ func localCmd() *cobra.Command {
 			"  helm diff local /path/to/chart-a /path/to/chart-b --set replicas=3",
 		}, "\n"),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			// Suppress the command usage on error. See #77 for more info
 			cmd.SilenceUsage = true
 
-			if v, _ := cmd.Flags().GetBool("version"); v {
-				fmt.Println(Version)
-				return nil
-			}
-
 			if err := checkArgsLength(len(args), "chart1 path", "chart2 path"); err != nil {
 				return err
 			}
@@ -154,32 +148,40 @@ func (l *local) run() error {
 }
 
 func (l *local) prepareStdinValues() (func(), error) {
+	var name string
+
 	for i, valueFile := range l.valueFiles {
 		if strings.TrimSpace(valueFile) == "-" {
-			data, err := io.ReadAll(os.Stdin)
-			if err != nil {
-				return nil, err
+			if name == "" {
+				data, err := io.ReadAll(os.Stdin)
+				if err != nil {
+					return nil, err
+				}
+
+				tmpfile, err := os.CreateTemp("", "helm-diff-stdin-values")
+				if err != nil {
+					return nil, err
+				}
+
+				if _, err := tmpfile.Write(data); err != nil {
+					_ = tmpfile.Close()
+					return nil, err
+				}
+
+				if err := tmpfile.Close(); err != nil {
+					return nil, err
+				}
+
+				name = tmpfile.Name()
 			}
 
-			tmpfile, err := os.CreateTemp("", "helm-diff-stdin-values")
-			if err != nil {
-				return nil, err
-			}
-
-			if _, err := tmpfile.Write(data); err != nil {
-				_ = tmpfile.Close()
-				return nil, err
-			}
-
-			if err := tmpfile.Close(); err != nil {
-				return nil, err
-			}
-
-			l.valueFiles[i] = tmpfile.Name()
-			name := tmpfile.Name()
-			return func() { _ = os.Remove(name) }, nil
+			l.valueFiles[i] = name
 		}
 	}
+
+	if name != "" {
+		return func() { _ = os.Remove(name) }, nil
+	}
 	return nil, nil
 }
 

cmd/local_test.go

@@ -87,13 +87,16 @@ data:
 	chart2 := t.TempDir()
 
 	oldStdout := os.Stdout
-	r, w, _ := os.Pipe()
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("Failed to create pipe: %v", err)
+	}
 	os.Stdout = w
 
 	cmd := localCmd()
 	cmd.SetArgs([]string{chart1, chart2})
 
-	err := cmd.Execute()
+	err = cmd.Execute()
 	w.Close()
 	os.Stdout = oldStdout
 
@@ -102,7 +105,10 @@ data:
 	}
 
 	var buf bytes.Buffer
-	buf.ReadFrom(r)
+	if _, err := buf.ReadFrom(r); err != nil {
+		t.Fatalf("Failed to read from pipe: %v", err)
+	}
+	r.Close()
 	if buf.String() != "" {
 		t.Errorf("Expected no output when charts are identical, got: %q", buf.String())
 	}
@@ -133,13 +139,16 @@ data:
 	chart2 := t.TempDir()
 
 	oldStdout := os.Stdout
-	r, w, _ := os.Pipe()
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("Failed to create pipe: %v", err)
+	}
 	os.Stdout = w
 
 	cmd := localCmd()
 	cmd.SetArgs([]string{chart1, chart2})
 
-	err := cmd.Execute()
+	err = cmd.Execute()
 	w.Close()
 	os.Stdout = oldStdout
 
@@ -148,7 +157,10 @@ data:
 	}
 
 	var buf bytes.Buffer
-	buf.ReadFrom(r)
+	if _, err := buf.ReadFrom(r); err != nil {
+		t.Fatalf("Failed to read from pipe: %v", err)
+	}
+	r.Close()
 	output := buf.String()
 
 	if !strings.Contains(output, "value1") || !strings.Contains(output, "value2") {
@@ -244,7 +256,10 @@ data:
 		t.Fatalf("Expected no error but got: %v", err)
 	}
 
-	args1, _ := os.ReadFile(argsFile)
+	args1, err := os.ReadFile(argsFile)
+	if err != nil {
+		t.Fatalf("Expected fake helm args file to be readable, but got: %v", err)
+	}
 	if !strings.Contains(string(args1), "--namespace myns") {
 		t.Errorf("Expected --namespace myns in helm template args, got: %q", string(args1))
 	}

cmd/main_test.go

@@ -16,13 +16,23 @@ func TestMain(m *testing.M) {
 			os.Exit(1)
 		case "dual":
 			countFile := os.Getenv("HELM_DIFF_FAKE_COUNT_FILE")
-			data, _ := os.ReadFile(countFile)
+			data, err := os.ReadFile(countFile)
+			if err != nil && !os.IsNotExist(err) {
+				fmt.Fprintf(os.Stderr, "failed to read count file %q: %v\n", countFile, err)
+				os.Exit(1)
+			}
 			count := 0
 			if len(data) > 0 {
-				fmt.Sscanf(string(data), "%d", &count)
+				if _, err := fmt.Sscanf(string(data), "%d", &count); err != nil {
+					fmt.Fprintf(os.Stderr, "failed to parse count from %q: %v\n", string(data), err)
+					os.Exit(1)
+				}
 			}
 			count++
-			os.WriteFile(countFile, []byte(fmt.Sprintf("%d", count)), 0644)
+			if err := os.WriteFile(countFile, []byte(fmt.Sprintf("%d", count)), 0644); err != nil {
+				fmt.Fprintf(os.Stderr, "failed to write count file %q: %v\n", countFile, err)
+				os.Exit(1)
+			}
 			if count == 1 {
 				fmt.Print(os.Getenv("HELM_DIFF_FAKE_OUTPUT_1"))
 			} else {
@@ -31,7 +41,10 @@ func TestMain(m *testing.M) {
 		case "capture_args":
 			argsFile := os.Getenv("HELM_DIFF_FAKE_ARGS_FILE")
 			if argsFile != "" {
-				os.WriteFile(argsFile, []byte(strings.Join(os.Args[1:], " ")), 0644)
+				if err := os.WriteFile(argsFile, []byte(strings.Join(os.Args[1:], " ")), 0644); err != nil {
+					fmt.Fprintf(os.Stderr, "failed to write fake helm args file %q: %v\n", argsFile, err)
+					os.Exit(1)
+				}
 			}
 			fmt.Print(os.Getenv("HELM_DIFF_FAKE_OUTPUT"))
 		default: