✨ Backwards compatibility with v1 config (#160)

* ✨ Add extensible config migration

* 🔥 Remove migration logic from config:migrate

* ✨ Handle own snapshot config migration logic

* ✨ Handle own upload config migration logic

* ✅ Move core queue tests to unit directory

* ✨ Handle own core config migration logic

packages/cli-config/src/commands/config/migrate.js

@@ -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';
 
@@ -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 {
@@ -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) {
@@ -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);
   }
 }

packages/cli-config/test/migrate.test.js

@@ -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 () => {
@@ -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');
   });
 });

packages/cli-snapshot/package.json

@@ -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": {

packages/cli-snapshot/src/config.js

@@ -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);
+  }
+}

packages/cli-snapshot/src/hooks/init.js

@@ -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);
 }

packages/cli-snapshot/test/unit/config.test.js

@@ -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({});
+  });
+});

packages/cli-upload/package.json

@@ -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": {

packages/cli-upload/src/config.js

@@ -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);
+  }
+}

packages/cli-upload/src/hooks/init.js

@@ -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);
 }

packages/cli-upload/test/unit/config.test.js

@@ -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({});
+  });
+});

packages/config/src/index.js

@@ -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';
 
@@ -11,6 +12,9 @@ export default {
   validate,
   addSchema,
   resetSchema,
+  migrate,
+  addMigration,
+  clearMigrations,
   getDefaults,
   stringify
 };