TT-1715 Create tests for the new flaky tests detector (#1299)

lukaszcl · web-flow · commit aa22cd4063fa · 2024-11-05T15:05:47.000+01:00
diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
@@ -22,6 +22,8 @@ jobs:
             path: ./tools/ghlatestreleasechecker/
           - name: asciitable
             path: ./tools/asciitable/
+          - name: flakeguard
+            path: ./tools/flakeguard/            
           - name: workflowresultparser
             path: ./tools/workflowresultparser/
     runs-on: ubuntu-latest
diff --git a/tools/flakeguard/Makefile b/tools/flakeguard/Makefile
@@ -0,0 +1,2 @@
+test_unit:
+	go test -timeout 5m -json -cover -covermode=count -coverprofile=unit-test-coverage.out ./... 2>&1 | tee /tmp/gotest.log | gotestloghelper -ci
diff --git a/tools/flakeguard/cmd/find.go b/tools/flakeguard/cmd/find.go
@@ -154,9 +154,6 @@ func findAffectedPackages(baseRef, projectPath string, excludes []string, levels
 
 func outputResults(packages []string, jsonOutput bool) {
 	if jsonOutput {
-		if packages == nil {
-			packages = make([]string, 0) // Ensure the slice is initialized to an empty array
-		}
 		data, err := json.Marshal(packages)
 		if err != nil {
 			log.Fatalf("Error marshaling test files to JSON: %v", err)
diff --git a/tools/flakeguard/cmd/run.go b/tools/flakeguard/cmd/run.go
@@ -57,7 +57,7 @@ var RunTestsCmd = &cobra.Command{
 		if len(failedTests) > 0 {
 			fmt.Printf("PassRatio threshold for flaky tests: %.2f\n", threshold)
 			fmt.Printf("%d failed tests:\n", len(failedTests))
-			reports.PrintTests(failedTests)
+			reports.PrintTests(failedTests, os.Stdout)
 		}
 
 		fmt.Printf("Summary: %d passed, %d skipped, %d failed\n", len(passedTests), len(skippedTests), len(failedTests))
diff --git a/tools/flakeguard/git/git_test.go b/tools/flakeguard/git/git_test.go
@@ -0,0 +1,96 @@
+package git
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetChangedGoPackagesFromDiff(t *testing.T) {
+	tests := []struct {
+		name        string
+		out         string
+		projectPath string
+		excludes    []string
+		fileMap     map[string][]string
+		expected    []string
+		expectError bool
+	}{
+		{
+			name:     "Basic Case",
+			out:      "pkg1/file1.go\npkg2/file2.go\n",
+			excludes: []string{},
+			fileMap: map[string][]string{
+				"pkg1/file1.go": {"pkg1"},
+				"pkg2/file2.go": {"pkg2"},
+			},
+			expected:    []string{"pkg1", "pkg2"},
+			expectError: false,
+		},
+		{
+			name:        "Empty Input",
+			out:         "",
+			excludes:    []string{},
+			fileMap:     map[string][]string{},
+			expected:    []string{},
+			expectError: false,
+		},
+		{
+			name:     "Non-Go Files Ignored",
+			out:      "pkg1/file1.txt\npkg2/file2.go\n",
+			excludes: []string{},
+			fileMap: map[string][]string{
+				"pkg2/file2.go": {"pkg2"},
+			},
+			expected:    []string{"pkg2"},
+			expectError: false,
+		},
+		{
+			name:     "Exclusions Applied",
+			out:      "pkg1/file1.go\npkg2/file2.go\npkg3/file3.go\n",
+			excludes: []string{"pkg2"},
+			fileMap: map[string][]string{
+				"pkg1/file1.go": {"pkg1"},
+				"pkg2/file2.go": {"pkg2"},
+				"pkg3/file3.go": {"pkg3"},
+			},
+			expected:    []string{"pkg1", "pkg3"},
+			expectError: false,
+		},
+		{
+			name:     "Multiple Imports",
+			out:      "pkg1/file1.go\n",
+			excludes: []string{},
+			fileMap: map[string][]string{
+				"pkg1/file1.go": {"pkg1", "pkg1/subpkg"},
+			},
+			expected:    []string{"pkg1", "pkg1/subpkg"},
+			expectError: false,
+		},
+		{
+			name:     "Duplicate Packages",
+			out:      "pkg1/file1.go\npkg1/file1.go\n",
+			excludes: []string{},
+			fileMap: map[string][]string{
+				"pkg1/file1.go": {"pkg1"},
+			},
+			expected:    []string{"pkg1"},
+			expectError: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			outBuffer := bytes.Buffer{}
+			outBuffer.WriteString(tt.out)
+			result, err := GetChangedGoPackagesFromDiff(outBuffer, tt.projectPath, tt.excludes, tt.fileMap)
+			if tt.expectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.ElementsMatch(t, tt.expected, result)
+			}
+		})
+	}
+}
diff --git a/tools/flakeguard/go.mod b/tools/flakeguard/go.mod
@@ -4,7 +4,14 @@ go 1.21.9
 
 require github.com/spf13/cobra v1.8.1
 
+require (
+	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
+)
+
 require (
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
+	github.com/stretchr/testify v1.9.0
 )
diff --git a/tools/flakeguard/go.sum b/tools/flakeguard/go.sum
@@ -1,10 +1,17 @@
 github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
+github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
+github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
 github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
+github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM=
 github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y=
 github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
+github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
+github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
diff --git a/tools/flakeguard/golang/golang.go b/tools/flakeguard/golang/golang.go
@@ -168,7 +168,7 @@ func GetFilePackages(files []string) ([]string, error) {
 }
 
 // Function to check if a package contains any test functions
-func hasTests(pkgName string) (bool, error) {
+var hasTests = func(pkgName string) (bool, error) {
 	cmd := exec.Command("go", "test", pkgName, "-run=^$", "-list", ".")
 
 	var out bytes.Buffer
@@ -184,7 +184,7 @@ func hasTests(pkgName string) (bool, error) {
 
 // Filter out test packages with no actual test functions
 func FilterPackagesWithTests(pkgs []string) []string {
-	var testPkgs []string
+	testPkgs := []string{}
 	for _, pkg := range pkgs {
 		hasT, err := hasTests(pkg)
 		if err != nil {
diff --git a/tools/flakeguard/golang/golang_test.go b/tools/flakeguard/golang/golang_test.go
@@ -0,0 +1,56 @@
+package golang
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// Mock version of hasTests function to simulate various scenarios
+func mockHasTests(pkgName string) (bool, error) {
+	switch pkgName {
+	case "pkgWithTests":
+		return true, nil
+	case "pkgWithoutTests":
+		return false, nil
+	case "pkgWithError":
+		return false, errors.New("test error")
+	default:
+		return false, nil
+	}
+}
+
+func TestFilterPackagesWithTests(t *testing.T) {
+	// Replace hasTests with mock function
+	originalHasTests := hasTests
+	hasTests = mockHasTests
+	defer func() { hasTests = originalHasTests }() // Restore original function after test
+
+	t.Run("should return packages that contain tests", func(t *testing.T) {
+		pkgs := []string{"pkgWithTests", "pkgWithoutTests", "pkgWithError"}
+		expected := []string{"pkgWithTests"}
+
+		result := FilterPackagesWithTests(pkgs)
+
+		assert.Equal(t, expected, result, "Expected packages with tests only")
+	})
+
+	t.Run("should return an empty slice when all packages have no tests", func(t *testing.T) {
+		pkgs := []string{"pkgWithoutTests"}
+		expected := []string{}
+
+		result := FilterPackagesWithTests(pkgs)
+
+		assert.Equal(t, expected, result, "Expected empty slice for packages without tests")
+	})
+
+	t.Run("should handle error scenarios gracefully", func(t *testing.T) {
+		pkgs := []string{"pkgWithError"}
+		expected := []string{}
+
+		result := FilterPackagesWithTests(pkgs)
+
+		assert.Equal(t, expected, result, "Expected empty slice for packages with errors")
+	})
+}
diff --git a/tools/flakeguard/reports/reports.go b/tools/flakeguard/reports/reports.go
@@ -2,6 +2,7 @@ package reports
 
 import (
 	"fmt"
+	"io"
 	"strings"
 )
 
@@ -49,19 +50,19 @@ func FilterSkippedTests(results []TestResult) []TestResult {
 }
 
 // PrintTests prints tests in a pretty format
-func PrintTests(tests []TestResult) {
+func PrintTests(tests []TestResult, w io.Writer) {
 	for i, test := range tests {
-		fmt.Printf("\n--- Test %d ---\n", i+1)
-		fmt.Printf("TestName: %s\n", test.TestName)
-		fmt.Printf("TestPackage: %s\n", test.TestPackage)
-		fmt.Printf("PassRatio: %.2f\n", test.PassRatio)
-		fmt.Printf("Skipped: %v\n", test.Skipped)
-		fmt.Printf("Runs: %d\n", test.Runs)
+		fmt.Fprintf(w, "\n--- Test %d ---\n", i+1)
+		fmt.Fprintf(w, "TestName: %s\n", test.TestName)
+		fmt.Fprintf(w, "TestPackage: %s\n", test.TestPackage)
+		fmt.Fprintf(w, "PassRatio: %.2f\n", test.PassRatio)
+		fmt.Fprintf(w, "Skipped: %v\n", test.Skipped)
+		fmt.Fprintf(w, "Runs: %d\n", test.Runs)
 		durationsStr := make([]string, len(test.Durations))
 		for i, duration := range test.Durations {
 			durationsStr[i] = fmt.Sprintf("%.2fs", duration)
 		}
-		fmt.Printf("Durations: %s\n", strings.Join(durationsStr, ", "))
-		fmt.Printf("Outputs:\n%s\n", strings.Join(test.Outputs, ""))
+		fmt.Fprintf(w, "Durations: %s\n", strings.Join(durationsStr, ", "))
+		fmt.Fprintf(w, "Outputs:\n%s\n", strings.Join(test.Outputs, ""))
 	}
 }
diff --git a/tools/flakeguard/reports/reports_test.go b/tools/flakeguard/reports/reports_test.go
@@ -0,0 +1,111 @@
+package reports
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+)
+
+func TestFilterFailedTests(t *testing.T) {
+	results := []TestResult{
+		{TestName: "Test1", PassRatio: 0.5, Skipped: false},
+		{TestName: "Test2", PassRatio: 0.9, Skipped: false},
+		{TestName: "Test3", PassRatio: 0.3, Skipped: false},
+		{TestName: "Test4", PassRatio: 0.8, Skipped: true}, // Skipped test
+	}
+
+	failedTests := FilterFailedTests(results, 0.6)
+	expected := []string{"Test1", "Test3"}
+
+	if len(failedTests) != len(expected) {
+		t.Fatalf("expected %d failed tests, got %d", len(expected), len(failedTests))
+	}
+
+	for i, test := range failedTests {
+		if test.TestName != expected[i] {
+			t.Errorf("expected test %s, got %s", expected[i], test.TestName)
+		}
+	}
+}
+
+func TestFilterPassedTests(t *testing.T) {
+	results := []TestResult{
+		{TestName: "Test1", PassRatio: 0.7, Skipped: false},
+		{TestName: "Test2", PassRatio: 1.0, Skipped: false},
+		{TestName: "Test3", PassRatio: 0.3, Skipped: false},
+		{TestName: "Test4", PassRatio: 0.8, Skipped: true}, // Skipped test
+	}
+
+	passedTests := FilterPassedTests(results, 0.6)
+	expected := []string{"Test1", "Test2"}
+
+	if len(passedTests) != len(expected) {
+		t.Fatalf("expected %d passed tests, got %d", len(expected), len(passedTests))
+	}
+
+	for i, test := range passedTests {
+		if test.TestName != expected[i] {
+			t.Errorf("expected test %s, got %s", expected[i], test.TestName)
+		}
+	}
+}
+
+func TestFilterSkippedTests(t *testing.T) {
+	results := []TestResult{
+		{TestName: "Test1", PassRatio: 0.7, Skipped: false},
+		{TestName: "Test2", PassRatio: 1.0, Skipped: true},
+		{TestName: "Test3", PassRatio: 0.3, Skipped: false},
+		{TestName: "Test4", PassRatio: 0.8, Skipped: true},
+	}
+
+	skippedTests := FilterSkippedTests(results)
+	expected := []string{"Test2", "Test4"}
+
+	if len(skippedTests) != len(expected) {
+		t.Fatalf("expected %d skipped tests, got %d", len(expected), len(skippedTests))
+	}
+
+	for i, test := range skippedTests {
+		if test.TestName != expected[i] {
+			t.Errorf("expected test %s, got %s", expected[i], test.TestName)
+		}
+	}
+}
+
+func TestPrintTests(t *testing.T) {
+	tests := []TestResult{
+		{
+			TestName:    "Test1",
+			TestPackage: "package1",
+			PassRatio:   0.75,
+			Skipped:     false,
+			Runs:        4,
+			Outputs:     []string{"Output1", "Output2"},
+			Durations:   []float64{1.2, 0.9, 1.1, 1.0},
+		},
+	}
+
+	// Use a buffer to capture the output
+	var buf bytes.Buffer
+
+	// Call PrintTests with the buffer
+	PrintTests(tests, &buf)
+
+	// Get the output as a string
+	output := buf.String()
+	expectedContains := []string{
+		"TestName: Test1",
+		"TestPackage: package1",
+		"PassRatio: 0.75",
+		"Skipped: false",
+		"Runs: 4",
+		"Durations: 1.20s, 0.90s, 1.10s, 1.00s",
+		"Outputs:\nOutput1Output2",
+	}
+
+	for _, expected := range expectedContains {
+		if !strings.Contains(output, expected) {
+			t.Errorf("expected output to contain %q, but it did not", expected)
+		}
+	}
+}
diff --git a/tools/flakeguard/utils/cmd_test.go b/tools/flakeguard/utils/cmd_test.go
diff --git a/tools/flakeguard/utils/utils_test.go b/tools/flakeguard/utils/utils_test.go

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+test_unit:`
	`2`	`+ go test -timeout 5m -json -cover -covermode=count -coverprofile=unit-test-coverage.out ./... 2>&1 \| tee /tmp/gotest.log \| gotestloghelper -ci`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ var RunTestsCmd = &cobra.Command{`
`57`	`57`	`if len(failedTests) > 0 {`
`58`	`58`	`fmt.Printf("PassRatio threshold for flaky tests: %.2f\n", threshold)`
`59`	`59`	`fmt.Printf("%d failed tests:\n", len(failedTests))`
`60`		`- reports.PrintTests(failedTests)`
	`60`	`+ reports.PrintTests(failedTests, os.Stdout)`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`fmt.Printf("Summary: %d passed, %d skipped, %d failed\n", len(passedTests), len(skippedTests), len(failedTests))`