🐛 Bug fixing: recurring results of the scorecard fuzzing check for go…

… built-in fuzzers (#2101) * save * save * save * save * save
ossf · Jul 28, 2022 · 1e0e44a · 1e0e44a
1 parent 8118e5d
commit 1e0e44a
Show file tree

Hide file tree

Showing 3 changed files with 138 additions and 8 deletions.
diff --git a/checks/raw/fuzzing.go b/checks/raw/fuzzing.go
@@ -98,7 +98,6 @@ func Fuzzing(c *checker.CheckRequest) (checker.FuzzingData, error) {
 		return checker.FuzzingData{}, fmt.Errorf("cannot get langs of repo: %w", err)
 	}
 	prominentLangs := getProminentLanguages(langs)
-
 	for _, lang := range prominentLangs {
 		usingFuzzFunc, files, e := checkFuzzFunc(c, lang)
 		if e != nil {
@@ -224,18 +223,20 @@ func getProminentLanguages(langs []clients.Language) []clients.LanguageName {
 		return nil
 	}
 	totalLoC := 0
+	// Use a map to record languages and their lines of code to drop potential duplicates.
+	langMap := map[clients.LanguageName]int{}
 	for _, l := range langs {
 		totalLoC += l.NumLines
+		langMap[l.Name] += l.NumLines
 	}
-	// Var avgLoC calculates the average lines of code in the current repo,
-	// and it can stay as an int, no need for a float value.
+	// Calculate the average lines of code in the current repo.
+	// This var can stay as an int, no need for a precise float value.
 	avgLoC := totalLoC / numLangs
-
 	// Languages that have lines of code above average will be considered prominent.
 	ret := []clients.LanguageName{}
-	for _, l := range langs {
-		if l.NumLines >= avgLoC {
-			lang := clients.LanguageName(strings.ToLower(string(l.Name)))
+	for lName, loC := range langMap {
+		if loC >= avgLoC {
+			lang := clients.LanguageName(strings.ToLower(string(lName)))
 			ret = append(ret, lang)
 		}
 	}

diff --git a/checks/raw/fuzzing_test.go b/checks/raw/fuzzing_test.go
@@ -242,7 +242,6 @@ func Test_checkFuzzFunc(t *testing.T) {
 		fileContent string
 	}{
 		{
-			// TODO: more test cases needed. @aidenwang9867
 			name:    "Test_checkFuzzFunc failure",
 			want:    false,
 			wantErr: false,
@@ -286,3 +285,112 @@ func Test_checkFuzzFunc(t *testing.T) {
 		})
 	}
 }
+
+func Test_getProminentLanguages(t *testing.T) {
+	t.Parallel()
+	//nolint
+	tests := []struct {
+		name      string
+		languages []clients.Language
+		expected  []clients.LanguageName
+	}{
+		{
+			name: "case1",
+			languages: []clients.Language{
+				{
+					Name:     clients.Go,
+					NumLines: 1000,
+				},
+				{
+					Name:     clients.Python,
+					NumLines: 40,
+				}, {
+					Name:     clients.JavaScript,
+					NumLines: 800,
+				},
+			},
+			expected: []clients.LanguageName{
+				clients.Go, clients.JavaScript,
+			},
+		},
+		{
+			// This test case simulates the situation when the GitHub language API returns
+			// duplicated languages, but we can still drop them and get the correct result.
+			name: "case2: drop duplicates",
+			languages: []clients.Language{
+				{
+					Name:     clients.Go,
+					NumLines: 1000,
+				},
+				{
+					Name:     clients.Python,
+					NumLines: 40,
+				}, {
+					Name:     clients.JavaScript,
+					NumLines: 800,
+				},
+				{
+					Name:     clients.Go,
+					NumLines: 1000,
+				},
+				{
+					Name:     clients.Python,
+					NumLines: 40,
+				}, {
+					Name:     clients.JavaScript,
+					NumLines: 800,
+				},
+				{
+					Name:     clients.Go,
+					NumLines: 1000,
+				},
+				{
+					Name:     clients.Python,
+					NumLines: 40,
+				}, {
+					Name:     clients.JavaScript,
+					NumLines: 800,
+				},
+			},
+			expected: []clients.LanguageName{
+				clients.Go, clients.JavaScript,
+			},
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := getProminentLanguages(tt.languages)
+			if !unorderedEqual(got, tt.expected) {
+				t.Errorf(
+					"got (%s) != expected (%s)",
+					got, tt.expected,
+				)
+			}
+
+		})
+	}
+}
+
+func unorderedEqual(l1, l2 []clients.LanguageName) bool {
+	if len(l1) != len(l2) {
+		return false
+	}
+	l1Map, l2Map := map[clients.LanguageName]bool{}, map[clients.LanguageName]bool{}
+	for _, l := range l1 {
+		l1Map[l] = true
+	}
+	for _, l := range l2 {
+		l2Map[l] = true
+		if !l1Map[l] {
+			return false
+		}
+	}
+	for k := range l1Map {
+		if !l2Map[k] {
+			return false
+		}
+	}
+	return true
+}
diff --git a/e2e/fuzzing_test.go b/e2e/fuzzing_test.go
@@ -22,6 +22,7 @@ import (
 
 	"github.com/ossf/scorecard/v4/checker"
 	"github.com/ossf/scorecard/v4/checks"
+	"github.com/ossf/scorecard/v4/checks/raw"
 	"github.com/ossf/scorecard/v4/clients"
 	"github.com/ossf/scorecard/v4/clients/githubrepo"
 	scut "github.com/ossf/scorecard/v4/utests"
@@ -113,6 +114,26 @@ var _ = Describe("E2E TEST:"+checks.CheckFuzzing, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 			Expect(ossFuzzRepoClient.Close()).Should(BeNil())
 		})
+		It("Should return an expected number of GoBuiltInFuzzers", func() {
+			dl := scut.TestDetailLogger{}
+			repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-fuzzing-golang")
+			Expect(err).Should(BeNil())
+			repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger)
+			err = repoClient.InitRepo(repo, clients.HeadSHA)
+			Expect(err).Should(BeNil())
+			ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(context.Background(), logger)
+			Expect(err).Should(BeNil())
+			req := checker.CheckRequest{
+				Ctx:         context.Background(),
+				RepoClient:  repoClient,
+				OssFuzzRepo: ossFuzzRepoClient,
+				Repo:        repo,
+				Dlogger:     &dl,
+			}
+			rawData, err := raw.Fuzzing(&req)
+			Expect(err).Should(BeNil())
+			Expect(len(rawData.Fuzzers) == 1).Should(BeTrue())
+		})
 		It("Should return no fuzzing", func() {
 			dl := scut.TestDetailLogger{}
 			repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-packaging-e2e")