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

refactor(scm): use scm for getFileList, checkoutBranch #22006

Merged
merged 1 commit into from May 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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