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

✨ Backwards compatibility with v1 config #160

Merged
merged 6 commits into from
Feb 2, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 5 additions & 66 deletions packages/cli-config/src/commands/config/migrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ import Command, { flags } from '@oclif/command';
import PercyConfig from '@percy/config';
import logger from '@percy/logger';

function assignOrCreate(obj, key, value) {
return Object.assign(obj || {}, { [key]: value });
}

export class Migrate extends Command {
static description = 'Migrate a Percy config file to the latest version';

Expand Down Expand Up @@ -44,8 +40,6 @@ export class Migrate extends Command {
flags: { 'dry-run': dry }
} = this.parse();

logger.loglevel('info');

// load config using the explorer directly rather than the load method to
// better control logs and prevent validation
try {
Expand Down Expand Up @@ -77,13 +71,14 @@ export class Migrate extends Command {
// migrate config
this.log.info('Migrating config file...');
let format = path.extname(output).replace(/^./, '') || 'yaml';
config = PercyConfig.stringify(format, this.migrate(config));
let migrated = PercyConfig.migrate(config);
let body = PercyConfig.stringify(format, migrated);

// update the package.json entry via string replacement
if (!dry && path.basename(output) === 'package.json') {
fs.writeFileSync(output, fs.readFileSync(output).replace(
/(\s+)("percy":\s*){.*\1}/s,
`$1$2${config.replace(/\n/g, '$$1')}`
`$1$2${body.replace(/\n/g, '$$1')}`
));
// write to output
} else if (!dry) {
Expand All @@ -94,67 +89,11 @@ export class Migrate extends Command {
fs.renameSync(input, old);
}

fs.writeFileSync(output, config);
fs.writeFileSync(output, body);
}

this.log.info('Config file migrated!');
// when dry-running, print config to stdout when finished
if (dry) logger.instance.stdout.write('\n' + config);
}

// Migrating config options is recursive so no matter which input version is
// provided, the output will be the latest version.
migrate(input) {
switch (input.version) {
case 2: return input; // latest version
default: return this.migrate(this.v1(input));
}
}

// Migrate config from v1 to v2.
/* eslint-disable curly */
v1(input) {
let output = { version: 2 };

// previous snapshot options map 1:1
if (input.snapshot != null)
output.snapshot = input.snapshot;
// request-headers option moved
if (input.agent?.['asset-discovery']?.['request-headers'] != null)
output.snapshot = assignOrCreate(output.snapshot, 'request-headers', (
input.agent['asset-discovery']['request-headers']));
// only create discovery options when neccessary
if (input.agent?.['asset-discovery']?.['allowed-hostnames'] != null)
output.discovery = assignOrCreate(output.discovery, 'allowed-hostnames', (
input.agent['asset-discovery']['allowed-hostnames']));
if (input.agent?.['asset-discovery']?.['network-idle-timeout'] != null)
output.discovery = assignOrCreate(output.discovery, 'network-idle-timeout', (
input.agent['asset-discovery']['network-idle-timeout']));
// page pooling was rewritten to be a concurrent task queue
if (input.agent?.['asset-discovery']?.['page-pool-size-max'] != null)
output.discovery = assignOrCreate(output.discovery, 'concurrency', (
input.agent['asset-discovery']['page-pool-size-max']));
// cache-responses was renamed to match the CLI flag
if (input.agent?.['asset-discovery']?.['cache-responses'] != null)
output.discovery = assignOrCreate(output.discovery, 'disable-cache', (
!input.agent['asset-discovery']['cache-responses']));
// image-snapshots was renamed
if (input['image-snapshots'] != null)
output.upload = input['image-snapshots'];
// image-snapshots path was removed
if (output.upload?.path != null)
delete output.upload.path;
// static-snapshots and options were renamed
if (input['static-snapshots']?.['base-url'] != null)
output.static = assignOrCreate(output.static, 'base-url', (
input['static-snapshots']['base-url']));
if (input['static-snapshots']?.['snapshot-files'] != null)
output.static = assignOrCreate(output.static, 'files', (
input['static-snapshots']['snapshot-files']));
if (input['static-snapshots']?.['ignore-files'] != null)
output.static = assignOrCreate(output.static, 'ignore', (
input['static-snapshots']['ignore-files']));

return output;
if (dry) logger.instance.stdout.write('\n' + body);
}
}
59 changes: 12 additions & 47 deletions packages/cli-config/test/migrate.test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
import expect from 'expect';
import PercyConfig from '@percy/config';
import { logger, mockConfig, getMockConfig } from './helpers';
import { Migrate } from '../src/commands/config/migrate';

describe('percy config:migrate', () => {
beforeEach(() => {
mockConfig('.percy.yml', 'version: 1\n');
PercyConfig.addMigration((input, set) => {
if (input.migrate != null) set('migrated', input.migrate.replace('old', 'new'));
});
});

afterEach(() => {
PercyConfig.clearMigrations();
});

it('by default, renames the config before writing', async () => {
Expand Down Expand Up @@ -106,60 +114,17 @@ describe('percy config:migrate', () => {
expect(getMockConfig('.percy.old.yml')).toBeUndefined();
});

it('migrates v1 config', async () => {
it('runs registered migrations on the config', async () => {
mockConfig('.percy.yml', [
'version: 1',
'snapshot:',
' widths: [1000]',
' min-height: 1000',
' enable-javascript: true',
' percy-css: "iframe { display: none; }"',
'agent:',
' asset-discovery:',
' allowed-hostnames:',
' - cdn.example.com',
' request-headers:',
' Authorization: Basic dXNlcm5hbWU6cGFzc3dvcmQ=',
' network-idle-timeout: 500',
' cache-responses: false',
' page-pool-size-min: 10',
' page-pool-size-max: 50',
'static-snapshots:',
' path: _site/',
' base-url: /blog/',
' snapshot-files: "**/*.html"',
' ignore-files: "**/*.htm"',
'image-snapshots:',
' path: _images/',
' files: "**/*.html"',
' ignore: "**/*.htm"\n'
'migrate: old-value'
].join('\n'));

await Migrate.run([]);

expect(getMockConfig('.percy.yml')).toEqual([
'version: 2',
'snapshot:',
' widths:',
' - 1000',
' min-height: 1000',
' enable-javascript: true',
' percy-css: "iframe { display: none; }"',
' request-headers:',
' Authorization: Basic dXNlcm5hbWU6cGFzc3dvcmQ=',
'discovery:',
' allowed-hostnames:',
' - cdn.example.com',
' network-idle-timeout: 500',
' concurrency: 50',
' disable-cache: true',
'upload:',
' files: "**/*.html"',
' ignore: "**/*.htm"',
'static:',
' base-url: /blog/',
' files: "**/*.html"',
' ignore: "**/*.htm"\n'
].join('\n'));
'migrated: new-value'
].join('\n') + '\n');
});
});
2 changes: 1 addition & 1 deletion packages/cli-snapshot/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"postbuild": "oclif-dev manifest",
"readme": "oclif-dev readme",
"pretest": "node ../../scripts/install-browser",
"test": "cross-env NODE_ENV=test mocha",
"test": "cross-env NODE_ENV=test mocha --recursive",
"test:coverage": "nyc yarn test"
},
"publishConfig": {
Expand Down
13 changes: 13 additions & 0 deletions packages/cli-snapshot/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,16 @@ export const schema = {
}
}
};

export function migration(input, set) {
/* eslint-disable curly */
if (input.version < 2) {
// static-snapshots and options were renamed
if (input.staticSnapshots?.baseUrl != null)
set('static.baseUrl', input.staticSnapshots.baseUrl);
if (input.staticSnapshots?.snapshotFiles != null)
set('static.files', input.staticSnapshots.snapshotFiles);
if (input.staticSnapshots?.ignoreFiles != null)
set('static.ignore', input.staticSnapshots.ignoreFiles);
}
}
3 changes: 2 additions & 1 deletion packages/cli-snapshot/src/hooks/init.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import PercyConfig from '@percy/config';
import { schema } from '../config';
import { schema, migration } from '../config';

export default function() {
PercyConfig.addSchema(schema);
PercyConfig.addMigration(migration);
}
54 changes: 54 additions & 0 deletions packages/cli-snapshot/test/unit/config.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import expect from 'expect';
import { migration } from '../../src/config';

describe('unit / config', () => {
let migrate;
let set = (key, value) => (migrate[key] = value);

beforeEach(() => {
migrate = {};
});

it('migrates v1 config', () => {
migration({
version: 1,
staticSnapshots: {
baseUrl: 'base-url',
snapshotFiles: '*.html',
ignoreFiles: '*.htm'
}
}, set);

expect(migrate).toEqual({
'static.baseUrl': 'base-url',
'static.files': '*.html',
'static.ignore': '*.htm'
});
});

it('only migrates own config options', () => {
migration({
version: 1,
otherOptions: {
baseUrl: 'base-url',
snapshotFiles: '*.html',
ignoreFiles: '*.htm'
}
}, set);

expect(migrate).toEqual({});
});

it('does not migrate when not needed', () => {
migration({
version: 2,
static: {
baseUrl: 'base-url',
files: '*.html',
ignore: '*.htm'
}
}, set);

expect(migrate).toEqual({});
});
});
2 changes: 1 addition & 1 deletion packages/cli-upload/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"lint": "eslint --ignore-path ../../.gitignore .",
"postbuild": "oclif-dev manifest",
"readme": "oclif-dev readme",
"test": "cross-env NODE_ENV=test mocha",
"test": "cross-env NODE_ENV=test mocha --recursive",
"test:coverage": "nyc yarn test"
},
"publishConfig": {
Expand Down
11 changes: 11 additions & 0 deletions packages/cli-upload/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,14 @@ export const schema = {
}
}
};

export function migration(input, set) {
/* eslint-disable curly */
if (input.version < 2) {
// image-snapshots and options were renamed
if (input.imageSnapshots?.files != null)
set('upload.files', input.imageSnapshots.files);
if (input.imageSnapshots?.ignore != null)
set('upload.ignore', input.imageSnapshots.ignore);
}
}
3 changes: 2 additions & 1 deletion packages/cli-upload/src/hooks/init.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import PercyConfig from '@percy/config';
import { schema } from '../config';
import { schema, migration } from '../config';

export default function() {
PercyConfig.addSchema(schema);
PercyConfig.addMigration(migration);
}
52 changes: 52 additions & 0 deletions packages/cli-upload/test/unit/config.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import expect from 'expect';
import { migration } from '../../src/config';

describe('unit / config', () => {
let migrate;
let set = (key, value) => (migrate[key] = value);

beforeEach(() => {
migrate = {};
});

it('migrates v1 config', () => {
migration({
version: 1,
imageSnapshots: {
path: '~/pathname/',
files: '*.png',
ignore: '*.jpg'
}
}, set);

expect(migrate).toEqual({
'upload.files': '*.png',
'upload.ignore': '*.jpg'
});
});

it('only migrates own config options', () => {
migration({
version: 1,
otherOptions: {
path: '~/pathname/',
files: '*.png',
ignore: '*.jpg'
}
}, set);

expect(migrate).toEqual({});
});

it('does not migrate when not needed', () => {
migration({
version: 2,
upload: {
files: '*.png',
ignore: '*.jpg'
}
}, set);

expect(migrate).toEqual({});
});
});
4 changes: 4 additions & 0 deletions packages/config/src/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import load, { cache, explorer } from './load';
import validate, { addSchema, resetSchema } from './validate';
import migrate, { addMigration, clearMigrations } from './migrate';
import getDefaults from './defaults';
import stringify from './stringify';

Expand All @@ -11,6 +12,9 @@ export default {
validate,
addSchema,
resetSchema,
migrate,
addMigration,
clearMigrations,
getDefaults,
stringify
};
Loading