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(repo/init): return additional raw config from detectRepoFileConfig #17021

Merged
merged 16 commits into from
Aug 18, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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
57 changes: 44 additions & 13 deletions lib/workers/repository/init/merge.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
} from '../../../../test/util';
import * as _migrateAndValidate from '../../../config/migrate-validate';
import * as _migrate from '../../../config/migration';
import { logger } from '../../../logger';
import * as memCache from '../../../util/cache/memory';
import { initRepoCache } from '../../../util/cache/repository/init';
import {
checkForRepoConfigError,
Expand Down Expand Up @@ -36,6 +38,7 @@ jest.mock('../../../config/migrate-validate');
describe('workers/repository/init/merge', () => {
describe('detectRepoFileConfig()', () => {
beforeEach(async () => {
memCache.init();
await initRepoCache({});
});

Expand All @@ -54,14 +57,24 @@ describe('workers/repository/init/merge', () => {
},
});
fs.readLocalFile.mockResolvedValue(pJson);
platform.getJsonFile.mockResolvedValueOnce(pJson);
platform.getRawFile.mockResolvedValueOnce(pJson);
expect(await detectRepoFileConfig()).toEqual({
configFileName: 'package.json',
configFileParsed: { prHourlyLimit: 10 },
configFileRaw: undefined,
});
// get from memCache
expect(await detectRepoFileConfig()).toEqual({
configFileName: 'package.json',
configFileParsed: undefined,
configFileParsed: { prHourlyLimit: 10 },
configFileRaw: undefined,
});
memCache.reset();
// get from repoCache
expect(await detectRepoFileConfig()).toEqual({
configFileName: 'package.json',
configFileParsed: { prHourlyLimit: 10 },
configFileRaw: undefined,
});
});

Expand All @@ -72,7 +85,7 @@ describe('workers/repository/init/merge', () => {
renovate: 'github>renovatebot/renovate',
});
fs.readLocalFile.mockResolvedValue(pJson);
platform.getJsonFile.mockResolvedValueOnce(pJson);
platform.getRawFile.mockResolvedValueOnce(pJson);
expect(await detectRepoFileConfig()).toEqual({
configFileName: 'package.json',
configFileParsed: { extends: ['github>renovatebot/renovate'] },
Expand Down Expand Up @@ -107,13 +120,15 @@ describe('workers/repository/init/merge', () => {
});

it('finds and parse renovate.json5', async () => {
git.getFileList.mockResolvedValue(['package.json', 'renovate.json5']);
fs.readLocalFile.mockResolvedValue(`{
const configFileRaw = `{
// this is json5 format
}`);
}`;
git.getFileList.mockResolvedValue(['package.json', 'renovate.json5']);
fs.readLocalFile.mockResolvedValue(configFileRaw);
expect(await detectRepoFileConfig()).toEqual({
configFileName: 'renovate.json5',
configFileParsed: {},
configFileRaw,
});
});

Expand All @@ -126,6 +141,7 @@ describe('workers/repository/init/merge', () => {
expect(await detectRepoFileConfig()).toEqual({
configFileName: '.github/renovate.json',
configFileParsed: {},
configFileRaw: '{}',
});
});

Expand All @@ -138,23 +154,27 @@ describe('workers/repository/init/merge', () => {
expect(await detectRepoFileConfig()).toEqual({
configFileName: '.gitlab/renovate.json',
configFileParsed: {},
configFileRaw: '{}',
});
});

it('finds .renovaterc.json', async () => {
git.getFileList.mockResolvedValue(['package.json', '.renovaterc.json']);
fs.readLocalFile.mockResolvedValue('{}');
platform.getJsonFile.mockResolvedValueOnce('{"something":"new"}');
platform.getRawFile.mockResolvedValueOnce('{"something":"new"}');
expect(await detectRepoFileConfig()).toEqual({
configFileName: '.renovaterc.json',
configFileParsed: {},
configFileRaw: '{}',
});
memCache.reset();
expect(await detectRepoFileConfig()).toEqual({
configFileName: '.renovaterc.json',
configFileParsed: {
something: 'new',
},
configFileRaw: '{"something":"new"}',
});
expect(await detectRepoFileConfig()).toMatchInlineSnapshot(`
Object {
"configFileName": ".renovaterc.json",
"configFileParsed": "{\\"something\\":\\"new\\"}",
}
`);
});
});

Expand All @@ -174,6 +194,8 @@ describe('workers/repository/init/merge', () => {

describe('mergeRenovateConfig()', () => {
beforeEach(() => {
// memCache.reset();
platform.getRawFile.mockResolvedValueOnce(null);
Gabriel-Ladzaretti marked this conversation as resolved.
Show resolved Hide resolved
migrate.migrateConfig.mockReturnValue({
isMigrated: false,
migratedConfig: {},
Expand All @@ -194,6 +216,9 @@ describe('workers/repository/init/merge', () => {
}
expect(e).toBeDefined();
expect(e?.toString()).toBe('Error: config-validation');
expect(logger.debug).toHaveBeenCalledWith(
'Existing config file no longer exists'
);
});

it('migrates nested config', async () => {
Expand Down Expand Up @@ -224,6 +249,9 @@ describe('workers/repository/init/merge', () => {
},
],
});
expect(logger.debug).toHaveBeenCalledWith(
'Existing config file no longer exists'
);
});

it('continues if no errors', async () => {
Expand All @@ -235,6 +263,9 @@ describe('workers/repository/init/merge', () => {
});
config.extends = [':automergeDisabled'];
expect(await mergeRenovateConfig(config)).toBeDefined();
expect(logger.debug).toHaveBeenCalledWith(
'Existing config file no longer exists'
);
});
});
});
45 changes: 37 additions & 8 deletions lib/workers/repository/init/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,39 @@ import {
import { logger } from '../../../logger';
import * as npmApi from '../../../modules/datasource/npm';
import { platform } from '../../../modules/platform';
import * as memCache from '../../../util/cache/memory';
import { getCache } from '../../../util/cache/repository';
import { readLocalFile } from '../../../util/fs';
import { getFileList } from '../../../util/git';
import * as hostRules from '../../../util/host-rules';
import type { RepoFileConfig } from './types';

export async function detectRepoFileConfig(): Promise<RepoFileConfig> {
const cacheKey = 'detectRepoFileConfig';
let repoFileConfig = memCache.get<RepoFileConfig>(cacheKey);
if (repoFileConfig) {
return repoFileConfig;
}
const cache = getCache();
let { configFileName } = cache;
if (configFileName) {
let configFileParsed = (await platform.getJsonFile(configFileName))!;
let configFileRaw: string | undefined = (await platform.getRawFile(
rarkins marked this conversation as resolved.
Show resolved Hide resolved
rarkins marked this conversation as resolved.
Show resolved Hide resolved
Gabriel-Ladzaretti marked this conversation as resolved.
Show resolved Hide resolved
configFileName
))!;
Gabriel-Ladzaretti marked this conversation as resolved.
Show resolved Hide resolved
let configFileParsed = JSON5.parse(configFileRaw);
if (configFileParsed) {
if (configFileName === 'package.json') {
configFileParsed = configFileParsed.renovate;
configFileRaw = undefined;
}
return { configFileName, configFileParsed };
repoFileConfig = { configFileName, configFileRaw, configFileParsed };
memCache.set(cacheKey, repoFileConfig);
return repoFileConfig;
}
logger.debug('Existing config file no longer exists');
}
const fileList = await getFileList();

async function detectConfigFile(): Promise<string | null> {
for (const fileName of configFileNames) {
if (fileName === 'package.json') {
Expand All @@ -57,6 +70,7 @@ export async function detectRepoFileConfig(): Promise<RepoFileConfig> {
}
return null;
}

configFileName = (await detectConfigFile()) ?? undefined;
if (!configFileName) {
logger.debug('No renovate config file found');
Expand All @@ -66,6 +80,7 @@ export async function detectRepoFileConfig(): Promise<RepoFileConfig> {
logger.debug(`Found ${configFileName} config file`);
// TODO #7154
let configFileParsed: any;
let rawFileContents;
Gabriel-Ladzaretti marked this conversation as resolved.
Show resolved Hide resolved
if (configFileName === 'package.json') {
// We already know it parses
configFileParsed = JSON.parse(
Expand All @@ -78,7 +93,7 @@ export async function detectRepoFileConfig(): Promise<RepoFileConfig> {
}
logger.debug({ config: configFileParsed }, 'package.json>renovate config');
} else {
let rawFileContents = await readLocalFile(configFileName, 'utf8');
rawFileContents = await readLocalFile(configFileName, 'utf8');
Gabriel-Ladzaretti marked this conversation as resolved.
Show resolved Hide resolved
// istanbul ignore if
if (!is.string(rawFileContents)) {
logger.warn({ configFileName }, 'Null contents when reading config file');
Expand All @@ -101,10 +116,12 @@ export async function detectRepoFileConfig(): Promise<RepoFileConfig> {
);
const validationError = 'Invalid JSON5 (parsing failed)';
const validationMessage = `JSON5.parse error: ${String(err.message)}`;
return {
repoFileConfig = {
configFileName,
configFileParseError: { validationError, validationMessage },
};
memCache.set(cacheKey, repoFileConfig);
return repoFileConfig;
}
} else {
let allowDuplicateKeys = true;
Expand All @@ -115,10 +132,12 @@ export async function detectRepoFileConfig(): Promise<RepoFileConfig> {
if (jsonValidationError) {
const validationError = 'Invalid JSON (parsing failed)';
const validationMessage = jsonValidationError;
return {
repoFileConfig = {
configFileName,
configFileParseError: { validationError, validationMessage },
};
memCache.set(cacheKey, repoFileConfig);
return repoFileConfig;
}
allowDuplicateKeys = false;
jsonValidationError = jsonValidator.validate(
Expand All @@ -128,10 +147,12 @@ export async function detectRepoFileConfig(): Promise<RepoFileConfig> {
if (jsonValidationError) {
const validationError = 'Duplicate keys in JSON';
const validationMessage = JSON.stringify(jsonValidationError);
return {
repoFileConfig = {
configFileName,
configFileParseError: { validationError, validationMessage },
};
memCache.set(cacheKey, repoFileConfig);
return repoFileConfig;
}
try {
configFileParsed = JSON5.parse(rawFileContents);
Expand All @@ -142,18 +163,26 @@ export async function detectRepoFileConfig(): Promise<RepoFileConfig> {
);
const validationError = 'Invalid JSON (parsing failed)';
const validationMessage = `JSON.parse error: ${String(err.message)}`;
return {
repoFileConfig = {
configFileName,
configFileParseError: { validationError, validationMessage },
};
memCache.set(cacheKey, repoFileConfig);
return repoFileConfig;
}
}
logger.debug(
{ fileName: configFileName, config: configFileParsed },
'Repository config'
);
}
return { configFileName, configFileParsed };
repoFileConfig = {
configFileName,
configFileRaw: rawFileContents,
configFileParsed,
};
memCache.set(cacheKey, repoFileConfig);
return repoFileConfig;
}

export function checkForRepoConfigError(repoConfig: RepoFileConfig): void {
Expand Down
1 change: 1 addition & 0 deletions lib/workers/repository/init/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export interface RepoConfigError {

export interface RepoFileConfig {
configFileName?: string;
configFileRaw?: string;
Gabriel-Ladzaretti marked this conversation as resolved.
Show resolved Hide resolved
configFileParsed?: any;
configFileParseError?: RepoConfigError;
}
Expand Down