From 816aa53afb4f8ba141a816e3b2c2fad6796af96c Mon Sep 17 00:00:00 2001 From: ccoVeille <3875889+ccoVeille@users.noreply.github.com> Date: Fri, 7 Feb 2025 16:38:01 +0100 Subject: [PATCH] fix: refactor unit tests so they don't depend on previous tests The fact a flag can deactivate some other flags must be done after the flags are overloaded in unit tests. Some unit tests were incorrectly written to fulfill this behavior, they are now failing. --- analyzer/analyzer.go | 35 ++++++++++++++++------------------- analyzer/analyzer_test.go | 35 +++++++++++++++++++++-------------- 2 files changed, 37 insertions(+), 33 deletions(-) diff --git a/analyzer/analyzer.go b/analyzer/analyzer.go index 14bd11f..a551a24 100644 --- a/analyzer/analyzer.go +++ b/analyzer/analyzer.go @@ -74,30 +74,15 @@ func New() *analysis.Analyzer { r.Flags.BoolVar(&n.strFormat.strconcat, "strconcat", true, "optimizes into strings concatenation") r.Flags.BoolVar(&n.fiximports, "fiximports", true, "fix needed imports from other fixes") - if !n.intFormat.enabled { - n.intFormat.intConv = false - } - - if !n.errFormat.enabled { - n.errFormat.errError = false - n.errFormat.errorf = false - } - if !n.strFormat.enabled { - n.strFormat.sprintf1 = false - n.strFormat.strconcat = false - } - return r } // true if verb is a format string that could be replaced with concatenation. func isConcatable(verb string) bool { - hasPrefix := - (strings.HasPrefix(verb, "%s") && !strings.Contains(verb, "%[1]s")) || - (strings.HasPrefix(verb, "%[1]s") && !strings.Contains(verb, "%s")) - hasSuffix := - (strings.HasSuffix(verb, "%s") && !strings.Contains(verb, "%[1]s")) || - (strings.HasSuffix(verb, "%[1]s") && !strings.Contains(verb, "%s")) + hasPrefix := (strings.HasPrefix(verb, "%s") && !strings.Contains(verb, "%[1]s")) || + (strings.HasPrefix(verb, "%[1]s") && !strings.Contains(verb, "%s")) + hasSuffix := (strings.HasSuffix(verb, "%s") && !strings.Contains(verb, "%[1]s")) || + (strings.HasSuffix(verb, "%[1]s") && !strings.Contains(verb, "%s")) if strings.Count(verb, "%[1]s") > 1 { return false @@ -106,6 +91,18 @@ func isConcatable(verb string) bool { } func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) { + if !n.intFormat.enabled { + n.intFormat.intConv = false + } + if !n.errFormat.enabled { + n.errFormat.errError = false + n.errFormat.errorf = false + } + if !n.strFormat.enabled { + n.strFormat.sprintf1 = false + n.strFormat.strconcat = false + } + var fmtSprintObj, fmtSprintfObj, fmtErrorfObj types.Object for _, pkg := range pass.Pkg.Imports() { if pkg.Path() == "fmt" { diff --git a/analyzer/analyzer_test.go b/analyzer/analyzer_test.go index f171dbd..512f224 100644 --- a/analyzer/analyzer_test.go +++ b/analyzer/analyzer_test.go @@ -14,31 +14,38 @@ import ( func TestAnalyzer(t *testing.T) { t.Parallel() - a := analyzer.New() - analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), a, "default") - a.Flags.VisitAll(func(f *flag.Flag) { + + t.Run("default", func(t *testing.T) { + a := analyzer.New() + analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), a, "default") + }) + + defaultAnalyzer := analyzer.New() + defaultAnalyzer.Flags.VisitAll(func(f *flag.Flag) { if f.Name == "fiximports" { + // fiximports is a special case, let's skip it return } + + var changedVal string + switch f.DefValue { + case "false": + changedVal = "true" + case "true": + changedVal = "false" + default: + t.Fatalf("default value neither false or true") + } + t.Run(f.Name, func(t *testing.T) { - changedVal := "false" - if f.DefValue == "false" { - changedVal = "true" - } else if f.DefValue != "true" { - t.Fatalf("default value neither false or true") - } + a := analyzer.New() err := a.Flags.Set(f.Name, changedVal) if err != nil { t.Fatalf("failed to set %q flag", f.Name) } analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), a, f.Name) - err = a.Flags.Set(f.Name, f.DefValue) - if err != nil { - t.Fatalf("failed to set %q flag", f.Name) - } }) }) - } func TestReplacements(t *testing.T) {