diff --git a/pkg/state/issue_2355_test.go b/pkg/state/issue_2355_test.go index b0a93f46..da08d74c 100644 --- a/pkg/state/issue_2355_test.go +++ b/pkg/state/issue_2355_test.go @@ -123,9 +123,21 @@ func TestDryRunServerWithExistingTemplateArgs(t *testing.T) { // shouldAddDryRunServer determines whether to add --dry-run=server to template args. // This helper function encapsulates the logic from processChartification for testing. // -// IMPORTANT: This function duplicates the command classification logic from -// processChartification() in state.go (lines 1497-1507). If new commands are added -// or the classification changes in state.go, this function must be updated to match. +// NOTE ON DUPLICATION: This function intentionally duplicates the command classification +// logic from processChartification() in state.go (lines 1497-1523). While extracting this +// into a shared function would reduce duplication, it would require: +// 1. Exposing internal implementation details in the public API +// 2. Complex refactoring of processChartification which has many dependencies (chartify +// library, filesystem, HelmState) +// +// For this focused bug fix, the duplication is acceptable because: +// - The integration test (test/integration/test-cases/issue-2355.sh) exercises the actual +// processChartification code path end-to-end +// - This unit test documents the expected behavior and catches regressions quickly +// - The logic being tested is simple and unlikely to change frequently +// +// SYNC WARNING: If the command classification in processChartification() changes +// (state.go lines 1497-1507), this function must be updated to match. // // Parameters: // - helmfileCommand: the helmfile command being run (diff, apply, template, etc.) @@ -135,7 +147,7 @@ func TestDryRunServerWithExistingTemplateArgs(t *testing.T) { // Returns the updated template args string. func shouldAddDryRunServer(helmfileCommand string, validate bool, existingTemplateArgs string) string { // Determine if the command requires cluster access - // NOTE: This must be kept in sync with processChartification() in state.go + // SYNC: Keep in sync with processChartification() in state.go lines 1497-1507 var requiresCluster bool switch helmfileCommand { case "diff", "apply", "sync", "destroy", "delete", "test", "status": diff --git a/test/integration/test-cases/issue-2355.sh b/test/integration/test-cases/issue-2355.sh index 9966333e..e733015f 100755 --- a/test/integration/test-cases/issue-2355.sh +++ b/test/integration/test-cases/issue-2355.sh @@ -14,10 +14,9 @@ test_start "issue-2355: validate flag with kustomize charts (Helm 4 compatibilit # Test 1: helmfile diff --validate with kustomize chart should NOT fail due to validate/dry-run mutual exclusion # We deliberately use diff here because it is a cluster-requiring command that can trigger --dry-run=server via chartify. info "Test 1: Running diff --validate with kustomize chart" -if ! ${helmfile} -f ${issue_2355_input_dir}/helmfile.yaml diff --validate > /dev/null 2>&1; then - # Capture the error for debugging - error_output=$(${helmfile} -f ${issue_2355_input_dir}/helmfile.yaml diff --validate 2>&1) - +error_output=$(${helmfile} -f ${issue_2355_input_dir}/helmfile.yaml diff --validate 2>&1) +exit_code=$? +if [ $exit_code -ne 0 ]; then # Check if it's the specific mutual exclusion error we're fixing if echo "$error_output" | grep -q "validate.*dry-run.*were all set"; then fail "helmfile diff --validate with kustomize failed with mutual exclusion error (issue #2355 not fixed): $error_output" @@ -29,8 +28,9 @@ fi # Test 2: Verify that without --validate, the command does not hit the mutual exclusion error info "Test 2: Running diff without --validate (baseline test for flag interaction)" -if ! ${helmfile} -f ${issue_2355_input_dir}/helmfile.yaml diff > /dev/null 2>&1; then - error_output=$(${helmfile} -f ${issue_2355_input_dir}/helmfile.yaml diff 2>&1) +error_output=$(${helmfile} -f ${issue_2355_input_dir}/helmfile.yaml diff 2>&1) +exit_code=$? +if [ $exit_code -ne 0 ]; then if echo "$error_output" | grep -q "validate.*dry-run.*were all set"; then fail "helmfile diff without --validate failed with mutual exclusion error (issue #2355 not fixed): $error_output" else