From b52f151d2b316cb6900acec422c8d9e15367fa10 Mon Sep 17 00:00:00 2001 From: George Aristy Date: Sun, 24 Feb 2019 10:19:41 -0500 Subject: [PATCH] (#16) Add commit filters (#17) --- check_coverage.sh | 3 + cmd/go-gitlint/main.go | 23 ++++-- internal/commits/commits.go | 48 +++---------- internal/commits/commits_test.go | 40 ++++++----- internal/commits/filter/filter.go | 65 +++++++++++++++++ internal/commits/filter/filter_test.go | 66 +++++++++++++++++ internal/commits/issues/issues.go | 67 ++++++++++++++++++ internal/commits/issues/issues_test.go | 98 ++++++++++++++++++++++++++ 8 files changed, 348 insertions(+), 62 deletions(-) create mode 100644 internal/commits/filter/filter.go create mode 100644 internal/commits/filter/filter_test.go create mode 100644 internal/commits/issues/issues.go create mode 100644 internal/commits/issues/issues_test.go diff --git a/check_coverage.sh b/check_coverage.sh index 1c37295..d84dbb3 100755 --- a/check_coverage.sh +++ b/check_coverage.sh @@ -19,6 +19,9 @@ let THRESHOLD=80 let exit_code=0 +# @todo #16 The coverage script is ignoring packages that are not tested +# at all (0% coverage). It should be fixed so that all packages are +# tested (except for main). while read line; do if [ "$(echo $line | grep coverage)" != "" ]; then pkg=$(echo $line | sed 's/\s\+/ /g' | sed 's/%//' | cut -d ' ' -f 2) diff --git a/cmd/go-gitlint/main.go b/cmd/go-gitlint/main.go index 67f706b..b2050d2 100644 --- a/cmd/go-gitlint/main.go +++ b/cmd/go-gitlint/main.go @@ -18,6 +18,8 @@ import ( "os" "github.com/llorllale/go-gitlint/internal/commits" + "github.com/llorllale/go-gitlint/internal/commits/filter" + "github.com/llorllale/go-gitlint/internal/commits/issues" "github.com/llorllale/go-gitlint/internal/repo" ) @@ -25,11 +27,20 @@ import ( // flag instead of being hard coded. All other configuration // options should be passed in through CLI as well. func main() { - commits.Printed( - commits.In( - repo.Filesystem("."), + os.Exit( + len( + issues.Printed( + os.Stdout, "\n", + issues.Collected( + []func(*commits.Commit) issues.Issue{ + filter.OfSubjectRegex(".{,1}"), + filter.OfBodyRegex(".{,1}"), + }, + commits.In( + repo.Filesystem("."), + ), + ), + )(), ), - os.Stdout, - "\n", - )() + ) } diff --git a/internal/commits/commits.go b/internal/commits/commits.go index 6edcd32..eb48fdb 100644 --- a/internal/commits/commits.go +++ b/internal/commits/commits.go @@ -15,8 +15,6 @@ package commits import ( - "fmt" - "io" "strings" "github.com/llorllale/go-gitlint/internal/repo" @@ -34,24 +32,24 @@ type Commits func() []*Commit // Commit holds data for a single git commit. type Commit struct { - hash string - message string + Hash string + Message string } -// Hash is the commit's ID. -func (c *Commit) Hash() string { - return c.hash +// ID is the commit's hash. +func (c *Commit) ID() string { + return c.Hash } // Subject is the commit message's subject line. func (c *Commit) Subject() string { - return strings.Split(c.message, "\n\n")[0] + return strings.Split(c.Message, "\n\n")[0] } // Body is the commit message's body. func (c *Commit) Body() string { body := "" - parts := strings.Split(c.message, "\n\n") + parts := strings.Split(c.Message, "\n\n") if len(parts) > 1 { body = strings.Join(parts[1:], "") } @@ -78,8 +76,8 @@ func In(repository repo.Repo) Commits { commits = append( commits, &Commit{ - hash: c.Hash.String(), - message: c.Message, + Hash: c.Hash.String(), + Message: c.Message, }, ) return nil @@ -87,31 +85,3 @@ func In(repository repo.Repo) Commits { return commits } } - -// Printed prints the commits to the file. -func Printed(commits Commits, writer io.Writer, sep string) Commits { - return func() []*Commit { - input := commits() - for _, c := range input { - _, err := writer.Write( - []byte(fmt.Sprintf("%s%s", &pretty{c}, sep)), - ) - if err != nil { - panic(err) - } - } - return input - } -} - -// a Stringer for pretty-printing the commit. -type pretty struct { - *Commit -} - -func (p *pretty) String() string { - return fmt.Sprintf( - "hash: %s subject=%s body=%s", - p.Commit.Hash(), p.Commit.Subject(), p.Commit.Body(), - ) -} diff --git a/internal/commits/commits_test.go b/internal/commits/commits_test.go index 6b016c7..065ea93 100644 --- a/internal/commits/commits_test.go +++ b/internal/commits/commits_test.go @@ -15,7 +15,6 @@ package commits import ( - "bytes" "fmt" "io/ioutil" "os" @@ -31,6 +30,29 @@ import ( "gopkg.in/src-d/go-git.v4/plumbing/object" ) +func TestCommitID(t *testing.T) { + const ID = "test ID" + assert.Equal(t, + (&Commit{Hash: ID}).ID(), ID, + "Commit.ID() must return the commit's hash") +} + +func TestCommitSubject(t *testing.T) { + const subject = "test subject" + assert.Equal(t, + (&Commit{Message: subject + "\n\ntest body"}).Subject(), + subject, + `Commit.Subject() must return the substring before the first \n\n`) +} + +func TestCommitBody(t *testing.T) { + const body = "test body" + assert.Equal(t, + (&Commit{Message: "test subject\n\n" + body}).Body(), + body, + `Commit.Body() must return the substring after the first \n\n`) +} + func TestIn(t *testing.T) { msgs := []string{"subject1\n\nbody1", "subject2\n\nbody2", "subject3\n\nbody3"} r, cleanup := tmpRepo(msgs...) @@ -45,22 +67,6 @@ func TestIn(t *testing.T) { } } -func TestPrinted(t *testing.T) { - commit := &Commit{ - hash: "abc123", - message: "this is a test message", - } - const sep = " " - buffer := &bytes.Buffer{} - _ = Printed( - func() []*Commit { return []*Commit{commit} }, - buffer, - sep, - )() - assert.Equal(t, (&pretty{commit}).String()+sep, string(buffer.Bytes()), - "commits.Printed() did not pretty-print the commit correctly") -} - // A git repo initialized and with one commit per each of the messages provided. // This repo is created in a temporary directory; use the cleanup function // to delete it afterwards. diff --git a/internal/commits/filter/filter.go b/internal/commits/filter/filter.go new file mode 100644 index 0000000..7152c94 --- /dev/null +++ b/internal/commits/filter/filter.go @@ -0,0 +1,65 @@ +// Copyright 2019 George Aristy +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package filter + +import ( + "fmt" + "regexp" + + "github.com/llorllale/go-gitlint/internal/commits" + "github.com/llorllale/go-gitlint/internal/commits/issues" +) + +/* + Filters identify issues with a commit. + A filter returning a zero-valued Issue signals that it found no issue + with the commit. +*/ + +// OfSubjectRegex tests a commit's subject with the regex. +func OfSubjectRegex(regex string) func(*commits.Commit) issues.Issue { + return func(c *commits.Commit) issues.Issue { + var issue issues.Issue + matched, err := regexp.MatchString(regex, c.Subject()) + if err != nil { + panic(err) + } + if !matched { + issue = issues.Issue{ + Desc: fmt.Sprintf("subject does not match regex [%s]", regex), + Commit: *c, + } + } + return issue + } +} + +// OfBodyRegex tests a commit's body with the regex. +func OfBodyRegex(regex string) func(*commits.Commit) issues.Issue { + return func(c *commits.Commit) issues.Issue { + var issue issues.Issue + matched, err := regexp.MatchString(regex, c.Body()) + if err != nil { + panic(err) + } + if !matched { + issue = issues.Issue{ + Desc: fmt.Sprintf("body does not conform to regex [%s]", regex), + Commit: *c, + } + } + return issue + } +} diff --git a/internal/commits/filter/filter_test.go b/internal/commits/filter/filter_test.go new file mode 100644 index 0000000..ee05cfd --- /dev/null +++ b/internal/commits/filter/filter_test.go @@ -0,0 +1,66 @@ +// Copyright 2019 George Aristy +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package filter + +import ( + "testing" + + "github.com/llorllale/go-gitlint/internal/commits" + "github.com/stretchr/testify/assert" +) + +func TestOfSubjectRegexMatch(t *testing.T) { + assert.Zero(t, + OfSubjectRegex(`\(#\d+\) [\w ]{10,50}`)( + &commits.Commit{ + Message: "(#123) This is a good subject", + }, + ), + "filter.OfSubjectRegex() must match if the commit's subject matches the regex", + ) +} + +func TestOfSubjectRegexNonMatch(t *testing.T) { + assert.NotZero(t, + OfSubjectRegex(`\(#\d+\) [\w ]{,50}`)( + &commits.Commit{ + Message: "I break all the rules!", + }, + ), + "filter.OfSubjectRegex() must not match if the commit's subject does not match the regex", + ) +} + +func TestOfBodyRegexMatch(t *testing.T) { + assert.Zero(t, + OfBodyRegex(`^.{10,20}$`)( + &commits.Commit{ + Message: "subject\n\nBetween 10 and 20", + }, + ), + "filter.OfBodyRegex() must match if the commit's subject matches the regex", + ) +} + +func TestOfBodyRegexNonMatch(t *testing.T) { + assert.NotZero(t, + OfBodyRegex(`^.{10,20}$`)( + &commits.Commit{ + Message: "subject\n\nMore than twenty characters!", + }, + ), + "filter.OfBodyRegex() must not match if the commit's subject does not match the regex", + ) +} diff --git a/internal/commits/issues/issues.go b/internal/commits/issues/issues.go new file mode 100644 index 0000000..9e647bf --- /dev/null +++ b/internal/commits/issues/issues.go @@ -0,0 +1,67 @@ +// Copyright 2019 George Aristy +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package issues + +import ( + "fmt" + "io" + + "github.com/llorllale/go-gitlint/internal/commits" +) + +// Issue is a problem found with a commit. +type Issue struct { + Desc string + Commit commits.Commit +} + +func (i *Issue) String() string { + return fmt.Sprintf("Issue{Desc=%s Commit=%+v}", i.Desc, i.Commit) +} + +// Issues is a collection of Issues. +type Issues func() []Issue + +// Collected returns a collection of issues identified. +func Collected(filters []func(c *commits.Commit) Issue, cmts commits.Commits) Issues { + return func() []Issue { + issues := make([]Issue, 0) + for _, c := range cmts() { + for _, f := range filters { + if issue := f(c); issue != (Issue{}) { + issues = append(issues, issue) + break + } + } + } + return issues + } +} + +// Printed prints the issues to the writer. +func Printed(w io.Writer, sep string, issues Issues) Issues { + return func() []Issue { + iss := issues() + for i := range iss { + _, err := w.Write( + []byte(fmt.Sprintf("%s%s", iss[i].String(), sep)), + ) + if err != nil { + panic(err) + } + } + return iss + } +} diff --git a/internal/commits/issues/issues_test.go b/internal/commits/issues/issues_test.go new file mode 100644 index 0000000..8dcf58c --- /dev/null +++ b/internal/commits/issues/issues_test.go @@ -0,0 +1,98 @@ +// Copyright 2019 George Aristy +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package issues + +import ( + "testing" + + "github.com/llorllale/go-gitlint/internal/commits" + "github.com/stretchr/testify/assert" +) + +func TestCollected(t *testing.T) { + expected := []*commits.Commit{ + {Hash: "123"}, + {Hash: "456"}, + } + issues := Collected( + []func(*commits.Commit) Issue{ + func(c *commits.Commit) Issue { + var issue Issue + if c.Hash == "123" || c.Hash == "456" { + issue = Issue{ + Desc: "test", + Commit: *c, + } + } + return issue + }, + }, + func() []*commits.Commit { + return append(expected, &commits.Commit{Hash: "789"}) + }, + )() + assert.Len(t, + issues, + 2, + "issues.Collected() must return the filtered commits") + for _, i := range issues { + assert.Contains(t, + expected, &i.Commit, + "issues.Collected() must return the filtered commits") + } +} + +func TestPrinted(t *testing.T) { + const sep = "-" + issues := []Issue{ + { + Desc: "issueA", + Commit: commits.Commit{ + Hash: "1", + Message: "first commit", + }, + }, + { + Desc: "issueB", + Commit: commits.Commit{ + Hash: "2", + Message: "second commit", + }, + }, + } + var expected string + for _, i := range issues { + expected = expected + i.String() + sep + } + writer := &mockWriter{} + Printed( + writer, sep, + func() []Issue { + return issues + }, + )() + assert.Equal(t, + expected, writer.msg, + "issues.Printed() must concatenate Issue.String() with the separator") +} + +type mockWriter struct { + msg string +} + +func (m *mockWriter) Write(b []byte) (int, error) { + m.msg += string(b) + return len(b), nil +}