github star/size exclusion: incorporate feedback from code review (#58398)

internal/bytesize/BUILD.bazel

@@ -13,4 +13,5 @@ go_test(
     name = "bytesize_test",
     srcs = ["bytesize_test.go"],
     embed = [":bytesize"],
+    deps = ["@com_github_stretchr_testify//require"],
 )

internal/bytesize/bytesize.go

@@ -5,7 +5,6 @@ package bytesize
 import (
 	"strconv"
 	"strings"
-	"unicode"
 
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 )
@@ -39,7 +38,11 @@ const (
 func Parse(str string) (Bytes, error) {
 	str = strings.TrimSpace(str)
 
-	num, unitIndex := readNumber(str)
+	num, unitIndex, err := readNumber(str)
+	if err != nil {
+		return 0, errors.Newf("failed to parse %q into number: %s", str[:unitIndex], err)
+	}
+
 	if unitIndex == 0 {
 		return 0, errors.Newf("missing number at start of string: %s", str)
 	}
@@ -57,17 +60,22 @@ func Parse(str string) (Bytes, error) {
 	return result, nil
 }
 
-func readNumber(str string) (int, int) {
+func readNumber(str string) (int, int, error) {
 	for i, c := range str {
-		if unicode.IsDigit(c) {
+		if isDigit(c) {
 			continue
 		}
-		number, _ := strconv.Atoi(str[0:i])
-		return number, i
+		if i == 0 {
+			return 0, 0, nil
+		}
+		number, err := strconv.Atoi(str[0:i])
+		return number, i, err
 	}
-	return 0, 0
+	return 0, 0, nil
 }
 
+func isDigit(ch rune) bool { return '0' <= ch && ch <= '9' }
+
 func parseUnit(unit string) (Bytes, error) {
 	switch strings.TrimSpace(unit) {
 	case "B", "b":

internal/bytesize/bytesize_test.go

@@ -3,6 +3,8 @@ package bytesize
 import (
 	"fmt"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestParse(t *testing.T) {
@@ -90,7 +92,8 @@ func TestReadNumber(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		num, numLen := readNumber(tt.input)
+		num, numLen, err := readNumber(tt.input)
+		require.NoError(t, err)
 
 		if num != tt.want {
 			t.Errorf("readNumber(%q) = %d, want %d", tt.input, num, tt.want)

internal/repos/exclude.go

@@ -164,7 +164,14 @@ func buildSizeConstraintsExcludeFn(constraint string) (gitHubExcludeFunc, error)
 	}
 
 	return func(r github.Repository) bool {
-		return operator.Eval(int(r.SizeBytes()), int(size))
+		repoSize := int(r.SizeBytes())
+
+		// If we don't have a repository size, we don't exclude
+		if repoSize == 0 {
+			return false
+		}
+
+		return operator.Eval(repoSize, int(size))
 	}, nil
 }
 

internal/repos/exclude_test.go

@@ -93,6 +93,9 @@ func TestBuildGitHubExcludeRule(t *testing.T) {
 
 			{"< 1 GB", 1024*1024 - 1, false},
 			{"< 2 GB", 1024 * 1024, true},
+
+			// Ignore repositories with 0 size
+			{"< 1 MB", 0, false},
 		}
 
 		for _, tt := range tests {

schema/github.schema.json

@@ -136,7 +136,7 @@
             "format": "regex"
           },
           "size": {
-            "description": "If set, repositories with a size above the specified one will be excluded. Specify in kb.",
+            "description": "If set, repositories with a size above the specified one will be excluded.",
             "type": "string",
             "minLength": 2,
             "pattern": "^[<>]{1}[=]{0,1}\\s*\\d+\\s*\\w+$"

schema/schema.go

@@ -787,7 +787,7 @@ type ExcludedGitHubRepo struct {
 	Name string `json:"name,omitempty"`
 	// Pattern description: Regular expression which matches against the name of a GitHub repository ("owner/name").
 	Pattern string `json:"pattern,omitempty"`
-	// Size description: If set, repositories with a size above the specified one will be excluded. Specify in kb.
+	// Size description: If set, repositories with a size above the specified one will be excluded.
 	Size string `json:"size,omitempty"`
 	// Stars description: If set, repositories stars less than the specified number will be.
 	Stars string `json:"stars,omitempty"`