Skip to content

Commit

Permalink
feat: expose post upgrade command errors in PRs (#8944)
Browse files Browse the repository at this point in the history
  • Loading branch information
fgreinacher committed Mar 20, 2021
1 parent 7af7d3b commit 5f84737
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 12 deletions.
94 changes: 94 additions & 0 deletions lib/workers/branch/index.spec.ts
Expand Up @@ -9,6 +9,7 @@ import * as _npmPostExtract from '../../manager/npm/post-update';
import { PrState } from '../../types';
import * as _exec from '../../util/exec';
import { File, StatusResult } from '../../util/git';
import * as _sanitize from '../../util/sanitize';
import * as _limits from '../global/limits';
import * as _prWorker from '../pr';
import { BranchConfig, PrResult, ProcessBranchResult } from '../types';
Expand All @@ -29,6 +30,7 @@ jest.mock('./automerge');
jest.mock('./commit');
jest.mock('../pr');
jest.mock('../../util/exec');
jest.mock('../../util/sanitize');
jest.mock('../../util/git');
jest.mock('fs-extra');
jest.mock('../global/limits');
Expand All @@ -42,6 +44,7 @@ const automerge = mocked(_automerge);
const commit = mocked(_commit);
const prWorker = mocked(_prWorker);
const exec = mocked(_exec);
const sanitize = mocked(_sanitize);
const fs = mocked(_fs);
const limits = mocked(_limits);

Expand All @@ -65,7 +68,19 @@ describe('workers/branch', () => {
} as never;
schedule.isScheduledNow.mockReturnValue(true);
commit.commitFilesToBranch.mockResolvedValue('abc123');

platform.massageMarkdown.mockImplementation((prBody) => prBody);
prWorker.ensurePr.mockResolvedValue({
prResult: PrResult.Created,
pr: {
title: '',
sourceBranch: '',
state: '',
body: '',
},
});
setAdminConfig();
sanitize.sanitize.mockImplementation((input) => input);
});
afterEach(() => {
platform.ensureComment.mockClear();
Expand Down Expand Up @@ -743,6 +758,85 @@ describe('workers/branch', () => {
expect(exec.exec).toHaveBeenCalledWith('echo semver', {
cwd: '/localDir',
});
const errorMessage = expect.stringContaining(
"Post-upgrade command 'disallowed task' does not match allowed pattern '^echo {{{versioning}}}$'"
);
expect(platform.ensureComment).toHaveBeenCalledWith(
expect.objectContaining({
content: errorMessage,
})
);
expect(sanitize.sanitize).toHaveBeenCalledWith(errorMessage);
});

it('handles post-upgrade task exec errors', async () => {
const updatedPackageFile: File = {
name: 'pom.xml',
contents: 'pom.xml file contents',
};
getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
updatedPackageFiles: [updatedPackageFile],
artifactErrors: [],
} as never);
npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({
artifactErrors: [],
updatedArtifacts: [
{
name: 'yarn.lock',
contents: Buffer.from([1, 2, 3]) /* Binary content */,
},
],
} as never);
git.branchExists.mockReturnValueOnce(true);
platform.getBranchPr.mockResolvedValueOnce({
title: 'rebase!',
state: PrState.Open,
body: `- [x] <!-- rebase-check -->`,
} as never);
git.isBranchModified.mockResolvedValueOnce(true);
git.getRepoStatus.mockResolvedValueOnce({
modified: ['modified_file'],
not_added: [],
deleted: ['deleted_file'],
} as StatusResult);

fs.outputFile.mockReturnValue();
fs.readFile.mockResolvedValueOnce(Buffer.from('modified file content'));

schedule.isScheduledNow.mockReturnValueOnce(false);
commit.commitFilesToBranch.mockResolvedValueOnce(null);

const adminConfig = {
allowedPostUpgradeCommands: ['^exit 1$'],
allowPostUpgradeCommandTemplating: true,
trustLevel: 'high',
};
setAdminConfig(adminConfig);

exec.exec.mockRejectedValue(new Error('Meh, this went wrong!'));

await branchWorker.processBranch({
...config,
localDir: '/localDir',
upgrades: [
{
...defaultConfig,
depName: 'some-dep-name',
postUpgradeTasks: {
commands: ['exit 1'],
fileFilters: ['modified_file', 'deleted_file'],
},
} as never,
],
});

const errorMessage = expect.stringContaining('Meh, this went wrong!');
expect(platform.ensureComment).toHaveBeenCalledWith(
expect.objectContaining({
content: errorMessage,
})
);
expect(sanitize.sanitize).toHaveBeenCalledWith(errorMessage);
});

it('executes post-upgrade tasks with disabled post-upgrade command templating', async () => {
Expand Down
44 changes: 32 additions & 12 deletions lib/workers/branch/index.ts
Expand Up @@ -32,6 +32,7 @@ import {
isBranchModified,
} from '../../util/git';
import { regEx } from '../../util/regex';
import { sanitize } from '../../util/sanitize';
import * as template from '../../util/template';
import { Limit, isLimitReached } from '../global/limits';
import { checkAutoMerge, ensurePr, getPlatformPrOptions } from '../pr';
Expand Down Expand Up @@ -396,20 +397,29 @@ export async function processBranch(
regEx(pattern).test(cmd)
)
) {
const compiledCmd = allowPostUpgradeCommandTemplating
? template.compile(cmd, upgrade)
: cmd;
try {
const compiledCmd = allowPostUpgradeCommandTemplating
? template.compile(cmd, upgrade)
: cmd;

logger.debug({ cmd: compiledCmd }, 'Executing post-upgrade task');

const execResult = await exec(compiledCmd, {
cwd: config.localDir,
});
logger.debug(
{ cmd: compiledCmd },
'Executing post-upgrade task'
);
const execResult = await exec(compiledCmd, {
cwd: config.localDir,
});

logger.debug(
{ cmd: compiledCmd, ...execResult },
'Executed post-upgrade task'
);
logger.debug(
{ cmd: compiledCmd, ...execResult },
'Executed post-upgrade task'
);
} catch (error) {
config.artifactErrors.push({
lockFile: upgrade.packageFile,
stderr: sanitize(error.message),
});
}
} else {
logger.warn(
{
Expand All @@ -418,6 +428,16 @@ export async function processBranch(
},
'Post-upgrade task did not match any on allowed list'
);
config.artifactErrors.push({
lockFile: upgrade.packageFile,
stderr: sanitize(
`Post-upgrade command '${cmd}' does not match allowed pattern${
allowedPostUpgradeCommands.length === 1 ? '' : 's'
} ${allowedPostUpgradeCommands
.map((x) => `'${x}'`)
.join(', ')}`
),
});
}
}

Expand Down

0 comments on commit 5f84737

Please sign in to comment.