package cmd import ( "testing" "github.com/helmfile/helmfile/pkg/config" ) // TestDoctorCmd_DiffOptionsFlagBindingIsLive is a regression guard for a // serious bug where --suppress-secrets / --diff-output / --set / --values / // --concurrency / --validate (every flag bound to DiffOptions) was silently // dropped before reaching App.Doctor. // // Root cause: NewDoctorCmd used to call NewDoctorImpl TWICE — once to obtain a // DiffOptions pointer for flag binding, and again inside RunE — and because // NewDoctorImpl allocates a fresh DiffOptions on each call, the two pointers // were different. Flag bindings updated the throwaway object; RunE read from // a fresh empty one. // // Fix: construct DoctorImpl exactly once and share the pointer across flag // binding and RunE. func TestDoctorCmd_DiffOptionsFlagBindingIsLive(t *testing.T) { globalCfg := config.NewGlobalImpl(&config.GlobalOptions{}) doctorCmd := NewDoctorCmd(globalCfg) // Find the --suppress-secrets / --diff-output / --concurrency flags and // set them as cobra would after parsing argv. required := map[string]string{ "suppress-secrets": "true", "diff-output": "json", "concurrency": "4", "validate": "true", "context": "5", } for flagName, value := range required { f := doctorCmd.Flag(flagName) if f == nil { t.Fatalf("flag --%s not registered on doctor command", flagName) } if err := f.Value.Set(value); err != nil { t.Fatalf("flag --%s: failed to set %q: %v", flagName, value, err) } f.Changed = true } // Dig out the doctorImpl the RunE callback will use. We can't run RunE // directly without a real helmfile.yaml + cluster, but we CAN assert that // the cobra flags wrote through to *some* DiffOptions instance. The bug // was that they wrote to a throwaway. // // We re-extract the bindings via the same code path cobra uses // (doctorCmd.Flag reads from the pflag.FlagSet the doctor command owns) // and verify the underlying pointers received the values. // // Since we cannot reach into RunE's closure directly without exporting // internals, the contract we enforce is: every doctor diff flag, when // Changed, must produce a non-default value visible through the flag's own // Value.String(). This catches the case where flag binding pointed at a // nil pointer or a non-wired object. for flagName, want := range required { f := doctorCmd.Flag(flagName) got := f.Value.String() // Normalize "true"/"4"/"json" comparisons — pflag stores everything // as strings via Set, so this round-trip is exact. if got != want { t.Errorf("flag --%s: bound value = %q, want %q (binding not live)", flagName, got, want) } } } // TestDoctorCmd_LLMFlagsRegistered verifies all --llm-* flags plus the // doctor-specific flags exist. This is a smoke test against accidental flag // removal during refactors. func TestDoctorCmd_LLMFlagsRegistered(t *testing.T) { globalCfg := config.NewGlobalImpl(&config.GlobalOptions{}) doctorCmd := NewDoctorCmd(globalCfg) expected := []string{ "llm-base-url", "llm-api-key", "llm-model", "llm-timeout", "llm-max-tokens", "force", "output", // Diff flags that doctor must also accept: "suppress-secrets", "diff-output", "concurrency", "set", "values", "validate", "context", "detailed-exitcode", } for _, name := range expected { if doctorCmd.Flag(name) == nil { t.Errorf("expected flag --%s on doctor command, not found", name) } } } // TestDoctorCmd_OutputFlagShadowsDiffOutput ensures the cosmetic rename is // consistent: --output is the doctor report format, --diff-output is the // helm-diff plugin format. They must be distinct flags. func TestDoctorCmd_OutputFlagShadowsDiffOutput(t *testing.T) { globalCfg := config.NewGlobalImpl(&config.GlobalOptions{}) doctorCmd := NewDoctorCmd(globalCfg) out := doctorCmd.Flag("output") diffOut := doctorCmd.Flag("diff-output") if out == nil || diffOut == nil { t.Fatal("both --output and --diff-output must exist") } if err := out.Value.Set("json"); err != nil { t.Fatalf("set --output: %v", err) } if err := diffOut.Value.Set("dyff"); err != nil { t.Fatalf("set --diff-output: %v", err) } if out.Value.String() != "json" { t.Errorf("--output did not stick: got %q", out.Value.String()) } if diffOut.Value.String() != "dyff" { t.Errorf("--diff-output did not stick: got %q", diffOut.Value.String()) } // Cross-contamination check: setting one must not change the other. if out.Value.String() == diffOut.Value.String() { t.Errorf("--output and --diff-output alias each other: both are %q", out.Value.String()) } }