feat: improve branch cache logic (#17848)

Co-authored-by: Rhys Arkins <rhys@arkins.net>
This commit is contained in:
RahulGautamSingh
2022-10-06 10:07:47 +05:30
committed by GitHub
parent 368f635f62
commit 72371cb778
14 changed files with 711 additions and 207 deletions
+13 -1
View File
@@ -31,6 +31,14 @@ export interface BranchCache {
*Whether this branch has automerge enabled
*/
automerge: boolean;
/**
* Name of base branch
*/
baseBranch: string;
/**
* The base branch's most recent commit SHA
*/
baseBranchSha: string | null;
/**
* Hash of the manager fingerprints and the update branch config
*/
@@ -39,10 +47,14 @@ export interface BranchCache {
* Branch name
*/
branchName: string;
/**
* Whether the update branch is behind base branh
*/
isBehindBase?: boolean;
/**
* Whether a person not listed in gitIgnoredAuthors updated the branch.
*/
isModified: boolean;
isModified?: boolean;
/**
* Parent commit of branch sha
*/
+251 -24
View File
@@ -1,7 +1,10 @@
import { mocked, partial } from '../../../test/util';
import { logger, mocked, partial } from '../../../test/util';
import * as _repositoryCache from '../cache/repository';
import type { BranchCache, RepoCacheData } from '../cache/repository/types';
import { getCachedBehindBaseResult } from './behind-base-branch-cache';
import {
getCachedBehindBaseResult,
setCachedBehindBaseResult,
} from './behind-base-branch-cache';
jest.mock('../cache/repository');
const repositoryCache = mocked(_repositoryCache);
@@ -16,36 +19,260 @@ describe('util/git/behind-base-branch-cache', () => {
describe('getCachedBehindBaseResult', () => {
it('returns null if cache is not populated', () => {
expect(getCachedBehindBaseResult('foo', '111')).toBeNull();
expect(
getCachedBehindBaseResult(
'branch',
'branch_sha',
'base_branch',
'base_branch_sha'
)
).toBeNull();
});
it('returns null if branch not found', () => {
expect(getCachedBehindBaseResult('foo', '111')).toBeNull();
});
it('returns null if cache is partially defined', () => {
const branchName = 'branchName';
const branchCache = partial<BranchCache>({
branchName,
isModified: false,
});
const repoCache: RepoCacheData = { branches: [branchCache] };
repoCache = {
branches: [
partial<BranchCache>({
branchName: 'not_branch',
baseBranchSha: 'base_branch_sha',
baseBranch: 'base_branch',
sha: 'branch_sha',
isBehindBase: true,
}),
],
};
repositoryCache.getCache.mockReturnValue(repoCache);
expect(getCachedBehindBaseResult(branchName, '111')).toBeNull();
expect(
getCachedBehindBaseResult(
'branch',
'branch_sha',
'base_branch',
'base_branch_sha'
)
).toBeNull();
});
it('returns true if target SHA has changed', () => {
repoCache.branches = [
{ branchName: 'foo', sha: 'aaa', parentSha: '222' } as BranchCache,
];
expect(getCachedBehindBaseResult('foo', '111')).toBeTrue();
it('returns null if base branch SHA is different', () => {
repoCache = {
branches: [
partial<BranchCache>({
branchName: 'branch',
baseBranchSha: 'not_base_branch_sha',
baseBranch: 'base_branch',
sha: 'branch_sha',
isBehindBase: true,
}),
],
};
repositoryCache.getCache.mockReturnValue(repoCache);
expect(
getCachedBehindBaseResult(
'branch',
'branch_sha',
'base_branch',
'base_branch_sha'
)
).toBeNull();
});
it('returns false if target SHA has not changed', () => {
repoCache.branches = [
{ branchName: 'foo', sha: 'aaa', parentSha: '111' } as BranchCache,
];
expect(getCachedBehindBaseResult('foo', '111')).toBeFalse();
it('returns null if branch sha is different', () => {
repoCache = {
branches: [
partial<BranchCache>({
branchName: 'branch',
baseBranchSha: 'base_branch_sha',
baseBranch: 'base_branch',
sha: 'not_branch_sha',
isBehindBase: true,
}),
],
};
repositoryCache.getCache.mockReturnValue(repoCache);
expect(
getCachedBehindBaseResult(
'branch',
'branch_sha',
'base_branch',
'base_branch_sha'
)
).toBeNull();
});
it('returns null if cached value is undefined', () => {
repoCache = {
branches: [
partial<BranchCache>({
branchName: 'branch',
baseBranchSha: 'base_branch_sha',
baseBranch: 'base_branch',
sha: 'not_branch_sha',
}),
],
};
repositoryCache.getCache.mockReturnValue(repoCache);
expect(
getCachedBehindBaseResult(
'branch',
'branch_sha',
'base_branch',
'base_branch_sha'
)
).toBeNull();
});
it('returns null if base branch SHA is null', () => {
repoCache = {
branches: [
partial<BranchCache>({
branchName: 'branch',
baseBranchSha: null,
baseBranch: 'base_branch',
sha: 'branch_sha',
isBehindBase: true,
}),
],
};
repositoryCache.getCache.mockReturnValue(repoCache);
expect(
getCachedBehindBaseResult(
'branch',
'branch_sha',
'base_branch',
'base_branch_sha'
)
).toBeNull();
});
it('returns null if branch SHA is null', () => {
repoCache = {
branches: [
partial<BranchCache>({
branchName: 'branch',
baseBranchSha: 'base_branch_sha',
baseBranch: 'base_branch',
sha: null,
isBehindBase: true,
}),
],
};
repositoryCache.getCache.mockReturnValue(repoCache);
expect(
getCachedBehindBaseResult(
'branch',
'branch_sha',
'base_branch',
'base_branch_sha'
)
).toBeNull();
});
it('returns cached value', () => {
repoCache = {
branches: [
partial<BranchCache>({
branchName: 'branch',
baseBranchSha: 'base_branch_sha',
baseBranch: 'base_branch',
sha: 'branch_sha',
isBehindBase: true,
}),
],
};
repositoryCache.getCache.mockReturnValue(repoCache);
expect(
getCachedBehindBaseResult(
'branch',
'branch_sha',
'base_branch',
'base_branch_sha'
)
).toBeTrue();
});
});
describe('setCachedBehindBasedResult', () => {
it('returns without updating when cache not populated', () => {
setCachedBehindBaseResult('foo', false);
expect(repoCache).toEqual({});
expect(logger.logger.debug).toHaveBeenCalledWith(
'setCachedBehindBaseResult(): Branch cache not present'
);
});
it('returns without updating when branch not found', () => {
setCachedBehindBaseResult('foo', false);
expect(repoCache).toEqual({});
expect(logger.logger.debug).toHaveBeenCalledWith(
'setCachedBehindBaseResult(): Branch cache not present'
);
});
it('updates cached value', () => {
repoCache = {
branches: [
partial<BranchCache>({
branchName: 'foo',
sha: '121',
isBehindBase: true,
}),
],
};
repositoryCache.getCache.mockReturnValue(repoCache);
setCachedBehindBaseResult('foo', false);
expect(repoCache).toEqual({
branches: [
{
branchName: 'foo',
sha: '121',
isBehindBase: false,
},
],
});
});
it('handles multiple branches', () => {
repoCache = {
branches: [
partial<BranchCache>({
branchName: 'foo-1',
sha: '111',
isBehindBase: true,
}),
partial<BranchCache>({
branchName: 'foo-2',
sha: 'aaa',
isBehindBase: false,
}),
partial<BranchCache>({
branchName: 'foo-3',
sha: '222',
isBehindBase: true,
}),
],
};
repositoryCache.getCache.mockReturnValue(repoCache);
setCachedBehindBaseResult('foo-1', false);
setCachedBehindBaseResult('foo-2', true);
setCachedBehindBaseResult('foo-3', false);
expect(repoCache).toEqual({
branches: [
{
branchName: 'foo-1',
sha: '111',
isBehindBase: false,
},
{
branchName: 'foo-2',
sha: 'aaa',
isBehindBase: true,
},
{
branchName: 'foo-3',
sha: '222',
isBehindBase: false,
},
],
});
});
});
});
+31 -8
View File
@@ -1,20 +1,43 @@
import { logger } from '../../logger';
import { getCache } from '../cache/repository';
// Compare cached parent Sha of a branch to the fetched base-branch sha to determine whether the branch is behind the base
// Since cache is updated after each run, this will be sufficient to determine whether a branch is behind its parent.
export function getCachedBehindBaseResult(
branchName: string,
currentBaseBranchSha: string
branchSha: string | null,
baseBranch: string,
baseBranchSha: string | null
): boolean | null {
const cache = getCache();
const { branches = [] } = cache;
const cachedBranch = branches?.find(
const branch = cache.branches?.find(
(branch) => branch.branchName === branchName
);
if (!cachedBranch?.parentSha) {
return null;
if (
branch &&
branch.sha === branchSha &&
branch.baseBranch === baseBranch &&
branch.baseBranchSha === baseBranchSha &&
branch.isBehindBase !== undefined
) {
return branch.isBehindBase;
}
return currentBaseBranchSha !== cachedBranch.parentSha;
return null;
}
export function setCachedBehindBaseResult(
branchName: string,
isBehindBase: boolean
): void {
const cache = getCache();
const branch = cache.branches?.find(
(branch) => branch.branchName === branchName
);
if (!branch) {
logger.debug(`setCachedBehindBaseResult(): Branch cache not present`);
return;
}
branch.isBehindBase = isBehindBase;
}
+18 -18
View File
@@ -1,15 +1,14 @@
import fs from 'fs-extra';
import Git from 'simple-git';
import tmp from 'tmp-promise';
import { mocked, partial } from '../../../test/util';
import { logger, mocked } from '../../../test/util';
import { GlobalConfig } from '../../config/global';
import {
CONFIG_VALIDATION,
INVALID_PATH,
} from '../../constants/error-messages';
import * as _repoCache from '../cache/repository';
import type { BranchCache } from '../cache/repository/types';
import { newlineRegex, regEx } from '../regex';
import * as _behindBaseCache from './behind-base-branch-cache';
import * as _conflictsCache from './conflicts-cache';
import * as _modifiedCache from './modified-cache';
import * as _parentShaCache from './parent-sha-cache';
@@ -18,11 +17,12 @@ import * as git from '.';
import { setNoVerify } from '.';
jest.mock('./conflicts-cache');
jest.mock('./behind-base-branch-cache');
jest.mock('./modified-cache');
jest.mock('./parent-sha-cache');
jest.mock('delay');
jest.mock('../cache/repository');
const repoCache = mocked(_repoCache);
const behindBaseCache = mocked(_behindBaseCache);
const conflictsCache = mocked(_conflictsCache);
const modifiedCache = mocked(_modifiedCache);
const parentShaCache = mocked(_parentShaCache);
@@ -117,6 +117,7 @@ describe('util/git/index', () => {
const local = Git(tmpDir.path);
await local.addConfig('commit.gpgsign', 'false');
parentShaCache.getCachedBranchParentShaResult.mockReturnValue(null);
behindBaseCache.getCachedBehindBaseResult.mockReturnValue(null);
});
afterEach(async () => {
@@ -247,28 +248,27 @@ describe('util/git/index', () => {
describe('isBranchBehindBase()', () => {
it('should return false if same SHA as master', async () => {
repoCache.getCache.mockReturnValueOnce({});
expect(
await git.isBranchBehindBase('renovate/future_branch')
await git.isBranchBehindBase('renovate/future_branch', defaultBranch)
).toBeFalse();
});
it('should return true if SHA different from master', async () => {
repoCache.getCache.mockReturnValueOnce({});
expect(await git.isBranchBehindBase('renovate/past_branch')).toBeTrue();
expect(
await git.isBranchBehindBase('renovate/past_branch', defaultBranch)
).toBeTrue();
});
it('should return result even if non-default and not under branchPrefix', async () => {
const parentSha = 'SHA';
const branchCache = partial<BranchCache>({
branchName: 'develop',
parentSha,
});
repoCache.getCache.mockReturnValueOnce({}).mockReturnValueOnce({
branches: [branchCache],
});
expect(await git.isBranchBehindBase('develop')).toBeTrue();
expect(await git.isBranchBehindBase('develop')).toBeTrue(); // cache
expect(await git.isBranchBehindBase('develop', defaultBranch)).toBeTrue();
});
it('returns cached value', async () => {
behindBaseCache.getCachedBehindBaseResult.mockReturnValue(true);
expect(await git.isBranchBehindBase('develop', defaultBranch)).toBeTrue();
expect(logger.logger.debug).toHaveBeenCalledWith(
'branch.isBehindBase(): using cached result "true"'
);
});
});
+37 -26
View File
@@ -493,6 +493,9 @@ export async function getBranchParentSha(
const branchSha = getBranchCommit(branchName);
let parentSha = getCachedBranchParentShaResult(branchName, branchSha);
if (parentSha !== null) {
logger.debug(
`branch.getBranchParentSha(): using cached result "${parentSha}"`
);
return parentSha;
}
@@ -576,14 +579,23 @@ export function getBranchList(): string[] {
return Object.keys(config.branchCommits);
}
export async function isBranchBehindBase(branchName: string): Promise<boolean> {
const { currentBranchSha } = config;
let isBehind = getCachedBehindBaseResult(branchName, currentBranchSha);
export async function isBranchBehindBase(
branchName: string,
baseBranch: string
): Promise<boolean> {
let isBehind = getCachedBehindBaseResult(
branchName,
getBranchCommit(branchName), // branch sha
baseBranch,
getBranchCommit(baseBranch) // base branch sha
);
if (isBehind !== null) {
logger.debug(`branch.isBehindBase(): using cached result "${isBehind}"`);
return isBehind;
}
logger.debug('branch.isBehindBase(): using git to calculate');
await syncGit();
try {
const { currentBranchSha, currentBranch } = config;
@@ -595,8 +607,8 @@ export async function isBranchBehindBase(branchName: string): Promise<boolean> {
]);
isBehind = !branches.all.map(localName).includes(branchName);
logger.debug(
{ isBehind, currentBranch, currentBranchSha },
`isBranchBehindBase=${isBehind}`
{ currentBranch, currentBranchSha },
`branch.isBehindBase(): ${isBehind}`
);
return isBehind;
} catch (err) /* istanbul ignore next */ {
@@ -610,26 +622,26 @@ export async function isBranchBehindBase(branchName: string): Promise<boolean> {
export async function isBranchModified(branchName: string): Promise<boolean> {
if (!branchExists(branchName)) {
logger.debug(
{ branchName },
'Branch does not exist - cannot check isModified'
);
logger.debug('branch.isModified(): no cache');
return false;
}
// First check local config
if (config.branchIsModified[branchName] !== undefined) {
return config.branchIsModified[branchName];
}
// Second check repoCache
// Second check repository cache
const isModified = getCachedModifiedResult(
branchName,
config.branchCommits[branchName]
getBranchCommit(branchName) // branch sha
);
if (isModified !== null) {
logger.debug('Using cached result for isBranchModified');
return (config.branchIsModified[branchName] = isModified);
logger.debug(`branch.isModified(): using cached result "${isModified}"`);
config.branchIsModified[branchName] = isModified;
return isModified;
}
logger.debug('branch.isModified(): using git to calculate');
await syncGit();
// Retrieve the author of the most recent commit
let lastAuthor: string | undefined;
@@ -659,21 +671,17 @@ export async function isBranchModified(branchName: string): Promise<boolean> {
config.ignoredAuthors.some((ignoredAuthor) => lastAuthor === ignoredAuthor)
) {
// author matches - branch has not been modified
logger.debug({ branchName }, 'Branch has not been modified');
logger.debug('branch.isModified() = false');
config.branchIsModified[branchName] = false;
setCachedModifiedResult(
branchName,
config.branchCommits[branchName],
false
);
setCachedModifiedResult(branchName, false);
return false;
}
logger.debug(
{ branchName, lastAuthor, gitAuthorEmail },
'Last commit author does not match git author email - branch has been modified'
'branch.isModified() = true'
);
config.branchIsModified[branchName] = true;
setCachedModifiedResult(branchName, config.branchCommits[branchName], true);
setCachedModifiedResult(branchName, true);
return true;
}
@@ -693,19 +701,21 @@ export async function isBranchConflicted(
return true;
}
const cachedResult = getCachedConflictResult(
const isConflicted = getCachedConflictResult(
baseBranch,
baseBranchSha,
branch,
branchSha
);
if (is.boolean(cachedResult)) {
if (is.boolean(isConflicted)) {
logger.debug(
`Using cached result ${cachedResult} for isBranchConflicted(${baseBranch}, ${branch})`
`branch.isConflicted(): using cached result "${isConflicted}"`
);
return cachedResult;
return isConflicted;
}
logger.debug('branch.isConflicted(): using git to calculate');
let result = false;
await syncGit();
await writeGitAuthor();
@@ -741,6 +751,7 @@ export async function isBranchConflicted(
}
setCachedConflictResult(baseBranch, baseBranchSha, branch, branchSha, result);
logger.debug(`branch.isConflicted(): ${result}`);
return result;
}
+32 -61
View File
@@ -19,28 +19,35 @@ describe('util/git/modified-cache', () => {
describe('getCachedModifiedResult', () => {
it('returns null if cache is not populated', () => {
expect(getCachedModifiedResult('foo', '111')).toBeNull();
expect(getCachedModifiedResult('foo', 'aaa')).toBeNull();
});
it('returns null if target key not found', () => {
expect(getCachedModifiedResult('foo', '111')).toBeNull();
it('returns null if branch not found', () => {
repoCache.branches = [
partial<BranchCache>({ branchName: 'not_foo', sha: 'aaa' }),
];
expect(getCachedModifiedResult('foo', 'aaa')).toBeNull();
});
it('returns null if target SHA has changed', () => {
it('returns null if branch SHA has changed', () => {
repoCache.branches = [
partial<BranchCache>({ branchName: 'foo', sha: 'aaa' }),
];
expect(getCachedModifiedResult('foo', '111')).toBeNull();
expect(getCachedModifiedResult('foo', 'not_aaa')).toBeNull();
});
it('returns null if cache is partially defined', () => {
it('returns null if cached value is undefined', () => {
repoCache.branches = [
partial<BranchCache>({
branchName: 'foo',
sha: '111',
}),
partial<BranchCache>({ branchName: 'foo', sha: 'aaa' }),
];
expect(getCachedModifiedResult('foo', '111')).toBeNull();
expect(getCachedModifiedResult('foo', 'aaa')).toBeNull();
});
it('returns null if branch sha is null', () => {
repoCache.branches = [
partial<BranchCache>({ branchName: 'foo', sha: 'aaa' }),
];
expect(getCachedModifiedResult('foo', null)).toBeNull();
});
it('returns cached value', () => {
@@ -48,64 +55,28 @@ describe('util/git/modified-cache', () => {
partial<BranchCache>({
branchName: 'foo',
sha: '111',
isModified: false,
isModified: true,
}),
];
expect(getCachedModifiedResult('foo', '111')).toBeFalse();
expect(getCachedModifiedResult('foo', '111')).toBeTrue();
});
});
describe('setCachedModifiedResult', () => {
it('does not create new branch cache for when cache is empty', () => {
setCachedModifiedResult('foo', '111', false);
it('returns without updating when cache not populated', () => {
setCachedModifiedResult('foo', false);
expect(repoCache).toEqual({});
expect(logger.logger.debug).toHaveBeenCalledWith(
'Branch cache not present for foo'
'setCachedModifiedResult(): Branch cache not present'
);
});
it('replaces value when SHA has changed', () => {
repoCache = {
branches: [
partial<BranchCache>({
branchName: 'foo',
sha: '121',
isModified: true,
}),
],
};
repositoryCache.getCache.mockReturnValue(repoCache);
setCachedModifiedResult('foo', '131', false);
expect(repoCache).toEqual({
branches: [
{
branchName: 'foo',
sha: '131',
isModified: false,
},
],
});
});
it('adds SHA when it is not found', () => {
repoCache = {
branches: [
partial<BranchCache>({
branchName: 'foo',
}),
],
};
repositoryCache.getCache.mockReturnValue(repoCache);
setCachedModifiedResult('foo', '131', false);
expect(repoCache).toEqual({
branches: [
{
branchName: 'foo',
sha: '131',
isModified: false,
},
],
});
it('returns without updating when branch not found', () => {
setCachedModifiedResult('foo', false);
expect(repoCache).toEqual({});
expect(logger.logger.debug).toHaveBeenCalledWith(
'setCachedModifiedResult(): Branch cache not present'
);
});
it('handles multiple branches', () => {
@@ -129,9 +100,9 @@ describe('util/git/modified-cache', () => {
],
};
repositoryCache.getCache.mockReturnValue(repoCache);
setCachedModifiedResult('foo-1', '111', false);
setCachedModifiedResult('foo-2', 'aaa', true);
setCachedModifiedResult('foo-3', '222', false);
setCachedModifiedResult('foo-1', false);
setCachedModifiedResult('foo-2', true);
setCachedModifiedResult('foo-3', false);
expect(repoCache).toEqual({
branches: [
{
+6 -9
View File
@@ -3,10 +3,12 @@ import { getCache } from '../cache/repository';
export function getCachedModifiedResult(
branchName: string,
branchSha: string
branchSha: string | null
): boolean | null {
const { branches } = getCache();
const branch = branches?.find((branch) => branch.branchName === branchName);
const cache = getCache();
const branch = cache.branches?.find(
(branch) => branch.branchName === branchName
);
if (branch?.sha === branchSha && branch.isModified !== undefined) {
return branch.isModified;
@@ -17,7 +19,6 @@ export function getCachedModifiedResult(
export function setCachedModifiedResult(
branchName: string,
branchSha: string,
isModified: boolean
): void {
const cache = getCache();
@@ -26,13 +27,9 @@ export function setCachedModifiedResult(
);
if (!branch) {
logger.debug(`Branch cache not present for ${branchName}`);
logger.debug(`setCachedModifiedResult(): Branch cache not present`);
return;
}
if (!branch.sha || branch.sha !== branchSha) {
branch.sha = branchSha;
}
branch.isModified = isModified;
}
+68
View File
@@ -0,0 +1,68 @@
import { git, logger, mocked, partial } from '../../../test/util';
import * as _repositoryCache from '../cache/repository';
import type { BranchCache, RepoCacheData } from '../cache/repository/types';
import { setBranchNewCommit } from './set-branch-commit';
jest.mock('../cache/repository');
jest.mock('.');
const repositoryCache = mocked(_repositoryCache);
describe('util/git/set-branch-commit', () => {
let repoCache: RepoCacheData = {};
beforeEach(() => {
repoCache = {};
repositoryCache.getCache.mockReturnValue(repoCache);
});
describe('setBranchCommit', () => {
it('sets new branch in cache if it doesn not exist', () => {
git.getBranchCommit.mockReturnValueOnce('base_SHA');
setBranchNewCommit('branch_name', 'base_branch', 'SHA');
expect(logger.logger.debug).toHaveBeenCalledWith(
'setBranchCommit(): Branch cache not present'
);
expect(repoCache.branches).toEqual([
{
branchName: 'branch_name',
baseBranch: 'base_branch',
sha: 'SHA',
baseBranchSha: 'base_SHA',
isBehindBase: false,
isModified: false,
parentSha: 'base_SHA',
},
]);
});
it('sets new values in branch when old state exists', () => {
repoCache = {
branches: [
partial<BranchCache>({
branchName: 'branch_name',
baseBranch: 'base_branch',
sha: 'SHA',
baseBranchSha: 'base_SHA',
isBehindBase: false,
isModified: false,
parentSha: 'base_SHA',
}),
],
};
git.getBranchCommit.mockReturnValueOnce('base_SHA');
repositoryCache.getCache.mockReturnValue(repoCache);
setBranchNewCommit('branch_name', 'base_branch', 'SHA');
expect(repoCache.branches).toEqual([
{
branchName: 'branch_name',
baseBranch: 'base_branch',
sha: 'SHA',
baseBranchSha: 'base_SHA',
isBehindBase: false,
isModified: false,
parentSha: 'base_SHA',
},
]);
});
});
});
+36
View File
@@ -0,0 +1,36 @@
import { logger } from '../../logger';
import { getCache } from '../cache/repository';
import type { BranchCache } from '../cache/repository/types';
import { getBranchCommit } from '.';
/**
* Called when a new commit is added to branch
*
* ie. when branch is created/updated
*/
export function setBranchNewCommit(
branchName: string,
baseBranch: string,
commitSha: string
): void {
logger.debug('setBranchCommit()');
const cache = getCache();
cache.branches ??= [];
let branch = cache.branches.find((br) => br.branchName === branchName);
if (!branch) {
logger.debug(`setBranchCommit(): Branch cache not present`); // should never be called
branch = {
branchName,
baseBranch,
} as BranchCache;
cache.branches.push(branch);
}
const baseBranchSha = getBranchCommit(baseBranch);
branch.sha = commitSha;
branch.baseBranchSha = baseBranchSha;
branch.isBehindBase = false;
branch.isModified = false;
branch.parentSha = baseBranchSha;
}
+17 -13
View File
@@ -10,6 +10,7 @@ import type {
import {
getBranchCommit,
getBranchParentSha,
isBranchBehindBase,
isBranchModified,
} from '../../util/git';
import type { BranchConfig, BranchUpgradeConfig } from '../types';
@@ -47,40 +48,43 @@ function generateBranchUpgradeCache(
async function generateBranchCache(
branch: BranchConfig
): Promise<BranchCache | null> {
const { branchName } = branch;
const { baseBranch, branchName } = branch;
try {
const sha = getBranchCommit(branchName) ?? null;
// TODO: fix types (#7154)
const baseBranchSha = getBranchCommit(baseBranch!)!;
let prNo = null;
let parentSha = null;
let isModified = false;
let isBehindBase = false;
if (sha) {
parentSha = await getBranchParentSha(branchName);
const branchPr = await platform.getBranchPr(branchName);
if (branchPr) {
prNo = branchPr.number;
}
isModified = await isBranchModified(branchName);
// TODO: fix types (#7154)
isBehindBase = await isBranchBehindBase(branchName, baseBranch!);
}
const automerge = !!branch.automerge;
let isModified = false;
if (sha) {
try {
isModified = await isBranchModified(branchName);
} catch (err) /* istanbul ignore next */ {
// Do nothing
}
}
const upgrades: BranchUpgradeCache[] = branch.upgrades
? branch.upgrades.map(generateBranchUpgradeCache)
: [];
const branchFingerprint = branch.branchFingerprint;
return {
automerge,
baseBranchSha,
// TODO: fix types (#7154)
baseBranch: baseBranch!,
branchFingerprint,
branchName,
sha,
isBehindBase,
isModified,
parentSha,
prNo,
automerge,
isModified,
sha,
upgrades,
branchFingerprint,
};
} catch (error) {
const err = error.err || error; // external host error nests err
@@ -29,10 +29,10 @@ export async function rebaseOnboardingBranch(
const configFile = defaultConfigFile(config);
const existingContents = await getFile(configFile, config.onboardingBranch);
const contents = await getOnboardingConfigContents(config, configFile);
// TODO #7154
// TODO: fix types (#7154)
if (
contents === existingContents &&
!(await isBranchBehindBase(config.onboardingBranch!))
!(await isBranchBehindBase(config.onboardingBranch!, config.defaultBranch!))
) {
logger.debug('Onboarding branch is up to date');
return null;
+129 -21
View File
@@ -20,7 +20,11 @@ import { Limit, isLimitReached } from '../../global/limits';
import { BranchConfig, BranchResult, BranchUpgradeConfig } from '../../types';
import * as _branchWorker from '../update/branch';
import * as _limits from './limits';
import { canSkipBranchUpdateCheck, writeUpdates } from './write';
import {
canSkipBranchUpdateCheck,
syncBranchState,
writeUpdates,
} from './write';
jest.mock('../../../util/git');
jest.mock('../../../util/cache/repository');
@@ -39,6 +43,7 @@ let config: RenovateConfig;
beforeEach(() => {
jest.resetAllMocks();
config = getConfig();
repoCache.getCache.mockReturnValue({});
});
describe('workers/repository/process/write', () => {
@@ -57,7 +62,6 @@ describe('workers/repository/process/write', () => {
{ branchName: 'test_branch', manager: 'npm', upgrades: [] },
{ branchName: 'test_branch', manager: 'npm', upgrades: [] },
]);
repoCache.getCache.mockReturnValue({});
git.branchExists.mockReturnValue(true);
branchWorker.processBranch.mockResolvedValueOnce({
branchExists: true,
@@ -92,8 +96,7 @@ describe('workers/repository/process/write', () => {
branchExists: true,
result: BranchResult.PrCreated,
});
git.branchExists.mockReturnValueOnce(false);
git.branchExists.mockReturnValueOnce(true);
git.branchExists.mockReturnValueOnce(false).mockReturnValueOnce(true);
limits.getBranchesRemaining.mockResolvedValueOnce(1);
expect(isLimitReached(Limit.Branches)).toBeFalse();
GlobalConfig.set({ dryRun: 'full' });
@@ -135,8 +138,6 @@ describe('workers/repository/process/write', () => {
branchExists: true,
result: BranchResult.NoWork,
});
git.branchExists.mockReturnValue(true);
config.repositoryCache = 'enabled';
expect(await writeUpdates(config, branches)).toBe('done');
});
@@ -165,7 +166,6 @@ describe('workers/repository/process/write', () => {
result: BranchResult.Done,
commitSha: 'some-value',
});
git.branchExists.mockReturnValue(true);
const branchManagersFingerprint = hasha(
[
...new Set(
@@ -179,40 +179,58 @@ describe('workers/repository/process/write', () => {
JSON.stringify(branches[0]),
branchManagersFingerprint,
]);
config.repositoryCache = 'enabled';
expect(await writeUpdates(config, branches)).toBe('done');
expect(branches[0].branchFingerprint).toBe(fingerprint);
});
it('shows debug log when the cache is enabled, but branch cache not found', async () => {
it('caches same fingerprint when no commit is made', async () => {
const branches = partial<BranchConfig[]>([
{
branchName: 'new/some-branch',
baseBranch: 'base_branch',
manager: 'npm',
upgrades: [
{
manager: 'npm',
manager: 'unknown-manager',
} as BranchUpgradeConfig,
],
},
]);
repoCache.getCache.mockReturnValueOnce({});
const branchManagersFingerprint = hasha(
[
...new Set(
branches[0].upgrades
.map((upgrade) => hashMap.get(upgrade.manager) ?? upgrade.manager)
.filter(is.string)
),
].sort()
);
const fingerprint = hasha([
JSON.stringify(branches[0]),
branchManagersFingerprint,
]);
repoCache.getCache.mockReturnValueOnce({
branches: [
{
branchName: 'new/some-branch',
baseBranch: 'base_branch',
branchFingerprint: fingerprint,
} as BranchCache,
],
});
branchWorker.processBranch.mockResolvedValueOnce({
branchExists: true,
result: BranchResult.NoWork,
result: BranchResult.Done,
});
git.branchExists.mockReturnValueOnce(true);
config.repositoryCache = 'enabled';
await writeUpdates(config, branches);
expect(logger.logger.debug).toHaveBeenCalledWith(
'No branch cache found for new/some-branch'
);
expect(await writeUpdates(config, branches)).toBe('done');
expect(branches[0].branchFingerprint).toBe(fingerprint);
});
it('adds branch to cache when cache is not enabled', async () => {
it('creates new branchCache when cache is not enabled', async () => {
const branches = partial<BranchConfig[]>([
{
branchName: 'new/some-branch',
baseBranch: 'base_branch',
manager: 'npm',
upgrades: [
{
@@ -227,7 +245,9 @@ describe('workers/repository/process/write', () => {
branchExists: true,
result: BranchResult.NoWork,
});
git.getBranchCommit.mockReturnValue('101');
git.getBranchCommit
.mockReturnValueOnce('sha')
.mockReturnValueOnce('base_sha');
git.branchExists.mockReturnValueOnce(true);
await writeUpdates(config, branches);
expect(logger.logger.debug).not.toHaveBeenCalledWith(
@@ -237,7 +257,9 @@ describe('workers/repository/process/write', () => {
branches: [
{
branchName: 'new/some-branch',
sha: '101',
baseBranch: 'base_branch',
baseBranchSha: 'base_sha',
sha: 'sha',
},
],
});
@@ -273,4 +295,90 @@ describe('workers/repository/process/write', () => {
expect(canSkipBranchUpdateCheck(branchCache, '222')).toBe(true);
});
});
describe('syncBranchState()', () => {
it('creates minimal branch state when cache is not populated', () => {
const repoCacheObj = {} as RepoCacheData;
repoCache.getCache.mockReturnValue(repoCacheObj);
git.getBranchCommit.mockReturnValueOnce('sha');
git.getBranchCommit.mockReturnValueOnce('base_sha');
expect(syncBranchState('branch_name', 'base_branch')).toEqual({
branchName: 'branch_name',
sha: 'sha',
baseBranch: 'base_branch',
baseBranchSha: 'base_sha',
});
});
it('when base branch name is different updates it and invalidates isModified value', () => {
const repoCacheObj = {
branches: [
partial<BranchCache>({
branchName: 'branch_name',
sha: 'sha',
baseBranch: 'base_branch',
baseBranchSha: 'base_sha',
isModified: true,
}),
],
};
repoCache.getCache.mockReturnValue(repoCacheObj);
git.getBranchCommit.mockReturnValueOnce('sha');
git.getBranchCommit.mockReturnValueOnce('base_sha');
expect(syncBranchState('branch_name', 'new_base_branch')).toEqual({
branchName: 'branch_name',
sha: 'sha',
baseBranch: 'new_base_branch',
baseBranchSha: 'base_sha',
});
});
it('when base branch sha is different updates it and invalidates related values', () => {
const repoCacheObj = {
branches: [
partial<BranchCache>({
branchName: 'branch_name',
sha: 'sha',
baseBranch: 'base_branch',
baseBranchSha: 'base_sha',
isBehindBase: true,
}),
],
};
repoCache.getCache.mockReturnValue(repoCacheObj);
git.getBranchCommit.mockReturnValueOnce('sha');
git.getBranchCommit.mockReturnValueOnce('new_base_sha');
expect(syncBranchState('branch_name', 'base_branch')).toEqual({
branchName: 'branch_name',
sha: 'sha',
baseBranch: 'base_branch',
baseBranchSha: 'new_base_sha',
});
});
it('when branch sha is different updates it and invalidates related values', () => {
const repoCacheObj = {
branches: [
partial<BranchCache>({
branchName: 'branch_name',
sha: 'sha',
baseBranch: 'base_branch',
baseBranchSha: 'base_sha',
isBehindBase: true,
isModified: true,
branchFingerprint: '123',
}),
],
};
repoCache.getCache.mockReturnValue(repoCacheObj);
git.getBranchCommit.mockReturnValueOnce('new_sha');
git.getBranchCommit.mockReturnValueOnce('base_sha');
expect(syncBranchState('branch_name', 'base_branch')).toEqual({
branchName: 'branch_name',
sha: 'new_sha',
baseBranch: 'base_branch',
baseBranchSha: 'base_sha',
});
});
});
});
+69 -23
View File
@@ -7,6 +7,7 @@ import { hashMap } from '../../../modules/manager';
import { getCache } from '../../../util/cache/repository';
import type { BranchCache } from '../../../util/cache/repository/types';
import { branchExists, getBranchCommit } from '../../../util/git';
import { setBranchNewCommit } from '../../../util/git/set-branch-commit';
import { Limit, incLimitedValue, setMaxLimit } from '../../global/limits';
import { BranchConfig, BranchResult } from '../../types';
import { processBranch } from '../update/branch';
@@ -15,22 +16,79 @@ import { getBranchesRemaining, getPrsRemaining } from './limits';
export type WriteUpdateResult = 'done' | 'automerged';
export function canSkipBranchUpdateCheck(
branchCache: BranchCache,
branchState: BranchCache,
branchFingerprint: string
): boolean {
if (!branchCache.branchFingerprint) {
if (!branchState.branchFingerprint) {
logger.trace('branch.isUpToDate(): no fingerprint');
return false;
}
if (branchFingerprint !== branchCache.branchFingerprint) {
logger.debug('Branch fingerprint has changed, full check required');
if (branchFingerprint !== branchState.branchFingerprint) {
logger.debug('branch.isUpToDate(): needs recalculation');
return false;
}
logger.debug('Branch fingerprint is unchanged, updates check can be skipped');
logger.debug('branch.isUpToDate(): using cached result "true"');
return true;
}
export function syncBranchState(
branchName: string,
baseBranch: string
): BranchCache {
logger.debug('syncBranchState()');
const branchSha = getBranchCommit(branchName)!;
const baseBranchSha = getBranchCommit(baseBranch)!;
const cache = getCache();
cache.branches ??= [];
const { branches: cachedBranches } = cache;
let branchState = cachedBranches.find((br) => br.branchName === branchName);
if (!branchState) {
logger.debug(
'syncBranchState(): Branch cache not found, creating minimal branchState'
);
// create a minimal branch state
branchState = {
branchName,
sha: branchSha,
baseBranch,
baseBranchSha,
} as BranchCache;
cachedBranches.push(branchState);
}
// if base branch name has changed invalidate cached isModified state
if (baseBranch !== branchState.baseBranch) {
logger.debug('syncBranchState(): update baseBranch name');
branchState.baseBranch = baseBranch!;
delete branchState.isModified;
}
// if base branch sha has changed invalidate cached isBehindBase state
if (baseBranchSha !== branchState.baseBranchSha) {
logger.debug('syncBranchState(): update baseBranchSha');
delete branchState.isBehindBase;
// update cached branchSha
branchState.baseBranchSha = baseBranchSha;
}
// if branch sha has changed invalidate all cached states
if (branchSha !== branchState.sha) {
logger.debug('syncBranchState(): update branchSha');
delete branchState.isBehindBase;
delete branchState.isModified;
delete branchState.branchFingerprint;
// update cached branchSha
branchState.sha = branchSha;
}
return branchState;
}
export async function writeUpdates(
config: RenovateConfig,
allBranches: BranchConfig[]
@@ -44,9 +102,6 @@ export async function writeUpdates(
.sort()
.join(', ')}`
);
const cache = getCache();
cache.branches ??= [];
const { branches: cachedBranches } = cache;
const prsRemaining = await getPrsRemaining(config, branches);
logger.debug({ prsRemaining }, 'Calculated maximum PRs remaining this run');
setMaxLimit(Limit.PullRequests, prsRemaining);
@@ -60,27 +115,14 @@ export async function writeUpdates(
for (const branch of branches) {
const { baseBranch, branchName } = branch;
const branchSha = getBranchCommit(branchName)!;
const meta: Record<string, string> = { branch: branchName };
if (config.baseBranches?.length && baseBranch) {
meta['baseBranch'] = baseBranch;
}
addMeta(meta);
const branchExisted = branchExists(branchName);
let branchState = cachedBranches.find((br) => br.branchName === branchName);
if (!branchState) {
if (branchExisted && config.repositoryCache === 'enabled') {
logger.debug(`No branch cache found for ${branchName}`);
}
// create a minimal branch state
branchState = {
branchName,
sha: branchSha,
} as BranchCache;
cachedBranches.push(branchState);
}
// TODO: base branch name cannot be undefined - fix optional types (#7154)
const branchState = syncBranchState(branchName, baseBranch!);
const branchManagersFingerprint = hasha(
[
@@ -108,6 +150,10 @@ export async function writeUpdates(
? branchFingerprint
: branchState.branchFingerprint;
if (res?.commitSha) {
// TODO: base branch name cannot be undefined - fix optional types (#7154)
setBranchNewCommit(branchName, baseBranch!, res.commitSha);
}
if (
branch.result === BranchResult.Automerged &&
branch.automergeType !== 'pr-comment'
@@ -62,7 +62,8 @@ export async function shouldReuseExistingBranch(
(config.rebaseWhen === 'auto' &&
(config.automerge || (await platform.getRepoForceRebase())))
) {
if (await isBranchBehindBase(branchName)) {
// TODO: fix types (#7154)
if (await isBranchBehindBase(branchName, baseBranch!)) {
logger.debug(`Branch is behind base branch and needs rebasing`);
// We can rebase the branch only if no PR or PR can be rebased
if (await isBranchModified(branchName)) {