honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(docs-infra): correctly check PR files on preview server (#25671) 

According to the docs, the response of GitHub's [PR files API][1]
_"includes a maximum of 300 files"_. This means that if a PR contains
more files, it is possible that not all files are retrieved (which
could, for example, give a false negative for the "significant files
touched" check - not likely but possible).

This commit fixes it by using paginated requests to retrieve all changed
files.

[1]: https://developer.github.com/v3/pulls/#list-pull-requests-files

PR Close #25671
This commit is contained in:
George Kalpakas 2018-08-27 12:07:35 +03:00 committed by Kara Erickson
parent c8c8436e58
commit f378454c92
3 changed files with 11 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-pull-requests.ts
View File

@ -74,6 +74,6 @@ export class GithubPullRequests {
*/
public fetchFiles(pr: number): Promise<FileInfo[]> {
assert(pr > 0, `Invalid PR number: ${pr}`);
return this.api.get<FileInfo[]>(`/repos/${this.repoSlug}/pulls/${pr}/files`);
return this.api.getPaginated<FileInfo>(`/repos/${this.repoSlug}/pulls/${pr}/files`);
}
}

2
aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/mock-external-apis.ts
View File

@ -76,7 +76,7 @@ const GITHUB_PULLS_URL = `/repos/${AIO_GITHUB_ORGANIZATION}/${AIO_GITHUB_REPO}/p
const GITHUB_TEAMS_URL = `/orgs/${AIO_GITHUB_ORGANIZATION}/teams`;
const getIssueUrl = (prNum: number) => `${GITHUB_ISSUES_URL}/${prNum}`;
const getFilesUrl = (prNum: number) => `${GITHUB_PULLS_URL}/${prNum}/files`;
const getFilesUrl = (prNum: number, pageNum = 0) => `${GITHUB_PULLS_URL}/${prNum}/files?page=${pageNum}&per_page=100`;
const getCommentUrl = (prNum: number) => `${getIssueUrl(prNum)}/comments`;
const getTeamMembershipUrl = (teamId: number, username: string) => `/teams/${teamId}/memberships/${username}`;

15
aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-pull-requests.spec.ts
View File

@ -4,13 +4,13 @@ import {GithubPullRequests} from '../../lib/common/github-pull-requests';
// Tests
describe('GithubPullRequests', () => {
let githubApi: jasmine.SpyObj<GithubApi>;
beforeEach(() => {
githubApi = jasmine.createSpyObj('githubApi', ['post', 'get', 'getPaginated']);
});
describe('constructor()', () => {
it('should throw if \'githubOrg\' is missing or empty', () => {
@ -95,6 +95,7 @@ describe('GithubPullRequests', () => {
done();
});
});
});
@ -128,8 +129,10 @@ describe('GithubPullRequests', () => {
githubApi.getPaginated.and.returnValue('Test');
expect(prs.fetchAll() as any).toBe('Test');
});
});
describe('fetchFiles()', () => {
let prs: GithubPullRequests;
@ -138,21 +141,21 @@ describe('GithubPullRequests', () => {
});
it('should make a GET request to GitHub with the correct pathname', () => {
it('should make a paginated GET request to GitHub with the correct pathname', () => {
prs.fetchFiles(42);
expect(githubApi.get).toHaveBeenCalledWith('/repos/foo/bar/pulls/42/files');
expect(githubApi.getPaginated).toHaveBeenCalledWith('/repos/foo/bar/pulls/42/files');
});
it('should resolve with the data returned from GitHub', done => {
const expected: any = [{ sha: 'ABCDE', filename: 'a/b/c'}, { sha: '12345', filename: 'x/y/z' }];
githubApi.get.and.callFake(() => Promise.resolve(expected));
const expected: any = [{sha: 'ABCDE', filename: 'a/b/c'}, {sha: '12345', filename: 'x/y/z'}];
githubApi.getPaginated.and.callFake(() => Promise.resolve(expected));
prs.fetchFiles(42).then(data => {
expect(data).toEqual(expected);
done();
});
});
});
});