From a9b64cd7ad92a44a9db00a917e381b762c7d7acb Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 17 Feb 2024 21:50:25 +0100 Subject: [PATCH] features: print warning for stable/depreacted feature flags --- cmd/restic/main.go | 4 +++- internal/feature/features.go | 6 +++--- internal/feature/features_test.go | 34 +++++++++++++++++++++---------- internal/feature/testing.go | 8 ++++++-- 4 files changed, 35 insertions(+), 17 deletions(-) diff --git a/cmd/restic/main.go b/cmd/restic/main.go index 1a11abc40..a4acb1cab 100644 --- a/cmd/restic/main.go +++ b/cmd/restic/main.go @@ -104,7 +104,9 @@ func main() { // we can show the logs log.SetOutput(logBuffer) - err := feature.Flag.Apply(os.Getenv("RESTIC_FEATURES")) + err := feature.Flag.Apply(os.Getenv("RESTIC_FEATURES"), func(s string) { + fmt.Fprintln(os.Stderr, s) + }) if err != nil { fmt.Fprintln(os.Stderr, err) Exit(1) diff --git a/internal/feature/features.go b/internal/feature/features.go index 1e1f3785c..e3b625e92 100644 --- a/internal/feature/features.go +++ b/internal/feature/features.go @@ -57,7 +57,7 @@ func (f *FlagSet) SetFlags(flags map[FlagName]FlagDesc) { } } -func (f *FlagSet) Apply(flags string) error { +func (f *FlagSet) Apply(flags string, logWarning func(string)) error { if flags == "" { return nil } @@ -92,9 +92,9 @@ func (f *FlagSet) Apply(flags string) error { case Alpha, Beta: f.enabled[fname] = value case Stable: - // FIXME print warning + logWarning(fmt.Sprintf("feature flag %q is always enabled and will be removed in a future release", fname)) case Deprecated: - // FIXME print warning + logWarning(fmt.Sprintf("feature flag %q is always disabled and will be removed in a future release", fname)) default: panic("unknown feature phase") } diff --git a/internal/feature/features_test.go b/internal/feature/features_test.go index 3611ac998..f5d405fa7 100644 --- a/internal/feature/features_test.go +++ b/internal/feature/features_test.go @@ -56,9 +56,13 @@ func TestFeatureDefaults(t *testing.T) { } } +func panicIfCalled(msg string) { + panic(msg) +} + func TestEmptyApply(t *testing.T) { flags := buildTestFlagSet() - rtest.OK(t, flags.Apply("")) + rtest.OK(t, flags.Apply("", panicIfCalled)) rtest.Assert(t, !flags.Enabled(alpha), "expected alpha feature to be disabled") rtest.Assert(t, flags.Enabled(beta), "expected beta feature to be enabled") @@ -66,29 +70,37 @@ func TestEmptyApply(t *testing.T) { func TestFeatureApply(t *testing.T) { flags := buildTestFlagSet() - rtest.OK(t, flags.Apply(string(alpha))) + rtest.OK(t, flags.Apply(string(alpha), panicIfCalled)) rtest.Assert(t, flags.Enabled(alpha), "expected alpha feature to be enabled") - rtest.OK(t, flags.Apply(fmt.Sprintf("%s=false", alpha))) + rtest.OK(t, flags.Apply(fmt.Sprintf("%s=false", alpha), panicIfCalled)) rtest.Assert(t, !flags.Enabled(alpha), "expected alpha feature to be disabled") - rtest.OK(t, flags.Apply(fmt.Sprintf("%s=true", alpha))) + rtest.OK(t, flags.Apply(fmt.Sprintf("%s=true", alpha), panicIfCalled)) rtest.Assert(t, flags.Enabled(alpha), "expected alpha feature to be enabled again") - rtest.OK(t, flags.Apply(fmt.Sprintf("%s=false", beta))) + rtest.OK(t, flags.Apply(fmt.Sprintf("%s=false", beta), panicIfCalled)) rtest.Assert(t, !flags.Enabled(beta), "expected beta feature to be disabled") - rtest.OK(t, flags.Apply(fmt.Sprintf("%s=false", stable))) - rtest.Assert(t, flags.Enabled(stable), "expected stable feature to remain enabled") + logMsg := "" + log := func(msg string) { + logMsg = msg + } - rtest.OK(t, flags.Apply(fmt.Sprintf("%s=true", deprecated))) + rtest.OK(t, flags.Apply(fmt.Sprintf("%s=false", stable), log)) + rtest.Assert(t, flags.Enabled(stable), "expected stable feature to remain enabled") + rtest.Assert(t, strings.Contains(logMsg, string(stable)), "unexpected log message for stable flag: %v", logMsg) + + logMsg = "" + rtest.OK(t, flags.Apply(fmt.Sprintf("%s=true", deprecated), log)) rtest.Assert(t, !flags.Enabled(deprecated), "expected deprecated feature to remain disabled") + rtest.Assert(t, strings.Contains(logMsg, string(deprecated)), "unexpected log message for deprecated flag: %v", logMsg) } func TestFeatureMultipleApply(t *testing.T) { flags := buildTestFlagSet() - rtest.OK(t, flags.Apply(fmt.Sprintf("%s=true,%s=false", alpha, beta))) + rtest.OK(t, flags.Apply(fmt.Sprintf("%s=true,%s=false", alpha, beta), panicIfCalled)) rtest.Assert(t, flags.Enabled(alpha), "expected alpha feature to be enabled") rtest.Assert(t, !flags.Enabled(beta), "expected beta feature to be disabled") } @@ -96,10 +108,10 @@ func TestFeatureMultipleApply(t *testing.T) { func TestFeatureApplyInvalid(t *testing.T) { flags := buildTestFlagSet() - err := flags.Apply("invalid-flag") + err := flags.Apply("invalid-flag", panicIfCalled) rtest.Assert(t, err != nil && strings.Contains(err.Error(), "unknown feature flag"), "expected unknown feature flag error, got: %v", err) - err = flags.Apply(fmt.Sprintf("%v=invalid", alpha)) + err = flags.Apply(fmt.Sprintf("%v=invalid", alpha), panicIfCalled) rtest.Assert(t, err != nil && strings.Contains(err.Error(), "failed to parse value"), "expected parsing error, got: %v", err) } diff --git a/internal/feature/testing.go b/internal/feature/testing.go index c13f52509..b796e89b5 100644 --- a/internal/feature/testing.go +++ b/internal/feature/testing.go @@ -15,13 +15,17 @@ import ( func TestSetFlag(t *testing.T, f *FlagSet, flag FlagName, value bool) func() { current := f.Enabled(flag) - if err := f.Apply(fmt.Sprintf("%s=%v", flag, value)); err != nil { + panicIfCalled := func(msg string) { + panic(msg) + } + + if err := f.Apply(fmt.Sprintf("%s=%v", flag, value), panicIfCalled); err != nil { // not reachable panic(err) } return func() { - if err := f.Apply(fmt.Sprintf("%s=%v", flag, current)); err != nil { + if err := f.Apply(fmt.Sprintf("%s=%v", flag, current), panicIfCalled); err != nil { // not reachable panic(err) }