diff --git a/cmd/git-validation/contract_decided_test.go b/cmd/git-validation/contract_decided_test.go new file mode 100644 index 0000000..e9f0f8b --- /dev/null +++ b/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) + } +} diff --git a/git/commits.go b/git/commits.go index 8c9eaec..b40ad8f 100644 --- a/git/commits.go +++ b/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{ diff --git a/git/commits_test.go b/git/commits_test.go index 6576ed0..087b8e7 100644 --- a/git/commits_test.go +++ b/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) } diff --git a/git/contract_test.go b/git/contract_test.go index c021d3f..a84adb9 100644 --- a/git/contract_test.go +++ b/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) diff --git a/rules/shortsubject/shortsubject.go b/rules/shortsubject/shortsubject.go index 4cb22b1..726b341 100644 --- a/rules/shortsubject/shortsubject.go +++ b/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,7 +30,11 @@ 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" @@ -37,7 +42,7 @@ func ValidateShortSubject(r validate.Rule, c git.CommitEntry) (vr validate.Resul } 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*"