Skip to content

Commit

Permalink
refactor: remove platform.getPrFiles (#6702)
Browse files Browse the repository at this point in the history
  • Loading branch information
rarkins committed Jul 8, 2020
1 parent e75d696 commit 4f48cd1
Show file tree
Hide file tree
Showing 13 changed files with 15 additions and 79 deletions.
4 changes: 0 additions & 4 deletions lib/platform/__snapshots__/index.spec.ts.snap
Expand Up @@ -22,7 +22,6 @@ Array [
"getIssueList",
"getPr",
"getPrBody",
"getPrFiles",
"getPrList",
"getRepoForceRebase",
"getRepos",
Expand Down Expand Up @@ -56,7 +55,6 @@ Array [
"getIssueList",
"getPr",
"getPrBody",
"getPrFiles",
"getPrList",
"getRepoForceRebase",
"getRepos",
Expand Down Expand Up @@ -90,7 +88,6 @@ Array [
"getIssueList",
"getPr",
"getPrBody",
"getPrFiles",
"getPrList",
"getRepoForceRebase",
"getRepos",
Expand Down Expand Up @@ -124,7 +121,6 @@ Array [
"getIssueList",
"getPr",
"getPrBody",
"getPrFiles",
"getPrList",
"getRepoForceRebase",
"getRepos",
Expand Down
5 changes: 0 additions & 5 deletions lib/platform/azure/index.ts
Expand Up @@ -233,11 +233,6 @@ export async function getPrList(): Promise<AzurePr[]> {
return config.prList;
}

/* istanbul ignore next */
export async function getPrFiles(pr: Pr): Promise<string[]> {
return git.getBranchFiles(pr.branchName, pr.targetBranch);
}

export async function getPr(pullRequestId: number): Promise<Pr | null> {
logger.debug(`getPr(${pullRequestId})`);
if (!pullRequestId) {
Expand Down
5 changes: 0 additions & 5 deletions lib/platform/bitbucket-server/index.ts
Expand Up @@ -365,11 +365,6 @@ export async function getPrList(_args?: any): Promise<Pr[]> {
return config.prList;
}

/* istanbul ignore next */
export async function getPrFiles(pr: Pr): Promise<string[]> {
return git.getBranchFiles(pr.branchName, pr.targetBranch);
}

// TODO: coverage
// istanbul ignore next
export async function findPr({
Expand Down
5 changes: 0 additions & 5 deletions lib/platform/bitbucket/index.ts
Expand Up @@ -211,11 +211,6 @@ export async function getPrList(): Promise<Pr[]> {
return config.prList;
}

/* istanbul ignore next */
export async function getPrFiles(pr: Pr): Promise<string[]> {
return git.getBranchFiles(pr.branchName, pr.targetBranch);
}

export async function findPr({
branchName,
prTitle,
Expand Down
1 change: 0 additions & 1 deletion lib/platform/common.ts
Expand Up @@ -188,7 +188,6 @@ export interface Platform {
getVulnerabilityAlerts(): Promise<VulnerabilityAlert[]>;
initRepo(config: RepoParams): Promise<RepoConfig>;
getPrList(): Promise<Pr[]>;
getPrFiles(pr: Pr): Promise<string[]>;
ensureIssueClosing(title: string): Promise<void>;
ensureIssue(
issueConfig: EnsureIssueConfig
Expand Down
6 changes: 0 additions & 6 deletions lib/platform/gitea/index.ts
Expand Up @@ -468,11 +468,6 @@ const platform: Platform = {
return config.prList;
},

/* istanbul ignore next */
async getPrFiles(pr: Pr): Promise<string[]> {
return git.getBranchFiles(pr.branchName, pr.targetBranch);
},

async getPr(number: number): Promise<Pr | null> {
// Search for pull request in cached list or attempt to query directly
const prList = await platform.getPrList();
Expand Down Expand Up @@ -883,7 +878,6 @@ export const {
getPr,
getPrBody,
getPrList,
getPrFiles,
getRepoForceRebase,
getRepos,
getVulnerabilityAlerts,
Expand Down
5 changes: 0 additions & 5 deletions lib/platform/github/index.ts
Expand Up @@ -907,11 +907,6 @@ export async function getPrList(): Promise<Pr[]> {
return config.prList;
}

/* istanbul ignore next */
export async function getPrFiles(pr: Pr): Promise<string[]> {
return git.getBranchFiles(pr.branchName, pr.targetBranch);
}

export async function findPr({
branchName,
prTitle,
Expand Down
5 changes: 0 additions & 5 deletions lib/platform/gitlab/index.ts
Expand Up @@ -1017,11 +1017,6 @@ export async function getPrList(): Promise<Pr[]> {
return config.prList;
}

/* istanbul ignore next */
export async function getPrFiles(pr: Pr): Promise<string[]> {
return git.getBranchFiles(pr.branchName, pr.targetBranch);
}

function matchesState(state: string, desiredState: string): boolean {
if (desiredState === PR_STATE_ALL) {
return true;
Expand Down
8 changes: 1 addition & 7 deletions lib/util/git/__snapshots__/index.spec.ts.snap
Expand Up @@ -2,13 +2,7 @@

exports[`platform/git getBranchCommit(branchName) should throw if branch does not exist 1`] = `[Error: Cannot fetch commit for branch that does not exist: not_found]`;

exports[`platform/git getBranchFiles(branchName, baseBranchName?) detects changed files 1`] = `
Array [
"some-new-file",
]
`;

exports[`platform/git getBranchFiles(branchName, baseBranchName?) detects changed files compared to current base branch 1`] = `
exports[`platform/git getBranchFiles(branchName) detects changed files compared to current base branch 1`] = `
Array [
"some-new-file",
]
Expand Down
20 changes: 1 addition & 19 deletions lib/util/git/index.spec.ts
Expand Up @@ -145,25 +145,7 @@ describe('platform/git', () => {
});
});

describe('getBranchFiles(branchName, baseBranchName?)', () => {
it('detects changed files', async () => {
const hex = await git.getBranchCommit('master');
await git.createBranch('renovate/branch_with_changes', hex);
const file = {
name: 'some-new-file',
contents: 'some new-contents',
};
await git.commitFiles({
branchName: 'renovate/branch_with_changes',
files: [file],
message: 'Create something',
});
const branchFiles = await git.getBranchFiles(
'renovate/branch_with_changes',
'master'
);
expect(branchFiles).toMatchSnapshot();
});
describe('getBranchFiles(branchName)', () => {
it('detects changed files compared to current base branch', async () => {
const hex = await git.getBranchCommit('master');
await git.createBranch('renovate/branch_with_changes', hex);
Expand Down
10 changes: 2 additions & 8 deletions lib/util/git/index.ts
Expand Up @@ -435,15 +435,9 @@ export async function getBranchLastCommitTime(
}
}

export async function getBranchFiles(
branchName: string,
baseBranchName?: string
): Promise<string[]> {
export async function getBranchFiles(branchName: string): Promise<string[]> {
try {
const diff = await git.diffSummary([
branchName,
baseBranchName || config.baseBranch,
]);
const diff = await git.diffSummary([branchName, config.baseBranch]);
return diff.files.map((file) => file.file);
} catch (err) /* istanbul ignore next */ {
checkForPlatformFailure(err);
Expand Down
15 changes: 8 additions & 7 deletions lib/workers/pr/code-owners.spec.ts
@@ -1,9 +1,10 @@
import { mock } from 'jest-mock-extended';
import { fs, platform } from '../../../test/util';
import { fs, git } from '../../../test/util';
import { Pr } from '../../platform';
import { codeOwnersForPr } from './code-owners';

jest.mock('../../util/fs');
jest.mock('../../util/git');

describe('workers/pr/code-owners', () => {
describe('codeOwnersForPr', () => {
Expand All @@ -14,15 +15,15 @@ describe('workers/pr/code-owners', () => {
});
it('returns global code owner', async () => {
fs.readLocalFile.mockResolvedValueOnce(['* @jimmy'].join('\n'));
platform.getPrFiles.mockResolvedValueOnce(['README.md']);
git.getBranchFiles.mockResolvedValueOnce(['README.md']);
const codeOwners = await codeOwnersForPr(pr);
expect(codeOwners).toEqual(['@jimmy']);
});
it('returns more specific code owners', async () => {
fs.readLocalFile.mockResolvedValueOnce(
['* @jimmy', 'package.json @john @maria'].join('\n')
);
platform.getPrFiles.mockResolvedValueOnce(['package.json']);
git.getBranchFiles.mockResolvedValueOnce(['package.json']);
const codeOwners = await codeOwnersForPr(pr);
expect(codeOwners).toEqual(['@john', '@maria']);
});
Expand All @@ -36,21 +37,21 @@ describe('workers/pr/code-owners', () => {
' package.json @john @maria ',
].join('\n')
);
platform.getPrFiles.mockResolvedValueOnce(['package.json']);
git.getBranchFiles.mockResolvedValueOnce(['package.json']);
const codeOwners = await codeOwnersForPr(pr);
expect(codeOwners).toEqual(['@john', '@maria']);
});
it('returns empty array when no code owners set', async () => {
fs.readLocalFile.mockResolvedValueOnce(null);
platform.getPrFiles.mockResolvedValueOnce(['package.json']);
git.getBranchFiles.mockResolvedValueOnce(['package.json']);
const codeOwners = await codeOwnersForPr(pr);
expect(codeOwners).toEqual([]);
});
it('returns empty array when no code owners match', async () => {
fs.readLocalFile.mockResolvedValueOnce(
['package-lock.json @mike'].join('\n')
);
platform.getPrFiles.mockResolvedValueOnce(['yarn.lock']);
git.getBranchFiles.mockResolvedValueOnce(['yarn.lock']);
const codeOwners = await codeOwnersForPr(pr);
expect(codeOwners).toEqual([]);
});
Expand All @@ -75,7 +76,7 @@ describe('workers/pr/code-owners', () => {
}
return Promise.resolve(null);
});
platform.getPrFiles.mockResolvedValueOnce(['README.md']);
git.getBranchFiles.mockResolvedValueOnce(['README.md']);
const codeOwners = await codeOwnersForPr(pr);
expect(codeOwners).toEqual(['@mike']);
});
Expand Down
5 changes: 3 additions & 2 deletions lib/workers/pr/code-owners.ts
@@ -1,7 +1,8 @@
import ignore from 'ignore';
import { logger } from '../../logger';
import { Pr, platform } from '../../platform';
import { Pr } from '../../platform';
import { readLocalFile } from '../../util/fs';
import { getBranchFiles } from '../../util/git';

export async function codeOwnersForPr(pr: Pr): Promise<string[]> {
try {
Expand All @@ -15,7 +16,7 @@ export async function codeOwnersForPr(pr: Pr): Promise<string[]> {
return [];
}

const prFiles = await platform.getPrFiles(pr);
const prFiles = await getBranchFiles(pr.branchName);
const rules = codeOwnersFile
.split('\n')
.map((line) => line.trim())
Expand Down

0 comments on commit 4f48cd1

Please sign in to comment.