Count subject characters, and refuse a range git cannot resolve

cmd/git-validation/contract_decided_test.go

@@ -0,0 +1,100 @@
+package main_test
+
+// Two decisions on the command's contract, each a case that fails on the
+// behaviour before them: the short-subject rule counts characters, not
+// bytes, so a subject of 89 non-ASCII characters passes and one of 90
+// fails; and a --range git cannot resolve is an error naming the range,
+// with nothing validated, where a shallow clone whose history does not
+// reach the range's start still runs over what it has.
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+//nolint:paralleltest // newRepo sets the environment with t.Setenv
+func TestContractShortSubjectCountsRunes(t *testing.T) {
+	r := newRepo(t)
+
+	r.commit("a", "a\n", "Root", true)
+
+	under := strings.Repeat("é", 89)
+	r.commit("b", "b\n", under, true)
+
+	if code, out := r.run(
+		nil,
+		"--run",
+		"short-subject",
+	); code != 0 || !strings.Contains(out, "PASS") ||
+		strings.Contains(out, "FAIL") {
+		t.Errorf("89 characters of two bytes each: exit %d, want the rule to pass\n%s", code, out)
+	}
+
+	over := strings.Repeat("é", 90)
+	r.commit("c", "c\n", over, true)
+
+	if code, out := r.run(nil, "--run", "short-subject"); code == 0 || !strings.Contains(out, "FAIL") {
+		t.Errorf("90 characters: exit %d, want the rule to fail\n%s", code, out)
+	}
+}
+
+//nolint:paralleltest // newRepo sets the environment with t.Setenv
+func TestContractUnresolvableRangeIsAnError(t *testing.T) {
+	r := newRepo(t)
+
+	r.commit("a", "a\n", "Root", true)
+	r.commit("b", "b\n", "Change", true)
+
+	cmd := exec.CommandContext(t.Context(), binary, "--range", "nosuchref..HEAD")
+	cmd.Dir = r.dir
+	cmd.Env = os.Environ()
+
+	var stderr strings.Builder
+
+	cmd.Stderr = &stderr
+
+	out, err := cmd.Output()
+	if err == nil {
+		t.Fatalf("an unresolvable range validated the history:\n%s", out)
+	}
+
+	if strings.Contains(string(out), " * ") {
+		t.Errorf("commits were validated under an unresolvable range:\n%s", out)
+	}
+
+	if !strings.Contains(stderr.String(), "nosuchref") {
+		t.Errorf("the error does not name the range:\n%s", stderr.String())
+	}
+}
+
+// A shallow clone's history may not reach the range's start a CI
+// configuration names; the run then covers what the clone has, as before.
+//
+//nolint:paralleltest // newRepo sets the environment with t.Setenv
+func TestContractShallowCloneRunsOverWhatItHas(t *testing.T) {
+	r := newRepo(t)
+
+	r.commit("a", "a\n", "Root", true)
+	base := r.git("rev-parse", "HEAD")
+	r.commit("b", "b\n", "Second", true)
+	r.commit("c", "c\n", "Third", true)
+
+	shallow := filepath.Join(t.TempDir(), "shallow")
+
+	clone := exec.CommandContext(t.Context(), "git", "clone", "-q", "--depth", "1", "file://"+r.dir, shallow)
+	if out, err := clone.CombinedOutput(); err != nil {
+		t.Fatalf("shallow clone: %v\n%s", err, out)
+	}
+
+	cmd := exec.CommandContext(t.Context(), binary, "--dir", shallow, "--range", base+"..HEAD")
+	cmd.Dir = shallow
+	cmd.Env = os.Environ()
+
+	out, err := cmd.Output()
+	if err != nil || !strings.Contains(string(out), "PASS") {
+		t.Errorf("shallow clone: %v\n%s", err, out)
+	}
+}

git/commits.go

@@ -48,8 +48,16 @@ func Commits(commitrange string) ([]CommitEntry, error) {
 	return commits, nil
 }
 
-// Since the commitrange requested may be longer than the depth being cloned in CI,
-// check for an error, if so do a git log to get the oldest available commit for a reduced range.
+// errRange is a --range git cannot resolve in a repository whose history is
+// whole: a typo, or a ref that is not there. It names the range.
+var errRange = errors.New("the commit range does not resolve")
+
+// checkRevList returns the range to validate. A range git resolves is
+// itself. One it does not resolve is an error in a repository with its
+// whole history; in a shallow clone, the CI case, the range's start may lie
+// past the depth fetched, and the run then covers what the clone has, from
+// its oldest commit. Before, every unresolvable range took that fallback,
+// so a typo validated the whole history and passed.
 func checkRevList(commitrange string) (string, error) {
 	cmdArgs := []string{"git", "rev-list", commitrange}
 
@@ -59,6 +67,18 @@ func checkRevList(commitrange string) (string, error) {
 		return commitrange, nil
 	}
 
+	if !shallowRepository() {
+		// The git command that failed is named, and its exit unwraps, as
+		// every git failure here does; git's own words follow.
+		detail := ""
+
+		if exitErr, ok := errors.AsType[*exec.ExitError](err); ok && len(exitErr.Stderr) > 0 {
+			detail = " (" + strings.TrimSpace(string(exitErr.Stderr)) + ")"
+		}
+
+		return "", fmt.Errorf("%w: %s: %w%s", errRange, strings.Join(cmdArgs, " "), err, detail)
+	}
+
 	cmdArgs = []string{"git", "log", "--pretty=oneline", "--no-show-signature"}
 
 	output, err := command(cmdArgs[1:]...).Output()
@@ -78,6 +98,14 @@ func checkRevList(commitrange string) (string, error) {
 	return lastCommit + "..HEAD", nil
 }
 
+// shallowRepository reports whether the repository was cloned with a depth,
+// so that its history may not reach a range's start.
+func shallowRepository() bool {
+	output, err := command("rev-parse", "--is-shallow-repository").Output()
+
+	return err == nil && strings.TrimSpace(string(output)) == "true"
+}
+
 // FieldNames are for the formating and rendering of the CommitEntry structs.
 // Keys here are from git log pretty format "format:...".
 var FieldNames = map[string]string{

git/commits_test.go

@@ -192,7 +192,7 @@ func TestCommitsOutsideARepository(t *testing.T) {
 		t.Fatal("Commits outside a repository succeeded")
 	}
 
-	if !strings.Contains(err.Error(), "git log --pretty=oneline --no-show-signature") {
+	if !strings.Contains(err.Error(), "git rev-list HEAD~1..HEAD") {
 		t.Errorf("Commits error = %q, want it to name the git command", err)
 	}
 

git/contract_test.go

@@ -188,16 +188,28 @@ func TestContractCommitsListsARange(t *testing.T) {
 	}
 
 	for _, c := range []struct {
-		rng  string
-		want []string
+		rng     string
+		want    []string
+		refused bool
 	}{
-		{hashes[1] + ".." + hashes[3], []string{"Subject 3", "Subject 2"}},
-		{hashes[3], []string{"Subject 3", "Subject 2", "Subject 1", "Subject 0"}},
-		{"HEAD", []string{"Subject 3", "Subject 2", "Subject 1", "Subject 0"}},
-		{hashes[3] + ".." + hashes[3], nil},
-		{"nonesuch..HEAD", []string{"Subject 3", "Subject 2", "Subject 1"}},
+		{hashes[1] + ".." + hashes[3], []string{"Subject 3", "Subject 2"}, false},
+		{hashes[3], []string{"Subject 3", "Subject 2", "Subject 1", "Subject 0"}, false},
+		{"HEAD", []string{"Subject 3", "Subject 2", "Subject 1", "Subject 0"}, false},
+		{hashes[3] + ".." + hashes[3], nil, false},
+		// A range git cannot resolve is an error naming it in a repository
+		// with its whole history; it used to validate the whole history.
+		{"nonesuch..HEAD", nil, true},
 	} {
 		got, err := git.Commits(c.rng)
+		if c.refused {
+			// The range, and git's own words on it.
+			if err == nil || !strings.Contains(err.Error(), c.rng) || !strings.Contains(err.Error(), "fatal:") {
+				t.Errorf("%s: error %v, want one naming the range with git's message", c.rng, err)
+			}
+
+			continue
+		}
+
 		if err != nil {
 			t.Errorf("%s: %v", c.rng, err)
 

rules/shortsubject/shortsubject.go

@@ -2,6 +2,7 @@ package shortsubject
 
 import (
 	"strings"
+	"unicode/utf8"
 
 	"github.com/forkcloser/git-validation/git"
 	"github.com/forkcloser/git-validation/validate"
@@ -29,15 +30,19 @@ func ValidateShortSubject(r validate.Rule, c git.CommitEntry) (vr validate.Resul
 		return vr
 	}
 
-	if len(c["subject"]) >= 90 {
+	// The limit is in characters, as GitHub's ellipsis and a reader count
+	// them; in bytes, 85 accented characters failed the rule.
+	length := utf8.RuneCountInString(c["subject"])
+
+	if length >= 90 {
 		vr.Pass = false
 		vr.Msg = "commit subject exceeds 90 characters"
 
 		return vr
 	}
 
 	vr.Pass = true
-	if len(c["subject"]) > 72 {
+	if length > 72 {
 		vr.Msg = "commit subject is under 90 characters, but is still more than 72 chars"
 	} else {
 		vr.Msg = "commit subject is 72 characters or less! *yay*"