Fix tests not using New func after introducing common flags

Signed-off-by: yxxhero <aiopsclub@163.com>
This commit is contained in:
Hubertbits 2025-04-11 23:11:53 +02:00 committed by yxxhero
parent 3cc8acb31a
commit 7219273d3b
12 changed files with 157 additions and 54 deletions

View File

@ -80,7 +80,7 @@ func TestApply_hooks(t *testing.T) {
app.Selectors = tc.selectors app.Selectors = tc.selectors
} }
syncErr := app.Apply(applyConfig{ syncErr := app.Apply(NewApplyConfigWithDefaults(&applyConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: tc.concurrency, concurrency: tc.concurrency,
logger: logger, logger: logger,
@ -88,7 +88,7 @@ func TestApply_hooks(t *testing.T) {
skipNeeds: tc.fields.skipNeeds, skipNeeds: tc.fields.skipNeeds,
includeNeeds: tc.fields.includeNeeds, includeNeeds: tc.fields.includeNeeds,
includeTransitiveNeeds: tc.fields.includeTransitiveNeeds, includeTransitiveNeeds: tc.fields.includeTransitiveNeeds,
}) }))
var gotErr string var gotErr string
if syncErr != nil { if syncErr != nil {

View File

@ -81,7 +81,7 @@ func TestApply_3(t *testing.T) {
app.Selectors = tc.selectors app.Selectors = tc.selectors
} }
syncErr := app.Apply(applyConfig{ syncErr := app.Apply(NewApplyConfigWithDefaults(&applyConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: tc.concurrency, concurrency: tc.concurrency,
logger: logger, logger: logger,
@ -89,7 +89,7 @@ func TestApply_3(t *testing.T) {
skipNeeds: tc.fields.skipNeeds, skipNeeds: tc.fields.skipNeeds,
includeNeeds: tc.fields.includeNeeds, includeNeeds: tc.fields.includeNeeds,
includeTransitiveNeeds: tc.fields.includeTransitiveNeeds, includeTransitiveNeeds: tc.fields.includeTransitiveNeeds,
}) }))
var gotErr string var gotErr string
if syncErr != nil { if syncErr != nil {

View File

@ -81,7 +81,7 @@ func TestApply_2(t *testing.T) {
app.Selectors = tc.selectors app.Selectors = tc.selectors
} }
syncErr := app.Apply(applyConfig{ syncErr := app.Apply(NewApplyConfigWithDefaults(&applyConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: tc.concurrency, concurrency: tc.concurrency,
logger: logger, logger: logger,
@ -89,7 +89,7 @@ func TestApply_2(t *testing.T) {
skipNeeds: tc.fields.skipNeeds, skipNeeds: tc.fields.skipNeeds,
includeNeeds: tc.fields.includeNeeds, includeNeeds: tc.fields.includeNeeds,
includeTransitiveNeeds: tc.fields.includeTransitiveNeeds, includeTransitiveNeeds: tc.fields.includeTransitiveNeeds,
}) }))
var gotErr string var gotErr string
if syncErr != nil { if syncErr != nil {

View File

@ -121,7 +121,7 @@ releases:
app.Selectors = tc.selectors app.Selectors = tc.selectors
} }
diffErr := app.Diff(applyConfig{ diffErr := app.Diff(NewApplyConfigWithDefaults(&applyConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: 1, concurrency: 1,
logger: logger, logger: logger,
@ -129,7 +129,7 @@ releases:
includeNeeds: tc.fields.includeNeeds, includeNeeds: tc.fields.includeNeeds,
includeTransitiveNeeds: tc.fields.includeTransitiveNeeds, includeTransitiveNeeds: tc.fields.includeTransitiveNeeds,
noHooks: tc.fields.noHooks, noHooks: tc.fields.noHooks,
}) }))
var gotErr string var gotErr string
if diffErr != nil { if diffErr != nil {
@ -352,12 +352,12 @@ func TestDiffWithInstalled(t *testing.T) {
app.Selectors = tc.selectors app.Selectors = tc.selectors
} }
diffErr := app.Diff(applyConfig{ diffErr := app.Diff(NewApplyConfigWithDefaults(&applyConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: 1, concurrency: 1,
logger: logger, logger: logger,
skipNeeds: true, skipNeeds: true,
}) }))
var gotErr string var gotErr string
if diffErr != nil { if diffErr != nil {

View File

@ -123,14 +123,14 @@ releases:
app.Selectors = tc.selectors app.Selectors = tc.selectors
} }
lintErr := app.Lint(applyConfig{ lintErr := app.Lint(NewApplyConfigWithDefaults(&applyConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: 1, concurrency: 1,
logger: logger, logger: logger,
skipNeeds: tc.fields.skipNeeds, skipNeeds: tc.fields.skipNeeds,
includeNeeds: tc.fields.includeNeeds, includeNeeds: tc.fields.includeNeeds,
includeTransitiveNeeds: tc.fields.includeTransitiveNeeds, includeTransitiveNeeds: tc.fields.includeTransitiveNeeds,
}) }))
var gotErr string var gotErr string
if lintErr != nil { if lintErr != nil {

View File

@ -223,10 +223,10 @@ database my-app true true chart:postgres,name:da
func TestListWithEnvironment(t *testing.T) { func TestListWithEnvironment(t *testing.T) {
t.Run("with skipCharts=false", func(t *testing.T) { t.Run("with skipCharts=false", func(t *testing.T) {
testListWithEnvironment(t, configImpl{skipCharts: false}) testListWithEnvironment(t, *NewConfigImplWithDefaults(&configImpl{skipCharts: false}))
}) })
t.Run("with skipCharts=true", func(t *testing.T) { t.Run("with skipCharts=true", func(t *testing.T) {
testListWithEnvironment(t, configImpl{skipCharts: true}) testListWithEnvironment(t, *NewConfigImplWithDefaults(&configImpl{skipCharts: true}))
}) })
} }
@ -292,9 +292,9 @@ releases:
func TestListWithJSONOutput(t *testing.T) { func TestListWithJSONOutput(t *testing.T) {
t.Run("with skipCharts=false", func(t *testing.T) { t.Run("with skipCharts=false", func(t *testing.T) {
testListWithJSONOutput(t, configImpl{skipCharts: false}) testListWithJSONOutput(t, *NewConfigImplWithDefaults(&configImpl{skipCharts: false}))
}) })
t.Run("with skipCharts=true", func(t *testing.T) { t.Run("with skipCharts=true", func(t *testing.T) {
testListWithJSONOutput(t, configImpl{skipCharts: true}) testListWithJSONOutput(t, *NewConfigImplWithDefaults(&configImpl{skipCharts: true}))
}) })
} }

View File

@ -79,7 +79,7 @@ func TestSync(t *testing.T) {
app.Selectors = tc.selectors app.Selectors = tc.selectors
} }
syncErr := app.Sync(applyConfig{ syncErr := app.Sync(NewApplyConfigWithDefaults(&applyConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: tc.concurrency, concurrency: tc.concurrency,
logger: logger, logger: logger,
@ -87,7 +87,7 @@ func TestSync(t *testing.T) {
skipNeeds: tc.fields.skipNeeds, skipNeeds: tc.fields.skipNeeds,
includeNeeds: tc.fields.includeNeeds, includeNeeds: tc.fields.includeNeeds,
includeTransitiveNeeds: tc.fields.includeTransitiveNeeds, includeTransitiveNeeds: tc.fields.includeTransitiveNeeds,
}) }))
var gotErr string var gotErr string
if syncErr != nil { if syncErr != nil {

View File

@ -126,7 +126,7 @@ releases:
app.Selectors = tc.selectors app.Selectors = tc.selectors
} }
tmplErr := app.Template(applyConfig{ tmplErr := app.Template(NewApplyConfigWithDefaults(&applyConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: 1, concurrency: 1,
logger: logger, logger: logger,
@ -135,7 +135,7 @@ releases:
includeTransitiveNeeds: tc.fields.includeTransitiveNeeds, includeTransitiveNeeds: tc.fields.includeTransitiveNeeds,
showOnly: tc.fields.showOnly, showOnly: tc.fields.showOnly,
noHooks: tc.fields.noHooks, noHooks: tc.fields.noHooks,
}) }))
var gotErr string var gotErr string
if tmplErr != nil { if tmplErr != nil {
@ -394,11 +394,11 @@ releases:
app.Namespace = tc.ns app.Namespace = tc.ns
} }
tmplErr := app.Template(applyConfig{ tmplErr := app.Template(NewApplyConfigWithDefaults(&applyConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: 1, concurrency: 1,
logger: logger, logger: logger,
}) }))
var gotErr string var gotErr string
if tmplErr != nil { if tmplErr != nil {
@ -499,11 +499,11 @@ releases:
app.Namespace = tc.ns app.Namespace = tc.ns
} }
tmplErr := app.Template(applyConfig{ tmplErr := app.Template(NewApplyConfigWithDefaults(&applyConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: 1, concurrency: 1,
logger: logger, logger: logger,
}) }))
var gotErr string var gotErr string
if tmplErr != nil { if tmplErr != nil {

View File

@ -21,9 +21,11 @@ import (
"go.uber.org/zap" "go.uber.org/zap"
"helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart"
"github.com/helmfile/helmfile/pkg/common"
"github.com/helmfile/helmfile/pkg/envvar" "github.com/helmfile/helmfile/pkg/envvar"
"github.com/helmfile/helmfile/pkg/exectest" "github.com/helmfile/helmfile/pkg/exectest"
ffs "github.com/helmfile/helmfile/pkg/filesystem" ffs "github.com/helmfile/helmfile/pkg/filesystem"
"github.com/helmfile/helmfile/pkg/flags"
"github.com/helmfile/helmfile/pkg/helmexec" "github.com/helmfile/helmfile/pkg/helmexec"
"github.com/helmfile/helmfile/pkg/remote" "github.com/helmfile/helmfile/pkg/remote"
"github.com/helmfile/helmfile/pkg/runtime" "github.com/helmfile/helmfile/pkg/runtime"
@ -2096,9 +2098,9 @@ type configImpl struct {
set []string set []string
output string output string
noHooks bool noHooks bool
includeCRDs bool skipCRDs common.BoolFlag
includeCRDs common.BoolFlag
skipCleanup bool skipCleanup bool
skipCRDs bool
skipDeps bool skipDeps bool
skipTests bool skipTests bool
skipSchemaValidation bool skipSchemaValidation bool
@ -2111,6 +2113,29 @@ type configImpl struct {
kubeVersion string kubeVersion string
} }
func NewConfigImpl() *configImpl {
return &configImpl{
skipCRDs: common.NewBoolFlag(false),
includeCRDs: common.NewBoolFlag(false),
}
}
// NewConfigImplWithDefaults creates a new configImpl with default values.
// If an existing config is provided, it ensures all fields are initialized.
// If the existing config is nil, a new config is created.
// The existing config is modified in place, so it should not be nil if you want to reuse it.
func NewConfigImplWithDefaults(existing *configImpl) *configImpl {
if existing == nil {
return NewConfigImpl()
}
// Initialize any nil fields in the existing config
flags.EnsureBoolFlag(&existing.skipCRDs, false)
flags.EnsureBoolFlag(&existing.includeCRDs, false)
return existing
}
func (c configImpl) Selectors() []string { func (c configImpl) Selectors() []string {
return c.selectors return c.selectors
} }
@ -2135,14 +2160,25 @@ func (c configImpl) SkipCleanup() bool {
return c.skipCleanup return c.skipCleanup
} }
func (c configImpl) SkipCRDs() bool {
return c.skipCRDs
}
func (c configImpl) SkipDeps() bool { func (c configImpl) SkipDeps() bool {
return c.skipDeps return c.skipDeps
} }
func (c configImpl) SkipCRDs() bool {
return c.skipCRDs.Value()
}
func (c configImpl) IncludeCRDs() bool {
return c.includeCRDs.Value()
}
func (c configImpl) ShouldIncludeCRDs() bool {
includeCRDsExplicit := c.includeCRDs.WasExplicitlySet() && c.includeCRDs.Value()
skipCRDsExplicit := c.skipCRDs.WasExplicitlySet() && !c.skipCRDs.Value()
return includeCRDsExplicit || skipCRDsExplicit
}
func (c configImpl) SkipRefresh() bool { func (c configImpl) SkipRefresh() bool {
return c.skipRefresh return c.skipRefresh
} }
@ -2171,10 +2207,6 @@ func (c configImpl) OutputDirTemplate() string {
return "" return ""
} }
func (c configImpl) IncludeCRDs() bool {
return c.includeCRDs
}
func (c configImpl) NoHooks() bool { func (c configImpl) NoHooks() bool {
return c.noHooks return c.noHooks
} }
@ -2223,7 +2255,8 @@ type applyConfig struct {
set []string set []string
validate bool validate bool
skipCleanup bool skipCleanup bool
skipCRDs bool skipCRDs common.BoolFlag
includeCRDs common.BoolFlag
skipDeps bool skipDeps bool
skipRefresh bool skipRefresh bool
skipNeeds bool skipNeeds bool
@ -2262,8 +2295,33 @@ type applyConfig struct {
syncReleaseLabels bool syncReleaseLabels bool
// template-only options // template-only options
includeCRDs, skipTests bool skipTests bool
outputDir, outputDirTemplate string outputDir string
outputDirTemplate string
}
// New creates a new applyConfig with default values
func NewApplyConfig() *applyConfig {
return &applyConfig{
skipCRDs: common.NewBoolFlag(false),
includeCRDs: common.NewBoolFlag(false),
}
}
// NewApplyConfigWithDefaults creates a new applyConfig with default values.
// If an existing config is provided, it ensures all fields are initialized.
// If the existing config is nil, a new config is created.
// The existing config is modified in place, so it should not be nil if you want to reuse it.
func NewApplyConfigWithDefaults(existing *applyConfig) *applyConfig {
if existing == nil {
return NewApplyConfig()
}
// Initialize any nil fields in the existing config
flags.EnsureBoolFlag(&existing.skipCRDs, false)
flags.EnsureBoolFlag(&existing.includeCRDs, false)
return existing
} }
func (a applyConfig) Args() string { func (a applyConfig) Args() string {
@ -2303,7 +2361,18 @@ func (a applyConfig) SkipCleanup() bool {
} }
func (a applyConfig) SkipCRDs() bool { func (a applyConfig) SkipCRDs() bool {
return a.skipCRDs return a.skipCRDs.Value()
}
func (a applyConfig) IncludeCRDs() bool {
return a.includeCRDs.Value()
}
func (c applyConfig) ShouldIncludeCRDs() bool {
includeCRDsExplicit := c.includeCRDs.WasExplicitlySet() && c.includeCRDs.Value()
skipCRDsExplicit := c.skipCRDs.WasExplicitlySet() && !c.skipCRDs.Value()
return includeCRDsExplicit || skipCRDsExplicit
} }
func (a applyConfig) SkipDeps() bool { func (a applyConfig) SkipDeps() bool {
@ -2399,10 +2468,6 @@ func (a applyConfig) DiffArgs() string {
} }
// helmfile-template-only flags // helmfile-template-only flags
func (a applyConfig) IncludeCRDs() bool {
return a.includeCRDs
}
func (a applyConfig) SkipTests() bool { func (a applyConfig) SkipTests() bool {
return a.skipTests return a.skipTests
} }
@ -2659,7 +2724,7 @@ releases:
}, },
}, files) }, files)
if err := app.Template(configImpl{set: []string{"foo=a", "bar=b"}, skipDeps: false}); err != nil { if err := app.Template(NewConfigImplWithDefaults(&configImpl{set: []string{"foo=a", "bar=b"}, skipDeps: false})); err != nil {
t.Fatalf("%v", err) t.Fatalf("%v", err)
} }
@ -2729,7 +2794,7 @@ releases:
fmt.Printf("CRAFTED APP WITH %p\n", app.fs.DirectoryExistsAt) fmt.Printf("CRAFTED APP WITH %p\n", app.fs.DirectoryExistsAt)
if err := app.Template(configImpl{}); err != nil { if err := app.Template(NewConfigImpl()); err != nil {
t.Fatalf("%v", err) t.Fatalf("%v", err)
} }
@ -3728,13 +3793,13 @@ releases:
app.Selectors = tc.selectors app.Selectors = tc.selectors
} }
applyErr := app.Apply(applyConfig{ applyErr := app.Apply(NewApplyConfigWithDefaults(&applyConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: tc.concurrency, concurrency: tc.concurrency,
logger: logger, logger: logger,
skipDiffOnInstall: tc.skipDiffOnInstall, skipDiffOnInstall: tc.skipDiffOnInstall,
skipNeeds: tc.fields.skipNeeds, skipNeeds: tc.fields.skipNeeds,
}) }))
switch { switch {
case tc.error == "" && applyErr != nil: case tc.error == "" && applyErr != nil:
t.Fatalf("unexpected error for data defined at %s: %v", tc.loc, applyErr) t.Fatalf("unexpected error for data defined at %s: %v", tc.loc, applyErr)
@ -4006,7 +4071,7 @@ releases:
expectNoCallsToHelm(app) expectNoCallsToHelm(app)
out, err := testutil.CaptureStdout(func() { out, err := testutil.CaptureStdout(func() {
err := app.ListReleases(configImpl{}) err := app.ListReleases(NewConfigImpl())
assert.Nil(t, err) assert.Nil(t, err)
}) })
assert.NoError(t, err) assert.NoError(t, err)

View File

@ -826,14 +826,14 @@ releases:
app.Selectors = tc.selectors app.Selectors = tc.selectors
} }
diffErr := app.Diff(diffConfig{ diffErr := app.Diff(NewDiffConfigWithDefaults(&diffConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: tc.concurrency, concurrency: tc.concurrency,
logger: logger, logger: logger,
detailedExitcode: tc.detailedExitcode, detailedExitcode: tc.detailedExitcode,
skipNeeds: tc.flags.skipNeeds, skipNeeds: tc.flags.skipNeeds,
includeNeeds: tc.flags.includeNeeds, includeNeeds: tc.flags.includeNeeds,
}) }))
var diffErrStr string var diffErrStr string
if diffErr != nil { if diffErr != nil {

View File

@ -9,8 +9,10 @@ import (
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"go.uber.org/zap" "go.uber.org/zap"
"github.com/helmfile/helmfile/pkg/common"
"github.com/helmfile/helmfile/pkg/exectest" "github.com/helmfile/helmfile/pkg/exectest"
ffs "github.com/helmfile/helmfile/pkg/filesystem" ffs "github.com/helmfile/helmfile/pkg/filesystem"
"github.com/helmfile/helmfile/pkg/flags"
"github.com/helmfile/helmfile/pkg/helmexec" "github.com/helmfile/helmfile/pkg/helmexec"
"github.com/helmfile/helmfile/pkg/testhelper" "github.com/helmfile/helmfile/pkg/testhelper"
) )
@ -22,7 +24,8 @@ type diffConfig struct {
retainValuesFiles bool retainValuesFiles bool
set []string set []string
validate bool validate bool
skipCRDs bool skipCRDs common.BoolFlag
includeCRDs common.BoolFlag
skipDeps bool skipDeps bool
skipRefresh bool skipRefresh bool
includeTests bool includeTests bool
@ -48,6 +51,30 @@ type diffConfig struct {
logger *zap.SugaredLogger logger *zap.SugaredLogger
} }
// New creates a new applyConfig with default values
func NewDiffConfig() *diffConfig {
return &diffConfig{
skipCRDs: common.NewBoolFlag(false),
includeCRDs: common.NewBoolFlag(false),
}
}
// NewDiffConfigWithDefaults creates a new diffConfig with default values.
// If an existing config is provided, it ensures all fields are initialized.
// If the existing config is nil, a new config is created.
// The existing config is modified in place, so it should not be nil if you want to reuse it.
func NewDiffConfigWithDefaults(existing *diffConfig) *diffConfig {
if existing == nil {
return NewDiffConfig()
}
// Initialize any nil fields in the existing config
flags.EnsureBoolFlag(&existing.skipCRDs, false)
flags.EnsureBoolFlag(&existing.includeCRDs, false)
return existing
}
func (a diffConfig) Args() string { func (a diffConfig) Args() string {
return a.args return a.args
} }
@ -69,7 +96,18 @@ func (a diffConfig) Validate() bool {
} }
func (a diffConfig) SkipCRDs() bool { func (a diffConfig) SkipCRDs() bool {
return a.skipCRDs return a.skipCRDs.Value()
}
func (a diffConfig) IncludeCRDs() bool {
return a.includeCRDs.Value()
}
func (a diffConfig) ShouldIncludeCRDs() bool {
includeCRDsExplicit := a.includeCRDs.WasExplicitlySet() && a.includeCRDs.Value()
skipCRDsExplicit := a.skipCRDs.WasExplicitlySet() && !a.skipCRDs.Value()
return includeCRDsExplicit || skipCRDsExplicit
} }
func (a diffConfig) SkipDeps() bool { func (a diffConfig) SkipDeps() bool {
@ -1181,7 +1219,7 @@ releases:
app.Selectors = tc.selectors app.Selectors = tc.selectors
} }
diffErr := app.Diff(diffConfig{ diffErr := app.Diff(NewDiffConfigWithDefaults(&diffConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: tc.concurrency, concurrency: tc.concurrency,
logger: logger, logger: logger,
@ -1189,7 +1227,7 @@ releases:
skipNeeds: tc.flags.skipNeeds, skipNeeds: tc.flags.skipNeeds,
includeNeeds: tc.flags.includeNeeds, includeNeeds: tc.flags.includeNeeds,
noHooks: tc.flags.noHooks, noHooks: tc.flags.noHooks,
}) }))
var diffErrStr string var diffErrStr string
if diffErr != nil { if diffErr != nil {

View File

@ -13,7 +13,7 @@ import (
"github.com/Masterminds/semver/v3" "github.com/Masterminds/semver/v3"
"github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp"
"github.com/tj/assert" "github.com/stretchr/testify/assert"
"go.uber.org/zap" "go.uber.org/zap"
) )