From f04adc50df6c2c6c355963fb9d6c32081a333bf4 Mon Sep 17 00:00:00 2001 From: Yura Beznos <git-ocado@you-ra.info> Date: Tue, 14 Apr 2020 05:58:27 +0100 Subject: [PATCH] refactor: extract and update now decoupled (#5835) --- lib/workers/common.ts | 4 +++ lib/workers/pr/index.spec.ts | 10 ++++++++ lib/workers/pr/index.ts | 4 +-- lib/workers/repository/extract/index.ts | 12 +++++++++ lib/workers/repository/index.spec.ts | 4 +-- lib/workers/repository/index.ts | 14 ++++++++--- .../process/__snapshots__/index.spec.ts.snap | 1 - .../repository/process/extract-update.spec.ts | 7 +++--- .../repository/process/extract-update.ts | 23 ++++++++++++----- lib/workers/repository/process/index.spec.ts | 7 +++--- lib/workers/repository/process/index.ts | 25 +++++++++++++------ 11 files changed, 82 insertions(+), 29 deletions(-) diff --git a/lib/workers/common.ts b/lib/workers/common.ts index f4c5f4d916..0a936cc520 100644 --- a/lib/workers/common.ts +++ b/lib/workers/common.ts @@ -13,6 +13,7 @@ import { } from '../config'; import { File, PlatformPrOptions } from '../platform'; import { Release } from '../datasource'; +import { ChangeLogResult } from './pr/changelog/common'; export interface BranchUpgradeConfig extends Merge<RenovateConfig, PackageDependency>, @@ -47,8 +48,11 @@ export interface BranchUpgradeConfig releaseTimestamp?: string; sourceDirectory?: string; + updatedPackageFiles?: File[]; updatedArtifacts?: File[]; + + logJSON?: ChangeLogResult; } export enum PrResult { diff --git a/lib/workers/pr/index.spec.ts b/lib/workers/pr/index.spec.ts index 6bf59c556f..6040791820 100644 --- a/lib/workers/pr/index.spec.ts +++ b/lib/workers/pr/index.spec.ts @@ -6,6 +6,7 @@ import { mocked } from '../../../test/util'; import { BranchStatus } from '../../types'; import { PLATFORM_TYPE_GITLAB } from '../../constants/platforms'; import { PrResult } from '../common'; +import { extractChangeLogJSON } from '../repository/extract'; const changelogHelper = mocked(_changelogHelper); const platform = mocked(_platform); @@ -180,6 +181,7 @@ describe('workers/pr', () => { }); it('should create PR if success', async () => { platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green); + await extractChangeLogJSON([config]); config.prCreation = 'status-success'; config.automerge = true; config.schedule = 'before 5am'; @@ -217,6 +219,9 @@ describe('workers/pr', () => { config.updateType = 'lockFileMaintenance'; config.recreateClosed = true; config.rebaseWhen = 'never'; + + await extractChangeLogJSON([config]); + const { prResult, pr } = await prWorker.ensurePr(config); expect(prResult).toEqual(PrResult.Created); expect(pr).toMatchObject({ displayNumber: 'New Pull Request' }); @@ -230,6 +235,7 @@ describe('workers/pr', () => { config.schedule = 'before 5am'; config.timezone = 'some timezone'; config.rebaseWhen = 'behind-base-branch'; + await extractChangeLogJSON([config]); const { prResult, pr } = await prWorker.ensurePr(config); expect(prResult).toEqual(PrResult.Created); expect(pr).toMatchObject({ displayNumber: 'New Pull Request' }); @@ -369,6 +375,7 @@ describe('workers/pr', () => { config.semanticCommitScope = null; config.automerge = true; config.schedule = 'before 5am'; + await extractChangeLogJSON([config]); const { prResult, pr } = await prWorker.ensurePr(config); expect(prResult).toEqual(PrResult.NotUpdated); expect(platform.updatePr.mock.calls).toMatchSnapshot(); @@ -383,6 +390,7 @@ describe('workers/pr', () => { config.semanticCommitScope = null; config.automerge = true; config.schedule = 'before 5am'; + await extractChangeLogJSON([config]); const { prResult, pr } = await prWorker.ensurePr(config); expect(prResult).toEqual(PrResult.NotUpdated); expect(platform.updatePr).toHaveBeenCalledTimes(0); @@ -392,6 +400,7 @@ describe('workers/pr', () => { config.newValue = '1.2.0'; config.automerge = true; config.schedule = 'before 5am'; + await extractChangeLogJSON([config]); platform.getBranchPr.mockResolvedValueOnce(existingPr); const { prResult, pr } = await prWorker.ensurePr(config); expect(prResult).toEqual(PrResult.NotUpdated); @@ -455,6 +464,7 @@ describe('workers/pr', () => { platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green); config.prCreation = 'status-success'; config.privateRepo = false; + await extractChangeLogJSON([config]); const { prResult, pr } = await prWorker.ensurePr(config); expect(prResult).toEqual(PrResult.Created); expect(pr).toMatchObject({ displayNumber: 'New Pull Request' }); diff --git a/lib/workers/pr/index.ts b/lib/workers/pr/index.ts index 27f6103d52..9c81a2ec68 100644 --- a/lib/workers/pr/index.ts +++ b/lib/workers/pr/index.ts @@ -1,7 +1,7 @@ import sampleSize from 'lodash/sampleSize'; import uniq from 'lodash/uniq'; import { logger } from '../../logger'; -import { ChangeLogError, getChangeLogJSON } from './changelog'; +import { ChangeLogError } from './changelog'; import { getPrBody } from './body'; import { platform, Pr, PlatformPrOptions } from '../../platform'; import { BranchConfig, PrResult } from '../common'; @@ -193,7 +193,7 @@ export async function ensurePr( } processedUpgrades.push(upgradeKey); - const logJSON = await getChangeLogJSON(upgrade); + const logJSON = upgrade.logJSON; if (logJSON) { if (typeof logJSON.error === 'undefined') { diff --git a/lib/workers/repository/extract/index.ts b/lib/workers/repository/extract/index.ts index 639e3144be..3f786d4be0 100644 --- a/lib/workers/repository/extract/index.ts +++ b/lib/workers/repository/extract/index.ts @@ -7,6 +7,8 @@ import { } from '../../../config'; import { getManagerPackageFiles } from './manager-files'; import { PackageFile } from '../../../manager/common'; +import { BranchConfig } from '../../common'; +import { getChangeLogJSON } from '../../pr/changelog'; export async function extractAllDependencies( config: RenovateConfig @@ -49,3 +51,13 @@ export async function extractAllDependencies( logger.debug(`Found ${fileCount} package file(s)`); return extractions; } + +export async function extractChangeLogJSON( + branches: BranchConfig[] +): Promise<void> { + for (const branch of branches) { + for (const upgrade of branch.upgrades) { + upgrade.logJSON = await getChangeLogJSON(upgrade); + } + } +} diff --git a/lib/workers/repository/index.spec.ts b/lib/workers/repository/index.spec.ts index e2155470a4..e57f04cd88 100644 --- a/lib/workers/repository/index.spec.ts +++ b/lib/workers/repository/index.spec.ts @@ -3,7 +3,7 @@ import { mock } from 'jest-mock-extended'; import { renovateRepository } from '.'; import * as _process from './process'; import { mocked, RenovateConfig, getConfig } from '../../../test/util'; -import { ExtractAndUpdateResult } from './process/extract-update'; +import { ExtractResult } from './process/extract-update'; const process = mocked(_process); @@ -19,7 +19,7 @@ describe('workers/repository', () => { config = getConfig(); }); it('runs', async () => { - process.processRepo.mockResolvedValue(mock<ExtractAndUpdateResult>()); + process.processRepo.mockResolvedValue(mock<ExtractResult>()); const res = await renovateRepository(config); expect(res).toMatchSnapshot(); }); diff --git a/lib/workers/repository/index.ts b/lib/workers/repository/index.ts index 6aaf8fe16f..44763f7afd 100644 --- a/lib/workers/repository/index.ts +++ b/lib/workers/repository/index.ts @@ -6,10 +6,11 @@ import { logger, setMeta } from '../../logger'; import { initRepo } from './init'; import { ensureOnboardingPr } from './onboarding/pr'; import { processResult, ProcessResult } from './result'; -import { processRepo } from './process'; +import { processRepo, updateRepo } from './process'; import { finaliseRepo } from './finalise'; import { ensureMasterIssue } from './master-issue'; import { RenovateConfig } from '../../config'; +import { extractChangeLogJSON } from './extract'; let renovateVersion = 'unknown'; try { @@ -31,10 +32,15 @@ export async function renovateRepository( await fs.ensureDir(config.localDir); logger.debug('Using localDir: ' + config.localDir); config = await initRepo(config); - const { res, branches, branchList, packageFiles } = await processRepo( - config - ); + const { branches, branchList, packageFiles } = await processRepo(config); + await extractChangeLogJSON(branches); await ensureOnboardingPr(config, packageFiles, branches); + const { res } = await updateRepo( + config, + branches, + branchList, + packageFiles + ); if (res !== 'automerged') { await ensureMasterIssue(config, branches); } diff --git a/lib/workers/repository/process/__snapshots__/index.spec.ts.snap b/lib/workers/repository/process/__snapshots__/index.spec.ts.snap index b9bf68470c..32e10c7425 100644 --- a/lib/workers/repository/process/__snapshots__/index.spec.ts.snap +++ b/lib/workers/repository/process/__snapshots__/index.spec.ts.snap @@ -10,7 +10,6 @@ Object { undefined, undefined, ], - "res": undefined, } `; diff --git a/lib/workers/repository/process/extract-update.spec.ts b/lib/workers/repository/process/extract-update.spec.ts index fe44bf28e7..fb78feeac3 100644 --- a/lib/workers/repository/process/extract-update.spec.ts +++ b/lib/workers/repository/process/extract-update.spec.ts @@ -1,4 +1,4 @@ -import { extractAndUpdate } from './extract-update'; +import { extract, update } from './extract-update'; import * as _branchify from '../updates/branchify'; import { mocked } from '../../../../test/util'; @@ -16,13 +16,14 @@ branchify.branchifyUpgrades.mockResolvedValueOnce({ }); describe('workers/repository/process/extract-update', () => { - describe('extractAndUpdate()', () => { + describe('extract()', () => { it('runs', async () => { const config = { repoIsOnboarded: true, suppressNotifications: ['deprecationWarningIssues'], }; - await extractAndUpdate(config); + const res = await extract(config); + await update(config, res.branches, res.branchList, res.packageFiles); }); }); }); diff --git a/lib/workers/repository/process/extract-update.ts b/lib/workers/repository/process/extract-update.ts index 5e258e56f3..ef9a59848b 100644 --- a/lib/workers/repository/process/extract-update.ts +++ b/lib/workers/repository/process/extract-update.ts @@ -9,16 +9,17 @@ import { PackageFile } from '../../../manager/common'; import { RenovateConfig } from '../../../config'; import { BranchConfig } from '../../common'; -export type ExtractAndUpdateResult = { - res: WriteUpdateResult | undefined; +export type ExtractResult = { branches: BranchConfig[]; branchList: string[]; packageFiles?: Record<string, PackageFile[]>; }; -export async function extractAndUpdate( - config: RenovateConfig -): Promise<ExtractAndUpdateResult> { +export type UpdateResult = { + res: WriteUpdateResult | undefined; +}; + +export async function extract(config: RenovateConfig): Promise<ExtractResult> { logger.debug('extractAndUpdate()'); const packageFiles = await extractAllDependencies(config); logger.trace({ config: packageFiles }, 'packageFiles'); @@ -30,10 +31,20 @@ export async function extractAndUpdate( packageFiles ); sortBranches(branches); + return { branches, branchList, packageFiles }; +} + +export async function update( + config: RenovateConfig, + branches: BranchConfig[], + branchList: string[], + packageFiles?: Record<string, PackageFile[]> +): Promise<UpdateResult> { let res: WriteUpdateResult | undefined; // istanbul ignore else if (config.repoIsOnboarded) { res = await writeUpdates(config, packageFiles, branches); } - return { res, branches, branchList, packageFiles }; + + return { res }; } diff --git a/lib/workers/repository/process/index.spec.ts b/lib/workers/repository/process/index.spec.ts index 3bac4fb50c..b0a98f473e 100644 --- a/lib/workers/repository/process/index.spec.ts +++ b/lib/workers/repository/process/index.spec.ts @@ -1,10 +1,10 @@ -import { processRepo } from './index'; +import { processRepo, updateRepo } from './index'; import * as _extractUpdate from './extract-update'; import { getConfig, mocked, RenovateConfig } from '../../../../test/util'; jest.mock('./extract-update'); -const extractAndUpdate = mocked(_extractUpdate).extractAndUpdate; +const extract = mocked(_extractUpdate).extract; let config: RenovateConfig; beforeEach(() => { @@ -19,9 +19,10 @@ describe('workers/repository/process/index', () => { expect(res).toMatchSnapshot(); }); it('processes baseBranches', async () => { - extractAndUpdate.mockResolvedValue({} as never); + extract.mockResolvedValue({} as never); config.baseBranches = ['branch1', 'branch2']; const res = await processRepo(config); + await updateRepo(config, res.branches, res.branchList, res.packageFiles); expect(res).toMatchSnapshot(); }); }); diff --git a/lib/workers/repository/process/index.ts b/lib/workers/repository/process/index.ts index 84ecb782ba..6fefa0f999 100644 --- a/lib/workers/repository/process/index.ts +++ b/lib/workers/repository/process/index.ts @@ -1,13 +1,13 @@ import { logger } from '../../../logger'; import { mergeChildConfig, RenovateConfig } from '../../../config'; -import { extractAndUpdate, ExtractAndUpdateResult } from './extract-update'; +import { extract, ExtractResult, update, UpdateResult } from './extract-update'; import { platform } from '../../../platform'; -import { WriteUpdateResult } from './write'; import { BranchConfig } from '../../common'; +import { PackageFile } from '../../../manager/common'; export async function processRepo( config: RenovateConfig -): Promise<ExtractAndUpdateResult> { +): Promise<ExtractResult> { logger.debug('processRepo()'); /* eslint-disable no-param-reassign */ config.masterIssueChecks = {}; @@ -45,7 +45,6 @@ export async function processRepo( } if (config.baseBranches && config.baseBranches.length) { logger.debug({ baseBranches: config.baseBranches }, 'baseBranches'); - let res: WriteUpdateResult | undefined; let branches: BranchConfig[] = []; let branchList: string[] = []; for (const baseBranch of config.baseBranches) { @@ -56,13 +55,23 @@ export async function processRepo( baseBranchConfig.hasBaseBranches = true; } await platform.setBaseBranch(baseBranch); - const baseBranchRes = await extractAndUpdate(baseBranchConfig); - ({ res } = baseBranchRes); + const baseBranchRes = await extract(baseBranchConfig); branches = branches.concat(baseBranchRes.branches); branchList = branchList.concat(baseBranchRes.branchList); } - return { res, branches, branchList }; + return { branches, branchList }; } logger.debug('No baseBranches'); - return extractAndUpdate(config); + return extract(config); +} + +export async function updateRepo( + config: RenovateConfig, + branches: BranchConfig[], + branchList: string[], + packageFiles?: Record<string, PackageFile[]> +): Promise<UpdateResult> { + logger.debug('processRepo()'); + + return update(config, branches, branchList, packageFiles); } -- GitLab