fix: improve error handling for Helm chart directory validation and add tests

Author: kumadee

artifactory/commands/helm/package.go

@@ -11,10 +11,12 @@ import (
 	"github.com/jfrog/jfrog-client-go/artifactory"
 )
 
+var ErrNoChartDirFound = errors.New("no valid Helm chart directory found in the provided paths")
+
 func isChartDir(dir string) bool {
 	chartPath := filepath.Join(dir, flexpack.ChartYaml)
 	_, err := os.Stat(chartPath)
-	return !errors.Is(err, os.ErrNotExist)
+	return err == nil
 }
 
 func handlePackageCommand(buildInfoOld *entities.BuildInfo, args []string, serviceManager artifactory.ArtifactoryServicesManager, buildName, buildNumber, project string) error {
@@ -33,7 +35,7 @@ func handlePackageCommand(buildInfoOld *entities.BuildInfo, args []string, servi
 		}
 	}
 	if chartPath == "" {
-		return fmt.Errorf("no valid Helm chart directory found in the provided paths")
+		return ErrNoChartDirFound
 	}
 	buildInfo, err := collectBuildInfoWithFlexPack(chartPath, buildName, buildNumber)
 	if err != nil {
@@ -49,7 +51,7 @@ func handlePackageCommand(buildInfoOld *entities.BuildInfo, args []string, servi
 	removeDuplicateDependencies(buildInfoOld)
 	err = saveBuildInfo(buildInfoOld, buildName, buildNumber, project)
 	if err != nil {
-		return fmt.Errorf("failed to save build info")
+		return fmt.Errorf("failed to save build info: %w", err)
 	}
 	return nil
 }

artifactory/commands/helm/package_test.go

@@ -43,11 +43,17 @@ func TestHandlePackageCommand(t *testing.T) {
 			},
 		},
 		{
-			name: "chart path with destination flag",
+			name: "chart path with destination flag - long form",
 			args: func(chartPath, destination string) []string {
 				return []string{chartPath, "--destination", destination}
 			},
 		},
+		{
+			name: "chart path with destination flag - short form",
+			args: func(chartPath, destination string) []string {
+				return []string{chartPath, "-d", destination}
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -69,6 +75,90 @@ func TestHandlePackageCommand(t *testing.T) {
 	}
 }
 
+func TestHandlePackageCommand_NoChartDir(t *testing.T) {
+	t.Setenv("JFROG_CLI_HOME_DIR", t.TempDir())
+	tests := []struct {
+		name string
+		args func(chartPath, destination string) []string
+	}{
+		{
+			name: "Destination exists but no Chart.yaml",
+			args: func(chartPath, destination string) []string {
+				return []string{chartPath, "-d", destination}
+			},
+		},
+		{
+			name: "No chart among paths",
+			args: func(chartPath, _ string) []string {
+				return []string{chartPath}
+			},
+		},
+		{
+			name: "No paths",
+			args: func(_, _ string) []string {
+				return []string{}
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			notAChartDir := t.TempDir()
+			destination := t.TempDir()
+			buildInfo := entities.New()
+			err := handlePackageCommand(
+				buildInfo,
+				tt.args(notAChartDir, destination),
+				&mockPackageServicesManager{},
+				"helm-package-test",
+				"1",
+				"",
+			)
+			require.ErrorIs(t, err, ErrNoChartDirFound, "Expected error when no valid chart directory is specified")
+			require.Empty(t, buildInfo.Modules, "Expected no modules in build info when no chart directory is found")
+		})
+	}
+}
+
+func TestIsChartDir(t *testing.T) {
+	tests := []struct {
+		name     string
+		dir      string
+		expected bool
+	}{
+		{
+			name:     "Chart directory exists and contains Chart.yaml",
+			dir:      createHelmChartWithDependencies(t),
+			expected: true,
+		},
+		{
+			name:     "Chart directory exists but does not contain Chart.yaml",
+			dir:      t.TempDir(),
+			expected: false,
+		},
+		{
+			name:     "Chart directory does not exist",
+			dir:      filepath.Join(t.TempDir(), "nonexistent"),
+			expected: false,
+		},
+		{
+			name: "Chart directory exists but permission denied",
+			dir: func() string {
+				chartPath := t.TempDir()
+				_ = os.Chmod(chartPath, 0o000) // Remove all permissions
+				return chartPath
+			}(),
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.expected, isChartDir(tt.dir))
+		})
+	}
+}
+
 func createHelmChartWithDependencies(t *testing.T) string {
 	t.Helper()
 
@@ -203,4 +293,4 @@ func requireBuildInfoDetails(t *testing.T, buildInfo *entities.BuildInfo) {
 	}
 	require.True(t, depIds["subchart-a:0.1.0"])
 	require.True(t, depIds["subchart-b:0.2.0"])
-}
+}