fix: address Copilot review comments on preBuildTransitiveSubchartDeps

- Correct parseLocalFileDeps doc to match behavior (ok=false only when
  Chart.yaml is missing/unreadable/unparseable; may return empty slice).
- Align log prefixes with function names (parseLocalFileDeps,
  runHelmDepBuild, execHelmDep) instead of the stale preBuildSubchartDeps.
- Log execHelmDep failures at Debug (best-effort) instead of Warn.
- Add skipIfNoHelm helper and guard tests that invoke helm.
- Rename NoOpOnRemoteDeps -> DoesNotRecurseIntoRemoteDeps for accuracy.

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

pkg/state/prebuild_subchart_deps_test.go

@@ -2,6 +2,7 @@ package state
 
 import (
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -12,6 +13,16 @@ import (
 	"github.com/helmfile/helmfile/pkg/filesystem"
 )
 
+// skipIfNoHelm skips the calling test when the helm binary is not available on
+// PATH. Tests that exercise runHelmDepBuild need helm installed to produce
+// meaningful results.
+func skipIfNoHelm(t *testing.T) {
+	t.Helper()
+	if _, err := exec.LookPath("helm"); err != nil {
+		t.Skipf("skipping: helm binary not found in PATH (%v)", err)
+	}
+}
+
 // writeChart creates a chart directory with Chart.yaml and the given templates.
 // deps maps dependency name → repository URL (skipped if empty).
 func writeChart(t *testing.T, dir, name string, deps map[string]string, templates map[string]string) {
@@ -53,6 +64,7 @@ func TestPreBuildTransitiveSubchartDeps_VisitsAllTransitiveDeps(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping in short mode (requires helm)")
 	}
+	skipIfNoHelm(t)
 
 	rootDir, err := os.MkdirTemp("", "helmfile-prebuild-")
 	if err != nil {
@@ -152,6 +164,7 @@ func TestPreBuildTransitiveSubchartDeps_HandlesCircularDeps(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping in short mode (requires helm)")
 	}
+	skipIfNoHelm(t)
 
 	rootDir, err := os.MkdirTemp("", "helmfile-prebuild-cycle-")
 	if err != nil {
@@ -187,9 +200,11 @@ func TestPreBuildTransitiveSubchartDeps_HandlesCircularDeps(t *testing.T) {
 	}
 }
 
-// TestPreBuildTransitiveSubchartDeps_NoOpOnRemoteDeps verifies the function
-// skips dependencies declared with non-file:// repositories (https://, oci://).
-func TestPreBuildTransitiveSubchartDeps_NoOpOnRemoteDeps(t *testing.T) {
+// TestPreBuildTransitiveSubchartDeps_DoesNotRecurseIntoRemoteDeps verifies the
+// function does not recurse into dependencies declared with non-file://
+// repositories (https://, oci://). It still runs helm dep build on the chart
+// dir itself, but the failure to resolve the remote is logged and swallowed.
+func TestPreBuildTransitiveSubchartDeps_DoesNotRecurseIntoRemoteDeps(t *testing.T) {
 	rootDir, err := os.MkdirTemp("", "helmfile-prebuild-remote-")
 	if err != nil {
 		t.Fatalf("mkdtemp: %v", err)

pkg/state/state.go

@@ -1871,7 +1871,8 @@ func (st *HelmState) prebuildChartDeps(chartDir string, visited map[string]bool)
 
 // parseLocalFileDeps reads chartDir/Chart.yaml and returns the absolute paths
 // of all local file:// dependencies that exist as directories. Returns ok=false
-// if Chart.yaml is missing, unreadable, or has no file:// deps.
+// only when Chart.yaml is missing, unreadable, or unparseable. When ok=true the
+// returned slice may be empty (e.g. no file:// deps present).
 func (st *HelmState) parseLocalFileDeps(chartDir string) (depDirs []string, ok bool) {
 	data, err := st.fs.ReadFile(filepath.Join(chartDir, "Chart.yaml"))
 	if err != nil {
@@ -1886,7 +1887,7 @@ func (st *HelmState) parseLocalFileDeps(chartDir string) (depDirs []string, ok b
 	}
 	var meta chartMeta
 	if err := yaml.Unmarshal(data, &meta); err != nil {
-		st.logger.Debugf("preBuildSubchartDeps: failed to parse %s/Chart.yaml: %v", chartDir, err)
+		st.logger.Debugf("parseLocalFileDeps: failed to parse %s/Chart.yaml: %v", chartDir, err)
 		return nil, false
 	}
 
@@ -1940,7 +1941,7 @@ func (st *HelmState) runHelmDepBuild(chartDir string) {
 	// (re-resolves from Chart.yaml) when the lock is missing or stale.
 	output, err := st.execHelmDep(helmBin, "build", chartDir)
 	if err != nil && isLockOutOfSync(output) {
-		st.logger.Debugf("preBuildSubchartDeps: falling back to `dependency update` for %s", chartDir)
+		st.logger.Debugf("runHelmDepBuild: falling back to `dependency update` for %s", chartDir)
 		_, _ = st.execHelmDep(helmBin, "update", chartDir)
 	}
 }
@@ -1950,9 +1951,9 @@ func (st *HelmState) runHelmDepBuild(chartDir string) {
 func (st *HelmState) execHelmDep(helmBin, subcmd, chartDir string) ([]byte, error) {
 	output, err := exec.Command(helmBin, "dependency", subcmd, chartDir, "--skip-refresh").CombinedOutput()
 	if err != nil {
-		st.logger.Warnf("preBuildSubchartDeps: helm dependency %s failed for %s: %v\n%s", subcmd, chartDir, err, string(output))
+		st.logger.Debugf("execHelmDep: helm dependency %s failed for %s: %v\n%s", subcmd, chartDir, err, string(output))
 	} else {
-		st.logger.Debugf("preBuildSubchartDeps: helm dependency %s succeeded for %s", subcmd, chartDir)
+		st.logger.Debugf("execHelmDep: helm dependency %s succeeded for %s", subcmd, chartDir)
 	}
 	return output, err
 }