Skip to content

Commit

Permalink
refactor(lib/workers): separate get dep warning func into two diff fu…
Browse files Browse the repository at this point in the history
…nctions (#16165)
  • Loading branch information
MaronHatoum committed Jun 27, 2022
1 parent fa18c74 commit 96a71ec
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 56 deletions.
27 changes: 23 additions & 4 deletions lib/workers/repository/errors-warnings.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { RenovateConfig, getConfig } from '../../../test/util';
import type { PackageFile } from '../../modules/manager/types';
import { getDepWarnings, getErrors, getWarnings } from './errors-warnings';
import { getDepWarningsPR, getErrors, getWarnings } from './errors-warnings';

describe('workers/repository/errors-warnings', () => {
describe('getWarnings()', () => {
Expand Down Expand Up @@ -31,14 +31,20 @@ describe('workers/repository/errors-warnings', () => {
"
`);
});

it('getWarning returns empty string', () => {
config.warnings = [];
const res = getWarnings(config);
expect(res).toBe('');
});
});

describe('getDepWarnings()', () => {
describe('getDepWarningsPR()', () => {
beforeEach(() => {
jest.resetAllMocks();
});

it('returns warning text', () => {
it('returns pr warning text', () => {
const packageFiles: Record<string, PackageFile[]> = {
npm: [
{
Expand Down Expand Up @@ -70,7 +76,8 @@ describe('workers/repository/errors-warnings', () => {
},
],
};
const res = getDepWarnings(packageFiles);

const res = getDepWarningsPR(packageFiles);
expect(res).toMatchInlineSnapshot(`
"
---
Expand All @@ -87,6 +94,12 @@ describe('workers/repository/errors-warnings', () => {
"
`);
});

it('PR warning returns empty string', () => {
const packageFiles: Record<string, PackageFile[]> = {};
const res = getDepWarningsPR(packageFiles);
expect(res).toBe('');
});
});

describe('getErrors()', () => {
Expand Down Expand Up @@ -117,5 +130,11 @@ describe('workers/repository/errors-warnings', () => {
"
`);
});

it('getError returns empty string', () => {
config.errors = [];
const res = getErrors(config);
expect(res).toBe('');
});
});
});
101 changes: 51 additions & 50 deletions lib/workers/repository/errors-warnings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,81 +3,82 @@ import type { RenovateConfig } from '../../config/types';
import { logger } from '../../logger';
import type { PackageFile } from '../../modules/manager/types';
import { emojify } from '../../util/emoji';
import type { DepWarnings } from '../types';

export function getWarnings(config: RenovateConfig): string {
const warnings = config.warnings!;
if (!warnings.length) {
if (!config.warnings?.length) {
return '';
}
let warningText = `\n# Warnings (${warnings.length})\n\n`;
let warningText = `\n# Warnings (${config.warnings.length})\n\n`;
warningText += `Please correct - or verify that you can safely ignore - these warnings before you merge this PR.\n\n`;
warnings.forEach((w) => {
for (const w of config.warnings) {
warningText += `- \`${w.topic}\`: ${w.message}\n`;
});
}
warningText += '\n---\n';
return warningText;
}

export function getErrors(config: RenovateConfig): string {
let errorText = '';
const errors = config.errors!;
if (!errors.length) {
if (!config.errors?.length) {
return '';
}
errorText = `\n# Errors (${errors.length})\n\n`;
let errorText = `\n# Errors (${config.errors.length})\n\n`;
errorText += `Renovate has found errors that you should fix (in this branch) before finishing this PR.\n\n`;
errors.forEach((e) => {
for (const e of config.errors) {
errorText += `- \`${e.topic}\`: ${e.message}\n`;
});
}
errorText += '\n---\n';
return errorText;
}

export function getDepWarnings(
function getDepWarnings(
packageFiles: Record<string, PackageFile[]>
): string {
let warningText = '';
try {
const warnings: string[] = [];
const warningFiles: string[] = [];
for (const files of Object.values(packageFiles || {})) {
for (const file of files || []) {
if (file.deps) {
for (const dep of file.deps || []) {
if (dep.warnings?.length) {
const message = dep.warnings[0].message;
if (!warnings.includes(message)) {
warnings.push(message);
}
if (!warningFiles.includes(file.packageFile!)) {
warningFiles.push(file.packageFile!);
}
): DepWarnings {
const warnings: string[] = [];
const warningFiles: string[] = [];
for (const files of Object.values(packageFiles ?? {})) {
for (const file of files ?? []) {
// TODO: remove condition when type is fixed (#7154)
if (file.packageFile) {
for (const dep of file.deps ?? []) {
for (const w of dep.warnings ?? []) {
const message = w.message;
if (!warnings.includes(message)) {
warnings.push(message);
}
if (!warningFiles.includes(file.packageFile)) {
warningFiles.push(file.packageFile);
}
}
}
}
}
if (!warnings.length) {
return '';
}
logger.debug(
{ warnings, warningFiles },
'Found package lookup warnings in onboarding'
);
warningText = emojify(
`\n---\n\n### :warning: Dependency Lookup Warnings :warning:\n\n`
);
warningText += `Please correct - or verify that you can safely ignore - these lookup failures before you merge this PR.\n\n`;
warnings.forEach((w) => {
warningText += `- \`${w}\`\n`;
});
warningText +=
'\nFiles affected: ' +
warningFiles.map((f) => '`' + f + '`').join(', ') +
'\n\n';
} catch (err) {
// istanbul ignore next
logger.error({ err }, 'Error generating dep warnings text');
}
return { warnings, warningFiles };
}

export function getDepWarningsPR(
packageFiles: Record<string, PackageFile[]>
): string {
const { warnings, warningFiles } = getDepWarnings(packageFiles);
let warningText = '';
if (!warnings.length) {
return '';
}
logger.debug(
{ warnings, warningFiles },
'Found package lookup warnings in onboarding'
);
warningText = emojify(
`\n---\n\n### :warning: Dependency Lookup Warnings :warning:\n\n`
);
warningText += `Please correct - or verify that you can safely ignore - these lookup failures before you merge this PR.\n\n`;
for (const w of warnings) {
warningText += `- \`${w}\`\n`;
}
warningText +=
'\nFiles affected: ' +
warningFiles.map((f) => '`' + f + '`').join(', ') +
'\n\n';
return warningText;
}
8 changes: 6 additions & 2 deletions lib/workers/repository/onboarding/pr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ import {
} from '../../../../util/git';
import * as template from '../../../../util/template';
import type { BranchConfig } from '../../../types';
import { getDepWarnings, getErrors, getWarnings } from '../../errors-warnings';
import {
getDepWarningsPR,
getErrors,
getWarnings,
} from '../../errors-warnings';
import { getPlatformPrOptions } from '../../update/pr';
import { prepareLabels } from '../../update/pr/labels';
import { addParticipants } from '../../update/pr/participants';
Expand Down Expand Up @@ -109,7 +113,7 @@ If you need any further assistance then you can also [request help here](${
prBody = prBody.replace('{{CONFIG}}\n', configDesc);
prBody = prBody.replace(
'{{WARNINGS}}\n',
getWarnings(config) + getDepWarnings(packageFiles!)
getWarnings(config) + getDepWarningsPR(packageFiles!)
);
prBody = prBody.replace('{{ERRORS}}\n', getErrors(config));
prBody = prBody.replace('{{BASEBRANCH}}\n', getBaseBranchDesc(config));
Expand Down
5 changes: 5 additions & 0 deletions lib/workers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,8 @@ export interface WorkerExtractConfig
enabledManagers?: string[];
enabled?: boolean;
}

export interface DepWarnings {
warnings: string[];
warningFiles: string[];
}

0 comments on commit 96a71ec

Please sign in to comment.