Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check PR changed files count directly from get pull request endpoint #875

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions pull/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func (loc Locator) IsComplete() bool {
case loc.Value.GetHead().GetRepo().GetID() == 0:
case loc.Value.GetHead().GetRepo().GetName() == "":
case loc.Value.GetHead().GetRepo().GetOwner().GetLogin() == "":
case loc.Value.GetChangedFiles() == 0:
default:
return true
}
Expand Down Expand Up @@ -108,6 +109,7 @@ func (loc Locator) toV4(ctx context.Context, client *githubv4.Client) (*v4PullRe
v4.BaseRefName = loc.Value.GetBase().GetRef()
v4.BaseRepository.DatabaseID = loc.Value.GetBase().GetRepo().GetID()
v4.IsDraft = loc.Value.GetDraft()
v4.ChangedFiles = loc.Value.GetChangedFiles()
return &v4, nil
}

Expand Down Expand Up @@ -270,6 +272,11 @@ func (ghc *GitHubContext) Branches() (base string, head string) {
}

func (ghc *GitHubContext) ChangedFiles() ([]*File, error) {
// Check if changed files exceeds the limit
if ghc.pr.ChangedFiles > MaxPullRequestFiles {
return nil, errors.Errorf("number of changed files (%d) exceeds limit (%d)", ghc.pr.ChangedFiles, MaxPullRequestFiles)
}

if ghc.files == nil {
opt := github.ListOptions{
PerPage: 100,
Expand Down Expand Up @@ -318,9 +325,7 @@ func (ghc *GitHubContext) ChangedFiles() ([]*File, error) {
})
}
}
if len(ghc.files) >= MaxPullRequestFiles {
return nil, errors.Errorf("too many files in pull request, maximum is %d", MaxPullRequestFiles)
}

return ghc.files, nil
}

Expand Down Expand Up @@ -1109,6 +1114,8 @@ type v4PullRequest struct {
BaseRepository struct {
DatabaseID int64
}

ChangedFiles int
}

type v4PageInfo struct {
Expand Down
22 changes: 22 additions & 0 deletions pull/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,27 @@ func TestChangedFiles(t *testing.T) {
assert.Equal(t, 2, filesRule.Count, "cached files were not used")
}

func TestChangedFilesExceedsLimit(t *testing.T) {
rp := &ResponsePlayer{}
rp.AddRule(
ExactPathMatcher("/repos/testorg/testrepo/pulls/123/files"),
"testdata/responses/pull_files.yml",
)
rp.AddRule(
GraphQLNodePrefixMatcher("repository.pullRequest.changedFiles"),
"testdata/responses/pull_changed_files_exceeds_limit.yml",
)

pr := defaultTestPR()
*pr.ChangedFiles = 3001

ctx := makeContext(t, rp, pr, nil)

_, err := ctx.ChangedFiles()
assert.Equal(t, 3001, pr.GetChangedFiles())
assert.Contains(t, err.Error(), "number of changed files (3001) exceeds limit (3000)")
}

func TestChangedFilesNoFiles(t *testing.T) {
rp := &ResponsePlayer{}
filesRule := rp.AddRule(
Expand Down Expand Up @@ -723,6 +744,7 @@ func defaultTestPR() *github.PullRequest {
Name: github.String("testrepo"),
},
},
ChangedFiles: github.Int(1),
}
}

Expand Down
18 changes: 18 additions & 0 deletions pull/testdata/responses/pull_changed_files_exceeds_limit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
- status: 200
body: |
{
"errors": [],
"data": {
"repository": {
"pullRequest": {
"body": "/no-platform",
"createdAt": "2018-01-12T00:11:50Z",
"lastEditedAt": "2018-01-12T00:11:50Z",
"author": {
"login": "agirlnamedsophia"
},
"changedFiles": 3001
}
}
}
}