diff --git a/Makefile b/Makefile index a319ff84d..78afd3c53 100644 --- a/Makefile +++ b/Makefile @@ -6,6 +6,7 @@ MYGOBIN := $(shell go env GOPATH)/bin SHELL := /bin/bash export PATH := $(MYGOBIN):$(PATH) +MODULES := "cmd/config" "api/" "kustomize/" "kyaml/" .PHONY: all all: verify-kustomize @@ -22,6 +23,7 @@ verify-kustomize: \ .PHONY: prow-presubmit-check prow-presubmit-check: \ lint-kustomize \ + test-multi-module \ test-unit-kustomize-all \ test-unit-cmd-all \ test-go-mod \ @@ -227,6 +229,16 @@ test-unit-cmd-all: test-go-mod: ./scripts/check-go-mod.sh +# Environment variables are defined at +# https://github.com/kubernetes/test-infra/blob/master/prow/jobs.md#job-environment-variables +.PHONY: test-multi-module +test-multi-module: $(MYGOBIN)/prchecker + $(MYGOBIN)/prchecker \ + -owner=$(REPO_OWNER) \ + -repo=$(REPO_NAME) \ + -pr=$(PULL_NUMBER) \ + $(MODULES) + .PHONY: test-examples-e2e-kustomize: $(MYGOBIN)/mdrip $(MYGOBIN)/kind ( \ diff --git a/cmd/prchecker/main.go b/cmd/prchecker/main.go index 0c13eeeda..4459e0d61 100644 --- a/cmd/prchecker/main.go +++ b/cmd/prchecker/main.go @@ -27,9 +27,8 @@ import ( // splitCommitsHelp is a help message displayed when a PR has commits which // span modules. -const splitCommitsHelp = "\nModifications to multiple restricted directories occurred.\n" + - "To resolve this please limit code changes in a commit to a single go module.\n\n" + - "You may learn more about how to split your existing commits at: https://git-scm.com/docs/git-rebase#_splitting_commits\n" +const splitCommitsHelp = "\nCommits that include multiple Go modules must be split into multiple commits that each touch no more than one module.\n" + + "Splitting instructions: https://git-scm.com/docs/git-rebase#_splitting_commits\n" // Ignore span pattern defines a regular expression pattern that, when matched, // causes this check to be ignored. @@ -62,16 +61,33 @@ type GitHubService interface { ListCommits(prId int, options *github.ListOptions) ([]*github.RepositoryCommit, *github.Response, error) } -func (repository GitHubRepository) GetPullRequest(prId int) (*github.PullRequest, *github.Response, error) { - return repository.client.PullRequests.Get(context.Background(), *repository.owner, *repository.repo, prId) +// GetPullRequest retrieves details about a pull request from GitHub API +func (repository GitHubRepository) GetPullRequest(prId int) ( + *github.PullRequest, *github.Response, error) { + return repository.client.PullRequests.Get( + context.Background(), + *repository.owner, + *repository.repo, + prId) } -func (repository GitHubRepository) GetCommit(commitSha string) (*github.RepositoryCommit, *github.Response, error) { - return repository.client.Repositories.GetCommit(context.Background(), *repository.owner, *repository.repo, commitSha) +// GetCommit retrieves commit details from GitHub API +func (repository GitHubRepository) GetCommit(commitSha string) ( + *github.RepositoryCommit, *github.Response, error) { + return repository.client.Repositories.GetCommit(context.Background(), + *repository.owner, + *repository.repo, + commitSha) } -func (repository GitHubRepository) ListCommits(prId int, options *github.ListOptions) ([]*github.RepositoryCommit, *github.Response, error) { - return repository.client.PullRequests.ListCommits(context.Background(), *repository.owner, *repository.repo, prId, options) +// ListCommits lists commits in a PR via GitHub API +func (repository GitHubRepository) ListCommits(prId int, options *github.ListOptions) ( + []*github.RepositoryCommit, *github.Response, error) { + return repository.client.PullRequests.ListCommits(context.Background(), + *repository.owner, + *repository.repo, + prId, + options) } func main() { @@ -145,7 +161,7 @@ func StringAllowsModuleSpan(body string) (bool, error) { return regexp.MatchString(ignoreSpanPattern, body) } -// IsModuleSpanAllowed tests a Pull Requests description for a regular +// ModuleSpanAllowed tests a Pull Requests description for a regular // expression. If the expression matches then spanning modules are allowed. func ModuleSpanAllowed(repository GitHubService, pullId int) (bool, error) { @@ -163,9 +179,6 @@ func ModuleSpanAllowed(repository GitHubService, pullId int) (bool, error) { // GetCommitChanges looks up a github commit by SHA and returns a Changeset // containing the modified files in the specified commit. func GetCommitChanges(repository GitHubService, commitSha string) (*Changeset, error) { - - // Note: There are multiple ways to pull a github commit object - // we want a RepositoryCommit. commit, _, err := repository.GetCommit(commitSha) if err != nil { @@ -210,7 +223,7 @@ func GetPullRequestCommits(repository GitHubService, pullrequest int) ([]*Change return changesetResults, nil } -// IsPullRequestSpanningPathList tests if a pull request spans multiple +// PullRequestSpanningPathList tests if a pull request spans multiple // directory paths func PullRequestSpanningPathList(repository GitHubService, pullrequest int, paths []string) (bool, []*Changeset, error) { // Create a buffer for commits @@ -222,7 +235,7 @@ func PullRequestSpanningPathList(repository GitHubService, pullrequest int, path spanningChangesExist := false for _, changeset := range changesets { - if ChangesetSpanningPathList(changeset, paths) { + if changeset.isSpanningPaths(paths) { // When detecting the first spanning changeset print a prefix message if !spanningChangesExist { fmt.Printf("Spanning changesets detected in the following commits:\n\n") @@ -237,9 +250,9 @@ func PullRequestSpanningPathList(repository GitHubService, pullrequest int, path return spanningChangesExist, changesets, nil } -// IsChangesetSpanningPathList tests if a changeset is spanning +// isSpanningPaths tests if a changeset is spanning // multiple directory paths. -func ChangesetSpanningPathList(changeset *Changeset, paths []string) bool { +func (changeset *Changeset) isSpanningPaths(paths []string) bool { matchedPath := "" for _, file := range changeset.files { @@ -254,6 +267,5 @@ func ChangesetSpanningPathList(changeset *Changeset, paths []string) bool { } } - // If more than 1 matched paths then the changeset spans paths return false } diff --git a/cmd/prchecker/main_test.go b/cmd/prchecker/main_test.go index edb6f9dbe..88bea9c31 100644 --- a/cmd/prchecker/main_test.go +++ b/cmd/prchecker/main_test.go @@ -260,7 +260,7 @@ func TestIsChangesetSpanning(t *testing.T) { changeset := &Changeset{files: tt.changeset} - result := ChangesetSpanningPathList(changeset, tt.files) + result := changeset.isSpanningPaths(tt.files) if result != tt.expected { t.Errorf("got %t, want %t", result, tt.expected)