From 6c7382df14b5f5ab9ac0c1760e6997303c614747 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Mon, 31 Jul 2017 16:03:19 +0200
Subject: [PATCH] fix: Check for valid version before performing npm lookup
(#565)
Closes #564
---
lib/workers/package/index.js | 10 ++
lib/workers/package/versions.js | 5 -
.../package/__snapshots__/index.spec.js.snap | 160 ++++++++++++++++++
.../__snapshots__/versions.spec.js.snap | 14 --
test/workers/package/index.spec.js | 13 +-
test/workers/package/versions.spec.js | 12 --
6 files changed, 180 insertions(+), 34 deletions(-)
diff --git a/lib/workers/package/index.js b/lib/workers/package/index.js
index 1ba8ee8613..526e220435 100644
--- a/lib/workers/package/index.js
+++ b/lib/workers/package/index.js
@@ -11,11 +11,21 @@ module.exports = {
// Returns all results for a given dependency config
async function renovatePackage(config) {
const logger = config.logger || pLogger;
+ logger.trace(`renovatePackage(${config.depName})`);
if (config.enabled === false) {
logger.debug('package is disabled');
return [];
}
let results = [];
+ if (!versions.isValidVersion(config.currentVersion)) {
+ results.push({
+ depName: config.depName,
+ type: 'warning',
+ message: `Dependency uses tag "\`${config.currentVersion}\`" as its version so that will never be changed by Renovate`,
+ });
+ logger.debug(results[0].message);
+ return results;
+ }
const npmDep = await npmApi.getDependency(config.depName, logger);
if (npmDep) {
results = await versions.determineUpgrades(npmDep, config);
diff --git a/lib/workers/package/versions.js b/lib/workers/package/versions.js
index 7afbda5eaa..8a0b7ca023 100644
--- a/lib/workers/package/versions.js
+++ b/lib/workers/package/versions.js
@@ -19,11 +19,6 @@ function determineUpgrades(npmDep, config) {
type: 'warning',
};
const currentVersion = config.currentVersion;
- if (!isValidVersion(currentVersion)) {
- result.message = `Dependency uses tag "${currentVersion}" as its version so that will never be changed`;
- logger.debug(result.message);
- return [result];
- }
const versions = npmDep.versions;
if (!versions || Object.keys(versions).length === 0) {
result.message = `No versions returned from registry for this package`;
diff --git a/test/workers/package/__snapshots__/index.spec.js.snap b/test/workers/package/__snapshots__/index.spec.js.snap
index cda4561fda..7f80bfe9a1 100644
--- a/test/workers/package/__snapshots__/index.spec.js.snap
+++ b/test/workers/package/__snapshots__/index.spec.js.snap
@@ -24,6 +24,166 @@ Array [
"assignees",
"reviewers",
"depName",
+ "currentVersion",
"repositoryUrl",
]
`;
+
+exports[`lib/workers/package/index renovatePackage(config) returns error if no npm dep found 1`] = `
+Array [
+ Object {
+ "assignees": Array [],
+ "automerge": "none",
+ "automergeType": "pr",
+ "branchName": "renovate/{{depName}}-{{newVersionMajor}}.x",
+ "commitMessage": "{{semanticPrefix}}Update dependency {{depName}} to version {{newVersion}}",
+ "currentVersion": "1.0.0",
+ "depName": "foo",
+ "group": Object {
+ "branchName": "renovate/{{groupSlug}}",
+ "commitMessage": "{{semanticPrefix}}Renovate {{groupName}} packages",
+ "prBody": "This {{#if isGitHub}}Pull{{else}}Merge{{/if}} Request renovates the package group \\"{{groupName}}\\".
+
+{{#if schedule}}
+**Note**: This PR was created on a configured schedule (\\"{{schedule}}\\"{{#if timezone}} in timezone \`{{timezone}}\`{{/if}}) and will not receive updates outside those times.
+{{/if}}
+
+{{#each upgrades as |upgrade|}}
+- [{{upgrade.depName}}]({{upgrade.repositoryUrl}}): from \`{{upgrade.currentVersion}}\` to \`{{upgrade.newVersion}}\`
+{{/each}}
+
+{{#unless isPin}}
+### Commits
+
+{{#each upgrades as |upgrade|}}
+{{#if upgrade.releases.length}}
+<details>
+<summary>{{upgrade.githubName}}</summary>
+{{#each upgrade.releases as |release|}}
+
+#### {{release.version}}
+{{#each release.commits as |commit|}}
+- [\`{{commit.shortSha}}\`]({{commit.url}}){{commit.message}}
+{{/each}}
+{{/each}}
+
+</details>
+{{/if}}
+{{/each}}
+{{/unless}}
+<br />
+
+{{#if hasErrors}}
+
+---
+
+### Errors
+
+Renovate encountered some errors when processing your repository, so you are being notified here even if they do not directly apply to this PR.
+
+{{#each errors as |error|}}
+- \`{{error.depName}}\`: {{error.message}}
+{{/each}}
+{{/if}}
+
+{{#if hasWarnings}}
+
+---
+
+### Warnings
+
+Please make sure the following warnings are safe to ignore:
+
+{{#each warnings as |warning|}}
+- \`{{warning.depName}}\`: {{warning.message}}
+{{/each}}
+{{/if}}
+
+---
+
+This {{#if isGitHub}}PR{{else}}MR{{/if}} has been generated by [Renovate Bot](https://renovateapp.com).",
+ "prTitle": "{{semanticPrefix}}Renovate {{groupName}} packages",
+ "recreateClosed": true,
+ },
+ "groupName": null,
+ "groupSlug": null,
+ "labels": Array [],
+ "lazyGrouping": true,
+ "message": "Failed to look up dependency",
+ "prBody": "This {{#if isGitHub}}Pull{{else}}Merge{{/if}} Request {{#if isRollback}}rolls back{{else}}updates{{/if}} dependency [{{depName}}]({{repositoryUrl}}) from version \`{{currentVersion}}\` to \`{{newVersion}}\`{{#if isRollback}}. This is necessary and important because version \`{{currentVersion}}\` cannot be found in the npm registry - probably because of it being unpublished.{{/if}}
+{{#if releases.length}}
+
+{{#if schedule}}
+**Note**: This PR was created on a configured schedule (\\"{{schedule}}\\"{{#if timezone}} in timezone \`{{timezone}}\`{{/if}}) and will not receive updates outside those times.
+{{/if}}
+
+### Commits
+
+<details>
+<summary>{{githubName}}</summary>
+
+{{#each releases as |release|}}
+#### {{release.version}}
+{{#each release.commits as |commit|}}
+- [\`{{commit.shortSha}}\`]({{commit.url}}) {{commit.message}}
+{{/each}}
+{{/each}}
+
+</details>
+{{/if}}
+
+{{#if hasErrors}}
+
+---
+
+### Errors
+
+Renovate encountered some errors when processing your repository, so you are being notified here even if they do not directly apply to this PR.
+
+{{#each errors as |error|}}
+- \`{{error.depName}}\`: {{error.message}}
+{{/each}}
+{{/if}}
+
+{{#if hasWarnings}}
+
+---
+
+### Warnings
+
+Please make sure the following warnings are safe to ignore:
+
+{{#each warnings as |warning|}}
+- \`{{warning.depName}}\`: {{warning.message}}
+{{/each}}
+{{/if}}
+
+---
+
+This {{#if isGitHub}}PR{{else}}MR{{/if}} has been generated by [Renovate Bot](https://renovateapp.com).",
+ "prCreation": "immediate",
+ "prTitle": "{{semanticPrefix}}{{#if isPin}}Pin{{else}}{{#if isRollback}}Roll back{{else}}Update{{/if}}{{/if}} dependency {{depName}} to version {{#if isRange}}{{newVersion}}{{else}}{{#if isMajor}}{{newVersionMajor}}.x{{else}}{{newVersion}}{{/if}}{{/if}}",
+ "rebaseStalePrs": false,
+ "recreateClosed": false,
+ "repoIsOnboarded": true,
+ "repositoryUrl": "",
+ "requiredStatusChecks": Array [],
+ "reviewers": Array [],
+ "schedule": "some schedule",
+ "semanticCommits": false,
+ "semanticPrefix": "chore(deps): ",
+ "timezone": null,
+ "type": "error",
+ },
+]
+`;
+
+exports[`lib/workers/package/index renovatePackage(config) returns warning if using invalid version 1`] = `
+Array [
+ Object {
+ "depName": "foo",
+ "message": "Dependency uses tag \\"\`git+ssh://git@github.com/joefraley/eslint-config-meridian.git\`\\" as its version so that will never be changed by Renovate",
+ "type": "warning",
+ },
+]
+`;
diff --git a/test/workers/package/__snapshots__/versions.spec.js.snap b/test/workers/package/__snapshots__/versions.spec.js.snap
index 64966888be..d91ab56823 100644
--- a/test/workers/package/__snapshots__/versions.spec.js.snap
+++ b/test/workers/package/__snapshots__/versions.spec.js.snap
@@ -109,13 +109,6 @@ Object {
}
`;
-exports[`workers/package/versions .determineUpgrades(npmDep, config) return warning if invalid current version 1`] = `
-Object {
- "message": "Dependency uses tag \\"invalid\\" as its version so that will never be changed",
- "type": "warning",
-}
-`;
-
exports[`workers/package/versions .determineUpgrades(npmDep, config) return warning if null versions 1`] = `
Object {
"message": "No versions returned from registry for this package",
@@ -123,13 +116,6 @@ Object {
}
`;
-exports[`workers/package/versions .determineUpgrades(npmDep, config) return warning if using a known tag 1`] = `
-Object {
- "message": "Dependency uses tag \\"next\\" as its version so that will never be changed",
- "type": "warning",
-}
-`;
-
exports[`workers/package/versions .determineUpgrades(npmDep, config) returns both updates if automerging minor 1`] = `
Array [
Object {
diff --git a/test/workers/package/index.spec.js b/test/workers/package/index.spec.js
index c617b7d33c..ac175021ce 100644
--- a/test/workers/package/index.spec.js
+++ b/test/workers/package/index.spec.js
@@ -5,7 +5,6 @@ const defaultConfig = require('../../../lib/config/defaults').getConfig();
const configParser = require('../../../lib/config');
jest.mock('../../../lib/workers/branch/schedule');
-jest.mock('../../../lib/workers/package/versions');
jest.mock('../../../lib/api/npm');
describe('lib/workers/package/index', () => {
@@ -14,23 +13,31 @@ describe('lib/workers/package/index', () => {
beforeEach(() => {
config = configParser.filterConfig(defaultConfig, 'package');
config.depName = 'foo';
+ config.currentVersion = '1.0.0';
});
it('returns empty if package is disabled', async () => {
config.enabled = false;
const res = await pkgWorker.renovatePackage(config);
expect(res).toMatchObject([]);
});
+ it('returns warning if using invalid version', async () => {
+ config.currentVersion =
+ 'git+ssh://git@github.com/joefraley/eslint-config-meridian.git';
+ const res = await pkgWorker.renovatePackage(config);
+ expect(res).toMatchSnapshot();
+ });
it('returns error if no npm dep found', async () => {
config.repoIsOnboarded = true;
config.schedule = 'some schedule';
const res = await pkgWorker.renovatePackage(config);
+ expect(res).toMatchSnapshot();
expect(res).toHaveLength(1);
expect(res[0].type).toEqual('error');
expect(npmApi.getDependency.mock.calls.length).toBe(1);
});
it('returns warning if warning found', async () => {
npmApi.getDependency.mockReturnValueOnce({});
- versions.determineUpgrades.mockReturnValueOnce([
+ versions.determineUpgrades = jest.fn(() => [
{
type: 'warning',
message: 'bad version',
@@ -41,7 +48,7 @@ describe('lib/workers/package/index', () => {
});
it('returns array if upgrades found', async () => {
npmApi.getDependency.mockReturnValueOnce({});
- versions.determineUpgrades.mockReturnValueOnce([{}]);
+ versions.determineUpgrades = jest.fn(() => [{}]);
const res = await pkgWorker.renovatePackage(config);
expect(res).toHaveLength(1);
expect(Object.keys(res[0])).toMatchSnapshot();
diff --git a/test/workers/package/versions.spec.js b/test/workers/package/versions.spec.js
index ecd4468a12..e7fabd0744 100644
--- a/test/workers/package/versions.spec.js
+++ b/test/workers/package/versions.spec.js
@@ -11,18 +11,6 @@ describe('workers/package/versions', () => {
});
describe('.determineUpgrades(npmDep, config)', () => {
- it('return warning if invalid current version', () => {
- config.currentVersion = 'invalid';
- const res = versions.determineUpgrades(qJson, config);
- expect(res).toHaveLength(1);
- expect(res[0]).toMatchSnapshot();
- });
- it('return warning if using a known tag', () => {
- config.currentVersion = 'next';
- const res = versions.determineUpgrades(qJson, config);
- expect(res).toHaveLength(1);
- expect(res[0]).toMatchSnapshot();
- });
it('return warning if null versions', () => {
config.currentVersion = '1.0.0';
const testDep = {
--
GitLab