Skip to content

Commit

Permalink
refactor(scm): use scm for getFileList, checkoutBranch (#22006)
Browse files Browse the repository at this point in the history
  • Loading branch information
rarkins committed May 7, 2023
1 parent 1cd405b commit 9276a54
Show file tree
Hide file tree
Showing 30 changed files with 130 additions and 114 deletions.
5 changes: 3 additions & 2 deletions lib/modules/manager/gradle/artifacts.spec.ts
Expand Up @@ -12,6 +12,7 @@ import {
logger,
mockedFunction,
partial,
scm,
} from '../../../../test/util';
import { GlobalConfig } from '../../../config/global';
import type { RepoGlobalConfig } from '../../../config/types';
Expand Down Expand Up @@ -61,7 +62,7 @@ describe('modules/manager/gradle/artifacts', () => {
});

fs.findUpLocal.mockResolvedValue('gradlew');
git.getFileList.mockResolvedValue([
scm.getFileList.mockResolvedValue([
'gradlew',
'build.gradle',
'gradle.lockfile',
Expand Down Expand Up @@ -92,7 +93,7 @@ describe('modules/manager/gradle/artifacts', () => {

it('aborts if no lockfile is found', async () => {
const execSnapshots = mockExecAll();
git.getFileList.mockResolvedValue(['build.gradle', 'settings.gradle']);
scm.getFileList.mockResolvedValue(['build.gradle', 'settings.gradle']);

expect(
await updateArtifacts({
Expand Down
5 changes: 3 additions & 2 deletions lib/modules/manager/gradle/artifacts.ts
Expand Up @@ -6,8 +6,9 @@ import { logger } from '../../../logger';
import { exec } from '../../../util/exec';
import type { ExecOptions } from '../../../util/exec/types';
import { findUpLocal, readLocalFile, writeLocalFile } from '../../../util/fs';
import { getFileList, getFiles, getRepoStatus } from '../../../util/git';
import { getFiles, getRepoStatus } from '../../../util/git';
import { regEx } from '../../../util/regex';
import { scm } from '../../platform/scm';
import {
extraEnv,
extractGradleVersion,
Expand Down Expand Up @@ -94,7 +95,7 @@ export async function updateArtifacts({
}: UpdateArtifact): Promise<UpdateArtifactsResult[] | null> {
logger.debug(`gradle.updateArtifacts(${packageFileName})`);

const fileList = await getFileList();
const fileList = await scm.getFileList();
const lockFiles = fileList.filter((file) => isLockFile(file));
if (!lockFiles.length) {
logger.debug('No Gradle dependency lockfiles found - skipping update');
Expand Down
4 changes: 2 additions & 2 deletions lib/modules/manager/nuget/artifacts.spec.ts
@@ -1,6 +1,6 @@
import { join } from 'upath';
import { envMock, mockExecAll } from '../../../../test/exec-util';
import { env, fs, git, mocked } from '../../../../test/util';
import { env, fs, git, mocked, scm } from '../../../../test/util';
import { GlobalConfig } from '../../../config/global';
import type { RepoGlobalConfig } from '../../../config/types';
import * as docker from '../../../util/exec/docker';
Expand Down Expand Up @@ -40,7 +40,7 @@ describe('modules/manager/nuget/artifacts', () => {
getDefaultRegistries.mockReturnValue([]);
env.getChildProcessEnv.mockReturnValue(envMock.basic);
fs.privateCacheDir.mockImplementation(realFs.privateCacheDir);
git.getFileList.mockResolvedValueOnce([]);
scm.getFileList.mockResolvedValueOnce([]);
GlobalConfig.set(adminConfig);
docker.resetPrefetchedImages();
});
Expand Down
18 changes: 9 additions & 9 deletions lib/modules/manager/nuget/package-tree.spec.ts
@@ -1,7 +1,7 @@
import { fs as memfs } from 'memfs';
import upath from 'upath';
import { Fixtures } from '../../../../test/fixtures';
import { git } from '../../../../test/util';
import { scm } from '../../../../test/util';
import { GlobalConfig } from '../../../config/global';
import type { RepoGlobalConfig } from '../../../config/types';
import { getDependentPackageFiles } from './package-tree';
Expand All @@ -27,7 +27,7 @@ describe('modules/manager/nuget/package-tree', () => {
});

it('returns self for single project', async () => {
git.getFileList.mockResolvedValue(['single.csproj']);
scm.getFileList.mockResolvedValue(['single.csproj']);
Fixtures.mock({
'/tmp/repo/single.csproj': Fixtures.get(
'single-project-file/single.csproj'
Expand All @@ -40,7 +40,7 @@ describe('modules/manager/nuget/package-tree', () => {
});

it('returns self for two projects with no references', async () => {
git.getFileList.mockResolvedValue(['one.csproj', 'two.csproj']);
scm.getFileList.mockResolvedValue(['one.csproj', 'two.csproj']);
Fixtures.mock({
'/tmp/repo/one.csproj': Fixtures.get('two-no-reference/one.csproj'),
'/tmp/repo/two.csproj': Fixtures.get('two-no-reference/two.csproj'),
Expand All @@ -55,7 +55,7 @@ describe('modules/manager/nuget/package-tree', () => {
});

it('returns projects for two projects with one reference', async () => {
git.getFileList.mockResolvedValue(['one/one.csproj', 'two/two.csproj']);
scm.getFileList.mockResolvedValue(['one/one.csproj', 'two/two.csproj']);
Fixtures.mock({
'/tmp/repo/one/one.csproj': Fixtures.get(
'two-one-reference/one/one.csproj'
Expand All @@ -72,7 +72,7 @@ describe('modules/manager/nuget/package-tree', () => {
});

it('returns project for two projects with one reference and central versions', async () => {
git.getFileList.mockResolvedValue(['one/one.csproj', 'two/two.csproj']);
scm.getFileList.mockResolvedValue(['one/one.csproj', 'two/two.csproj']);
Fixtures.mock({
'/tmp/repo/one/one.csproj': Fixtures.get(
'two-one-reference-with-central-versions/one/one.csproj'
Expand All @@ -94,7 +94,7 @@ describe('modules/manager/nuget/package-tree', () => {
});

it('returns projects for three projects with two linear references', async () => {
git.getFileList.mockResolvedValue([
scm.getFileList.mockResolvedValue([
'one/one.csproj',
'two/two.csproj',
'three/three.csproj',
Expand Down Expand Up @@ -128,7 +128,7 @@ describe('modules/manager/nuget/package-tree', () => {
});

it('returns projects for three projects with two tree-like references', async () => {
git.getFileList.mockResolvedValue([
scm.getFileList.mockResolvedValue([
'one/one.csproj',
'two/two.csproj',
'three/three.csproj',
Expand Down Expand Up @@ -160,7 +160,7 @@ describe('modules/manager/nuget/package-tree', () => {
});

it('throws error on circular reference', async () => {
git.getFileList.mockResolvedValue(['one/one.csproj', 'two/two.csproj']);
scm.getFileList.mockResolvedValue(['one/one.csproj', 'two/two.csproj']);
Fixtures.mock({
'/tmp/repo/one/one.csproj': Fixtures.get(
'circular-reference/one/one.csproj'
Expand All @@ -176,7 +176,7 @@ describe('modules/manager/nuget/package-tree', () => {
});

it('skips on invalid xml file', async () => {
git.getFileList.mockResolvedValue(['foo/bar.csproj']);
scm.getFileList.mockResolvedValue(['foo/bar.csproj']);
Fixtures.mock({ '/tmp/repo/foo/bar.csproj': '<invalid' });
expect(await getDependentPackageFiles('foo/bar.csproj')).toEqual([
{ isLeaf: true, name: 'foo/bar.csproj' },
Expand Down
4 changes: 2 additions & 2 deletions lib/modules/manager/nuget/package-tree.ts
Expand Up @@ -3,7 +3,7 @@ import { Graph } from 'graph-data-structure';
import { minimatch } from 'minimatch';
import upath from 'upath';
import { logger } from '../../../logger';
import { getFileList } from '../../../util/git';
import { scm } from '../../platform/scm';
import type { ProjectFile } from './types';
import { readFileAsXmlDocument } from './util';

Expand Down Expand Up @@ -129,7 +129,7 @@ function reframeRelativePathToRootOfRepo(
* Get a list of package files in localDir
*/
async function getAllPackageFiles(): Promise<string[]> {
const allFiles = await getFileList();
const allFiles = await scm.getFileList();
const filteredPackageFiles = allFiles.filter(
minimatch.filter('*.{cs,vb,fs}proj', { matchBase: true, nocase: true })
);
Expand Down
12 changes: 12 additions & 0 deletions lib/modules/platform/default-scm.spec.ts
Expand Up @@ -48,4 +48,16 @@ describe('modules/platform/default-scm', () => {
await defaultGitScm.isBranchModified('branchName');
expect(git.isBranchModified).toHaveBeenCalledTimes(1);
});

it('delegate getFileList to util/git', async () => {
git.getFileList.mockResolvedValueOnce([]);
await defaultGitScm.getFileList();
expect(git.getFileList).toHaveBeenCalledTimes(1);
});

it('delegate checkoutBranch to util/git', async () => {
git.checkoutBranch.mockResolvedValueOnce('');
await defaultGitScm.checkoutBranch('branchName');
expect(git.checkoutBranch).toHaveBeenCalledTimes(1);
});
});
8 changes: 8 additions & 0 deletions lib/modules/platform/default-scm.ts
Expand Up @@ -30,4 +30,12 @@ export class DefaultGitScm implements PlatformScm {
isBranchModified(branchName: string): Promise<boolean> {
return git.isBranchModified(branchName);
}

getFileList(): Promise<string[]> {
return git.getFileList();
}

checkoutBranch(branchName: string): Promise<CommitSha> {
return git.checkoutBranch(branchName);
}
}
2 changes: 2 additions & 0 deletions lib/modules/platform/types.ts
Expand Up @@ -225,4 +225,6 @@ export interface PlatformScm {
getBranchCommit(branchName: string): Promise<CommitSha | null>;
deleteBranch(branchName: string): Promise<void>;
commitAndPush(commitConfig: CommitFilesConfig): Promise<CommitSha | null>;
getFileList(): Promise<string[]>;
checkoutBranch(branchName: string): Promise<CommitSha>;
}
13 changes: 6 additions & 7 deletions lib/workers/repository/config-migration/branch/create.spec.ts
Expand Up @@ -2,7 +2,6 @@ import type { Indent } from 'detect-indent';
import { Fixtures } from '../../../../../test/fixtures';
import { RenovateConfig, getConfig, partial } from '../../../../../test/util';
import { scm } from '../../../../modules/platform/scm';
import { checkoutBranch } from '../../../../util/git';
import { createConfigMigrationBranch } from './create';
import { MigratedDataFactory } from './migrated-data';
import type { MigratedData } from './migrated-data';
Expand Down Expand Up @@ -37,7 +36,7 @@ describe('workers/repository/config-migration/branch/create', () => {
describe('createConfigMigrationBranch', () => {
it('applies the default commit message', async () => {
await createConfigMigrationBranch(config, migratedConfigData);
expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
expect(scm.checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
expect(scm.commitAndPush).toHaveBeenCalledWith({
branchName: 'renovate/migrate-config',
baseBranch: 'dev',
Expand All @@ -60,7 +59,7 @@ describe('workers/repository/config-migration/branch/create', () => {

await createConfigMigrationBranch(config, migratedConfigData);

expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
expect(scm.checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
expect(scm.commitAndPush).toHaveBeenCalledWith({
branchName: 'renovate/migrate-config',
baseBranch: 'dev',
Expand All @@ -84,7 +83,7 @@ describe('workers/repository/config-migration/branch/create', () => {
const message = `PREFIX: migrate config renovate.json`;
await createConfigMigrationBranch(config, migratedConfigData);

expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
expect(scm.checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
expect(scm.commitAndPush).toHaveBeenCalledWith({
branchName: 'renovate/migrate-config',
baseBranch: 'dev',
Expand All @@ -109,7 +108,7 @@ describe('workers/repository/config-migration/branch/create', () => {
const message = `Migrate config renovate.json ${suffix}`;
await createConfigMigrationBranch(config, migratedConfigData);

expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
expect(scm.checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
expect(scm.commitAndPush).toHaveBeenCalledWith({
branchName: 'renovate/migrate-config',
baseBranch: 'dev',
Expand All @@ -135,7 +134,7 @@ describe('workers/repository/config-migration/branch/create', () => {

await createConfigMigrationBranch(config, migratedConfigData);

expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
expect(scm.checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
expect(scm.commitAndPush).toHaveBeenCalledWith({
branchName: 'renovate/migrate-config',
baseBranch: 'dev',
Expand All @@ -160,7 +159,7 @@ describe('workers/repository/config-migration/branch/create', () => {

await createConfigMigrationBranch(config, migratedConfigData);

expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
expect(scm.checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
expect(scm.commitAndPush).toHaveBeenCalledWith({
branchName: 'renovate/migrate-config',
baseBranch: 'dev',
Expand Down
3 changes: 1 addition & 2 deletions lib/workers/repository/config-migration/branch/create.ts
Expand Up @@ -2,7 +2,6 @@ import { GlobalConfig } from '../../../../config/global';
import type { RenovateConfig } from '../../../../config/types';
import { logger } from '../../../../logger';
import { scm } from '../../../../modules/platform/scm';
import { checkoutBranch } from '../../../../util/git';
import { getMigrationBranchName } from '../common';
import { ConfigMigrationCommitMessageFactory } from './commit-message';
import { MigratedDataFactory } from './migrated-data';
Expand All @@ -29,7 +28,7 @@ export async function createConfigMigrationBranch(
return Promise.resolve(null);
}

await checkoutBranch(config.defaultBranch!);
await scm.checkoutBranch(config.defaultBranch!);
const contents = await MigratedDataFactory.applyPrettierFormatting(
migratedConfigData
);
Expand Down
10 changes: 5 additions & 5 deletions lib/workers/repository/config-migration/branch/index.spec.ts
Expand Up @@ -56,7 +56,7 @@ describe('workers/repository/config-migration/branch/index', () => {
const res = await checkConfigMigrationBranch(config, migratedData);
// TODO: types (#7154)
expect(res).toBe(`${config.branchPrefix!}migrate-config`);
expect(git.checkoutBranch).toHaveBeenCalledTimes(1);
expect(scm.checkoutBranch).toHaveBeenCalledTimes(1);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
expect(logger.debug).toHaveBeenCalledWith(
'Config Migration PR already exists'
Expand All @@ -72,7 +72,7 @@ describe('workers/repository/config-migration/branch/index', () => {
const res = await checkConfigMigrationBranch(config, migratedData);
// TODO: types (#7154)
expect(res).toBe(`${config.branchPrefix!}migrate-config`);
expect(git.checkoutBranch).toHaveBeenCalledTimes(0);
expect(scm.checkoutBranch).toHaveBeenCalledTimes(0);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
});

Expand All @@ -83,7 +83,7 @@ describe('workers/repository/config-migration/branch/index', () => {
const res = await checkConfigMigrationBranch(config, migratedData);
// TODO: types (#7154)
expect(res).toBe(`${config.branchPrefix!}migrate-config`);
expect(git.checkoutBranch).toHaveBeenCalledTimes(1);
expect(scm.checkoutBranch).toHaveBeenCalledTimes(1);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
expect(logger.debug).toHaveBeenCalledWith('Need to create migration PR');
});
Expand All @@ -98,7 +98,7 @@ describe('workers/repository/config-migration/branch/index', () => {
const res = await checkConfigMigrationBranch(config, migratedData);
// TODO: types (#7154)
expect(res).toBe(`${config.branchPrefix!}migrate-config`);
expect(git.checkoutBranch).toHaveBeenCalledTimes(0);
expect(scm.checkoutBranch).toHaveBeenCalledTimes(0);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
});

Expand All @@ -112,7 +112,7 @@ describe('workers/repository/config-migration/branch/index', () => {
scm.branchExists.mockResolvedValueOnce(true);
const res = await checkConfigMigrationBranch(config, migratedData);
expect(res).toBeNull();
expect(git.checkoutBranch).toHaveBeenCalledTimes(0);
expect(scm.checkoutBranch).toHaveBeenCalledTimes(0);
expect(scm.commitAndPush).toHaveBeenCalledTimes(0);
expect(scm.deleteBranch).toHaveBeenCalledTimes(1);
expect(logger.debug).toHaveBeenCalledWith(
Expand Down
3 changes: 1 addition & 2 deletions lib/workers/repository/config-migration/branch/index.ts
Expand Up @@ -4,7 +4,6 @@ import { logger } from '../../../../logger';
import { FindPRConfig, Pr, platform } from '../../../../modules/platform';
import { ensureComment } from '../../../../modules/platform/comment';
import { scm } from '../../../../modules/platform/scm';
import { checkoutBranch } from '../../../../util/git';
import { getMigrationBranchName } from '../common';
import { ConfigMigrationCommitMessageFactory } from './commit-message';
import { createConfigMigrationBranch } from './create';
Expand Down Expand Up @@ -67,7 +66,7 @@ export async function checkConfigMigrationBranch(
await createConfigMigrationBranch(config, migratedConfigData);
}
if (!GlobalConfig.get('dryRun')) {
await checkoutBranch(configMigrationBranch);
await scm.checkoutBranch(configMigrationBranch);
}
return configMigrationBranch;
}
Expand Down
@@ -1,11 +1,10 @@
import detectIndent from 'detect-indent';
import { Fixtures } from '../../../../../test/fixtures';
import { mockedFunction } from '../../../../../test/util';
import { mockedFunction, scm } from '../../../../../test/util';

import { migrateConfig } from '../../../../config/migration';
import { logger } from '../../../../logger';
import { readLocalFile } from '../../../../util/fs';
import { getFileList } from '../../../../util/git';
import { detectRepoFileConfig } from '../../init/merge';
import { MigratedDataFactory } from './migrated-data';

Expand Down Expand Up @@ -141,7 +140,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => {
});

beforeEach(() => {
mockedFunction(getFileList).mockResolvedValue([]);
mockedFunction(scm.getFileList).mockResolvedValue([]);
});

it('does not format when no prettier config is present', async () => {
Expand Down Expand Up @@ -173,7 +172,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => {

it('formats when prettier config file is found', async () => {
const formatted = formattedMigratedData.content;
mockedFunction(getFileList).mockResolvedValue(['.prettierrc']);
mockedFunction(scm.getFileList).mockResolvedValue(['.prettierrc']);
await MigratedDataFactory.getAsync();
await expect(
MigratedDataFactory.applyPrettierFormatting(migratedData)
Expand Down
Expand Up @@ -5,8 +5,8 @@ import upath from 'upath';
import { migrateConfig } from '../../../../config/migration';
import { prettier } from '../../../../expose.cjs';
import { logger } from '../../../../logger';
import { scm } from '../../../../modules/platform/scm';
import { readLocalFile } from '../../../../util/fs';
import { getFileList } from '../../../../util/git';
import { detectRepoFileConfig } from '../../init/merge';

export interface MigratedData {
Expand Down Expand Up @@ -43,7 +43,7 @@ export async function applyPrettierFormatting(
): Promise<string> {
try {
logger.trace('applyPrettierFormatting - START');
const fileList = await getFileList();
const fileList = await scm.getFileList();
let prettierExists = fileList.some((file) =>
prettierConfigFilenames.has(file)
);
Expand Down

0 comments on commit 9276a54

Please sign in to comment.