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.
catenacyber · Feb 8, 2025 · 816aa53 · 816aa53
1 parent eff1a84
commit 816aa53
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 33 deletions.
diff --git 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
@@ -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) {