Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add repoCache-check before cloning #16533

Merged
merged 22 commits into from
Jul 15, 2022
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
c25f24a
prototype
RahulGautamSingh Jul 11, 2022
261789c
move checkout further down
RahulGautamSingh Jul 11, 2022
644c77e
add get/set modified cache functions
RahulGautamSingh Jul 11, 2022
9eb4cbb
Merge branch 'main' into experiment/replace_syncGit
RahulGautamSingh Jul 11, 2022
f15ec04
revert unrelated changes
RahulGautamSingh Jul 11, 2022
0b3f331
Merge branch 'experiment/replace_syncGit' of https://github.com/Rahul…
RahulGautamSingh Jul 11, 2022
73422b2
fix new line
RahulGautamSingh Jul 11, 2022
981d2db
revert changes to isBranchConflicted()
RahulGautamSingh Jul 11, 2022
de4e92b
use mocked modified-cache in test-files
RahulGautamSingh Jul 11, 2022
fe6c091
add test for full coverage
RahulGautamSingh Jul 11, 2022
a368e90
rename vars
RahulGautamSingh Jul 12, 2022
12fcdd3
move tests in one describe-block
RahulGautamSingh Jul 12, 2022
8b3732a
Merge branch 'main' into experiment/replace_syncGit
RahulGautamSingh Jul 12, 2022
d87641a
Update lib/util/git/index.spec.ts
RahulGautamSingh Jul 13, 2022
4161930
refactor: remove redundant check
RahulGautamSingh Jul 13, 2022
213bc9e
fix: check branchExisst before checking cache
RahulGautamSingh Jul 13, 2022
4299754
Merge branch 'main' into experiment/replace_syncGit
RahulGautamSingh Jul 13, 2022
4f2ae91
remove .only from test
RahulGautamSingh Jul 13, 2022
15deefd
Merge branch 'experiment/replace_syncGit' of https://github.com/Rahul…
RahulGautamSingh Jul 13, 2022
b11886d
Update lib/util/git/index.ts
RahulGautamSingh Jul 14, 2022
65cec8a
Update lib/util/git/modified-cache.ts
RahulGautamSingh Jul 15, 2022
d7a2656
Merge branch 'main' into experiment/replace_syncGit
rarkins Jul 15, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions lib/util/git/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@ import {
} from '../../constants/error-messages';
import { newlineRegex, regEx } from '../regex';
import * as _conflictsCache from './conflicts-cache';
import * as _modifiedCache from './modified-cache';
import type { FileChange } from './types';
import * as git from '.';
import { setNoVerify } from '.';

jest.mock('./conflicts-cache');
jest.mock('./modified-cache');
jest.mock('delay');
const conflictsCache = mocked(_conflictsCache);
const modifiedCache = mocked(_modifiedCache);

// Class is no longer exported
const SimpleGit = Git().constructor as { prototype: ReturnType<typeof Git> };
Expand Down Expand Up @@ -250,6 +253,10 @@ describe('util/git/index', () => {
});

describe('isBranchModified()', () => {
beforeEach(() => {
modifiedCache.getCachedModifiedResult.mockReturnValue(null);
});

it('should return false when branch is not found', async () => {
expect(await git.isBranchModified('renovate/not_found')).toBeFalse();
});
Expand All @@ -269,6 +276,11 @@ describe('util/git/index', () => {
it('should return true when custom author is unknown', async () => {
expect(await git.isBranchModified('renovate/custom_author')).toBeTrue();
});

it('should return value stored in modifiedCacheResult', async () => {
modifiedCache.getCachedModifiedResult.mockReturnValue(true);
expect(await git.isBranchModified('renovate/future_branch')).toBeTrue();
});
});

describe('getBranchCommit(branchName)', () => {
Expand Down
29 changes: 24 additions & 5 deletions lib/util/git/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ import {
setCachedConflictResult,
} from './conflicts-cache';
import { checkForPlatformFailure, handleCommitError } from './error';
import {
getCachedModifiedResult,
setCachedModifiedResult,
} from './modified-cache';
import { configSigningKey, writePrivateKey } from './private-key';
import type {
CommitFilesConfig,
Expand Down Expand Up @@ -564,18 +568,27 @@ export async function isBranchStale(branchName: string): Promise<boolean> {
}

export async function isBranchModified(branchName: string): Promise<boolean> {
await syncGit();
// First check cache
if (config.branchIsModified[branchName] !== undefined) {
return config.branchIsModified[branchName];
}
if (!branchExists(branchName)) {
logger.debug(
{ branchName },
'Branch does not exist - cannot check isModified'
);
return false;
}
// First check local config
if (config.branchIsModified[branchName] !== undefined) {
return config.branchIsModified[branchName];
}
// Second check repoCache
const isModified = getCachedModifiedResult(
branchName,
config.branchCommits[branchName]
);
if (isModified !== null) {
return (config.branchIsModified[branchName] = isModified);
}

await syncGit();
// Retrieve the author of the most recent commit
let lastAuthor: string | undefined;
try {
Expand Down Expand Up @@ -606,13 +619,19 @@ export async function isBranchModified(branchName: string): Promise<boolean> {
// author matches - branch has not been modified
logger.debug({ branchName }, 'Branch has not been modified');
config.branchIsModified[branchName] = false;
setCachedModifiedResult(
branchName,
config.branchCommits[branchName],
false
);
return false;
}
logger.debug(
{ branchName, lastAuthor, gitAuthorEmail },
'Last commit author does not match git author email - branch has been modified'
);
config.branchIsModified[branchName] = true;
setCachedModifiedResult(branchName, config.branchCommits[branchName], true);
return true;
}

Expand Down
87 changes: 87 additions & 0 deletions lib/util/git/modified-cache.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { mocked } from '../../../test/util';
import * as _repositoryCache from '../cache/repository';
import type { BranchCache, RepoCacheData } from '../cache/repository/types';
import {
getCachedModifiedResult,
setCachedModifiedResult,
} from './modified-cache';

jest.mock('../cache/repository');
const repositoryCache = mocked(_repositoryCache);

describe('util/git/modified-cache', () => {
let repoCache: RepoCacheData = {};

beforeEach(() => {
repoCache = {};
repositoryCache.getCache.mockReturnValue(repoCache);
});

describe('getCachedModifiedResult', () => {
it('returns null if cache is not populated', () => {
expect(getCachedModifiedResult('foo', '111')).toBeNull();
});

it('returns null if target key not found', () => {
expect(getCachedModifiedResult('foo', '111')).toBeNull();
});

it('returns null if target SHA has changed', () => {
repoCache.branches = [{ branchName: 'foo', sha: 'aaa' } as BranchCache];
expect(getCachedModifiedResult('foo', '111')).toBeNull();
});

it('returns true', () => {
repoCache.branches = [
{ branchName: 'foo', sha: '111', isModified: true } as BranchCache,
];
expect(getCachedModifiedResult('foo', '111')).toBeTrue();
});

it('returns false', () => {
repoCache.branches = [
{ branchName: 'foo', sha: '111', isModified: false } as BranchCache,
];
expect(getCachedModifiedResult('foo', '111')).toBeFalse();
});
});

describe('setCachedModifiedResult', () => {
it('sets value for unpopulated cache', () => {
setCachedModifiedResult('foo', '111', false);
expect(repoCache).toEqual({
branches: [{ branchName: 'foo', sha: '111', isModified: false }],
});
});

it('replaces value when SHA has changed', () => {
setCachedModifiedResult('foo', '111', false);
setCachedModifiedResult('foo', '121', false);
setCachedModifiedResult('foo', '131', false);
expect(repoCache).toEqual({
branches: [{ branchName: 'foo', sha: '131', isModified: false }],
});
});

it('replaces value when both value and SHA have changed', () => {
setCachedModifiedResult('foo', '111', false);
setCachedModifiedResult('foo', 'aaa', true);
expect(repoCache).toEqual({
branches: [{ branchName: 'foo', sha: 'aaa', isModified: true }],
});
});

it('handles multiple branches', () => {
setCachedModifiedResult('foo-1', '111', false);
setCachedModifiedResult('foo-2', 'aaa', true);
setCachedModifiedResult('foo-3', '222', false);
expect(repoCache).toEqual({
branches: [
{ branchName: 'foo-1', sha: '111', isModified: false },
{ branchName: 'foo-2', sha: 'aaa', isModified: true },
{ branchName: 'foo-3', sha: '222', isModified: false },
],
});
});
});
});
41 changes: 41 additions & 0 deletions lib/util/git/modified-cache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import is from '@sindresorhus/is';
import { getCache } from '../cache/repository';
import type { BranchCache } from '../cache/repository/types';

export function getCachedModifiedResult(
branchName: string,
branchSha: string
): boolean | null {
const { branches } = getCache();
const branch = branches?.find((branch) => branch.branchName === branchName);

if (branch?.sha !== branchSha) {
return null;
}

return branch.isModified;
}

export function setCachedModifiedResult(
branchName: string,
branchSha: string,
isModified: boolean
): void {
const cache = getCache();
cache.branches ??= [];
const { branches } = cache;
const branch =
branches?.find((branch) => branch.branchName === branchName) ??
({ branchName: branchName } as BranchCache);
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved

// if branch not present add it to cache
if (is.undefined(branch.sha)) {
branches.push(branch);
}

if (branch.sha !== branchSha) {
branch.sha = branchSha;
}

branch.isModified = isModified;
}