Update codecov to use skip file as threshold as well (istio#11294)

bin/codecov_diff.sh

@@ -65,7 +65,7 @@ if [[ -n "${CIRCLE_PR_NUMBER:-}" ]]; then
   go test -v istio.io/istio/tests/codecov/... \
     --report_file="${REPORT_PATH}/coverage.html" \
     --baseline_file="${BASELINE_PATH}/coverage.html" \
-    --threshold_file="${THRESHOLD_FILE}" \
+    --threshold_files="${THRESHOLD_FILE},${CODECOV_SKIP}" \
     | tee "${GOPATH}"/out/codecov/out.log \
     | tee >(go-junit-report > "${GOPATH}"/out/tests/junit.xml)
 else

codecov.skip

@@ -1,3 +1,4 @@
+istio.io/istio/galley/pkg/testing
 istio.io/istio/mixer/pkg/adapter/test
 istio.io/istio/mixer/pkg/config/storetest
 istio.io/istio/mixer/pkg/mockapi

codecov.threshold

@@ -1,5 +1,6 @@
 # This overrides the default threshold used by the circle codecov job to prevent
-# unexpected drop of code coverage.
+# unexpected drop of code coverage. And this supplements the packages specified
+# in codecov.skip.
 #
 # This is useful to temporarily whitelist the packages that have either non
 # deterministic code path. (E.g. test may retry on failure. Some paths may not
@@ -11,10 +12,14 @@
 # are made deteministic.
 #
 # Format:
-# <package or filename>=<threshold in float64>
+# <package or filename>{=<threshold in float64>}
 #
+# If threshold is not specified, codecov check will be skipped for the package
+# or go file.
+
 # Istio wide default
 istio.io=5
+
 # Overrides
 istio.io/istio/mixer/adapter/solarwinds/internal/papertrail/papertrail_logger.go=50
 istio.io/istio/pilot/pkg/config/memory/monitor.go=30
@@ -24,11 +29,10 @@ istio.io/istio/pilot/pkg/serviceregistry/consul/monitor.go=20
 istio.io/istio/pkg/mcp/creds/watcher.go=100
 istio.io/istio/pkg/mcp/source/source.go=90
 istio.io/istio/security/pkg/nodeagent=15
+
 # Disable codecov check for testing and proto packages
-istio.io/istio/galley/pkg/testing=100	istio.io/istio/galley/pkg/testing=100
-istio.io/istio/pkg/test=100
 istio.io/istio/security/proto=100
-istio.io/istio/tests=100
+istio.io/istio/tests
 
 # Temporary until integ tests are restored
 istio.io/istio/pilot/pkg/networking/plugin=50

tests/codecov/main.go

@@ -27,10 +27,10 @@ import (
 )
 
 var (
-	reportFile    = flag.String("report_file", "", "Code coverage report file")
-	baselineFile  = flag.String("baseline_file", "", "Code coverage baseline file")
-	thresholdFile = flag.String("threshold_file", "", "File containing package to threshold mappings, as overrides")
-	skipDeleted   = flag.Bool("skip_deleted", true, "Whehter deleted files should be skipped")
+	reportFile     = flag.String("report_file", "", "Code coverage report file")
+	baselineFile   = flag.String("baseline_file", "", "Code coverage baseline file")
+	thresholdFiles = flag.String("threshold_files", "", "File containing package to threshold mappings, as overrides")
+	skipDeleted    = flag.Bool("skip_deleted", true, "Whehter deleted files should be skipped")
 )
 
 func parseReportLine(line string) (string, float64, error) {
@@ -68,39 +68,48 @@ func parseReport(filename string) (map[string]float64, error) {
 	return coverage, scanner.Err()
 }
 
-func parseThreshold(thresholdFile string) (map[string]float64, error) {
-	f, err := os.Open(thresholdFile)
-	if err != nil {
-		return nil, fmt.Errorf("failed to open threshold file, %s, %v", thresholdFile, err)
-	}
-	defer func() {
-		if err = f.Close(); err != nil {
-			glog.Errorf("failed to close file %s, %v", thresholdFile, err)
-		}
-	}()
-
-	scanner := bufio.NewScanner(f)
-	reg := regexp.MustCompile(`(.*)=(.*)`)
-
+func parseThreshold(thresholdFiles string) (map[string]float64, error) {
+	files := strings.Split(thresholdFiles, ",")
 	thresholds := make(map[string]float64)
 
-	for scanner.Scan() {
-		line := strings.TrimSpace(scanner.Text())
-		if strings.HasPrefix(line, "#") {
-			// Skip comments
-			continue
+	for _, thresholdFile := range files {
+		f, err := os.Open(thresholdFile)
+		if err != nil {
+			return nil, fmt.Errorf("failed to open threshold file, %s, %v", thresholdFile, err)
 		}
-		m := reg.FindStringSubmatch(line)
-		if len(m) == 3 {
-			threshold, err := strconv.ParseFloat(strings.TrimSpace(m[2]), 64)
-			if err != nil {
-				return nil, fmt.Errorf("failed to parse threshold to float64 for package %s: %s, %v",
-					m[1], m[2], err)
+		defer func() {
+			if err = f.Close(); err != nil {
+				glog.Errorf("failed to close file %s, %v", thresholdFile, err)
+			}
+		}()
+
+		scanner := bufio.NewScanner(f)
+		reg := regexp.MustCompile(`(.*)=(.*)`)
+
+		for scanner.Scan() {
+			line := strings.TrimSpace(scanner.Text())
+			if strings.HasPrefix(line, "#") {
+				// Skip comments
+				continue
+			}
+			m := reg.FindStringSubmatch(line)
+			if len(m) == 3 {
+				threshold, err := strconv.ParseFloat(strings.TrimSpace(m[2]), 64)
+				if err != nil {
+					return nil, fmt.Errorf("failed to parse threshold to float64 for package %s: %s, %v",
+						m[1], m[2], err)
+				}
+				thresholds[strings.TrimSpace(m[1])] = threshold
+			} else if len(line) > 0 {
+				// The line is the package being ignored.
+				thresholds[strings.TrimSpace(line)] = 100
+			}
+			if scanner.Err() != nil {
+				return thresholds, scanner.Err()
 			}
-			thresholds[strings.TrimSpace(m[1])] = threshold
 		}
 	}
-	return thresholds, scanner.Err()
+	return thresholds, nil
 }
 
 func findDelta(report, baseline map[string]float64) map[string]float64 {
@@ -177,7 +186,7 @@ func checkCoverage(reportFile, baselineFile, thresholdFile string, skipMissingFi
 // code coverage has dropped above the given threshold.
 func main() {
 	flag.Parse()
-	err := checkCoverage(*reportFile, *baselineFile, *thresholdFile, *skipDeleted)
+	err := checkCoverage(*reportFile, *baselineFile, *thresholdFiles, *skipDeleted)
 	if err != nil {
 		glog.Error(err)
 		os.Exit(1)

tests/codecov/main_test.go

@@ -57,12 +57,22 @@ func TestParseThreshold(t *testing.T) {
 	if err := ioutil.WriteFile(outFile, []byte(example), 0644); err != nil {
 		t.Errorf("Failed to write example file, %v", err)
 	}
+	example2 :=
+		"#Some comments\n" +
+			"  # more comments\n" +
+			"istio.io/istio/mixer/pkg\n" +
+			" istio.io/istio/pilot/test\n" +
+			"\n"
+	outFile2 := filepath.Join(tmpDir, "outFile2")
+	if err := ioutil.WriteFile(outFile2, []byte(example2), 0644); err != nil {
+		t.Errorf("Failed to write example file, %v", err)
+	}
 
-	thresholds, err := parseThreshold(outFile)
+	thresholds, err := parseThreshold(outFile + "," + outFile2)
 	if err != nil {
 		t.Errorf("Failed to parse outFile, %v", err)
 	} else {
-		if len(thresholds) != 2 {
+		if len(thresholds) != 4 {
 			t.Error("Wrong result count from parseThresholds()")
 		}
 		if thresholds["istio.io/istio/galley/pkg/crd"] != 10.5 {
@@ -71,6 +81,12 @@ func TestParseThreshold(t *testing.T) {
 		if thresholds["istio.io/istio/pilot"] != 20.2 {
 			t.Error("Wrong result from parseThreshold()")
 		}
+		if thresholds["istio.io/istio/mixer/pkg"] != 100 {
+			t.Error("Wrong result from parseThreshold()")
+		}
+		if thresholds["istio.io/istio/pilot/test"] != 100 {
+			t.Error("Wrong result from parseThreshold()")
+		}
 	}
 }
 
@@ -191,10 +207,10 @@ func TestCheckDeltaGood(t *testing.T) {
 
 // Actual codecov diff test
 func TestCheckCoverage(t *testing.T) {
-	if len(*reportFile) == 0 || len(*baselineFile) == 0 || len(*thresholdFile) == 0 {
+	if len(*reportFile) == 0 || len(*baselineFile) == 0 || len(*thresholdFiles) == 0 {
 		t.Skip("Test files are not provided.")
 	}
-	err := checkCoverage(*reportFile, *baselineFile, *thresholdFile, *skipDeleted)
+	err := checkCoverage(*reportFile, *baselineFile, *thresholdFiles, *skipDeleted)
 
 	if err != nil {
 		t.Errorf("%v", err)