[plugin-helpers] report task failures to CLI with exitCode 1 (#17647) (#17649)

* [plugin-helpers] return promises/fail cli when async tasks fail

* [plugin-helpers] rename taskRunner to commanderAction

* [plugin-helpers] await async assertion

packages/kbn-plugin-helpers/cli.js

@@ -1,18 +1,10 @@
 const program = require('commander');
 
 const pkg = require('./package.json');
-const run = require('./lib/run');
+const createCommanderAction = require('./lib/commander_action');
 const docs = require('./lib/docs');
 const enableCollectingUnknownOptions = require('./lib/enable_collecting_unknown_options');
 
-function taskRunner(fn) {
-  return function actionWrapper() {
-    const args = [].slice.apply(arguments);
-    const command = args.pop();
-    fn.apply(null, [command].concat(args));
-  };
-}
-
 program
   .version(pkg.version);
 
@@ -21,11 +13,9 @@ enableCollectingUnknownOptions(
     .command('start')
     .description('Start kibana and have it include this plugin')
     .on('--help', docs('start'))
-    .action(taskRunner(function (command) {
-      run('start', {
-        flags: command.unknownOptions
-      });
-    }))
+    .action(createCommanderAction('start', (command) => ({
+      flags: command.unknownOptions
+    })))
 );
 
 program
@@ -36,23 +26,19 @@ program
   .option('-d, --build-destination <path>', 'Target path for the build output, absolute or relative to the plugin root')
   .option('-b, --build-version <version>', 'Version for the build output')
   .option('-k, --kibana-version <version>', 'Kibana version for the build output')
-  .action(taskRunner(function (command, files) {
-    run('build', {
-      buildDestination: command.buildDestination,
-      buildVersion: command.buildVersion,
-      kibanaVersion: command.kibanaVersion,
-      skipArchive: Boolean(command.skipArchive),
-      files: files,
-    });
-  }));
+  .action(createCommanderAction('build', (command, files) => ({
+    buildDestination: command.buildDestination,
+    buildVersion: command.buildVersion,
+    kibanaVersion: command.kibanaVersion,
+    skipArchive: Boolean(command.skipArchive),
+    files: files,
+  })));
 
 program
   .command('test')
   .description('Run the server and browser tests')
   .on('--help', docs('test/all'))
-  .action(taskRunner(function () {
-    run('testAll');
-  }));
+  .action(createCommanderAction('testAll'));
 
 program
   .command('test:browser')
@@ -60,22 +46,22 @@ program
   .option('--dev', 'Enable dev mode, keeps the test server running')
   .option('-p, --plugins <plugin-ids>', 'Manually specify which plugins\' test bundles to run')
   .on('--help', docs('test/browser'))
-  .action(taskRunner(function (command) {
-    run('testBrowser', {
-      dev: Boolean(command.dev),
-      plugins: command.plugins,
-    });
-  }));
+  .action(createCommanderAction('testBrowser', (command) => ({
+    dev: Boolean(command.dev),
+    plugins: command.plugins,
+  })));
 
 program
   .command('test:server [files...]')
   .description('Run the server tests using mocha')
   .on('--help', docs('test/server'))
-  .action(taskRunner(function (command, files) {
-    run('testServer', {
-      files: files
-    });
-  }));
+  .action(createCommanderAction('testServer', (command, files) => ({
+    files: files
+  })));
+
+program
+  .command('postinstall')
+  .action(createCommanderAction('postinstall'));
 
 program
   .parse(process.argv);

packages/kbn-plugin-helpers/lib/__snapshots__/commander_action.test.js.snap

@@ -0,0 +1,42 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`commander action exits with status 1 when task throws error asynchronously 1`] = `
+Array [
+  Array [
+    "Task \\"mockTask\\" failed:
+
+Error: async error thrown
+  ...stack trace...
+",
+  ],
+]
+`;
+
+exports[`commander action exits with status 1 when task throws synchronously 1`] = `
+Array [
+  Array [
+    "Task \\"mockTask\\" failed:
+
+Error: sync error thrown
+  ...stack trace...
+",
+  ],
+]
+`;
+
+exports[`commander action passes args to getOptions, calls run() with taskName and options 1`] = `
+Array [
+  Array [
+    "taskName",
+    Object {
+      "args": Array [
+        "b",
+        "c",
+        "d",
+        "e",
+        "f",
+      ],
+    },
+  ],
+]
+`;

packages/kbn-plugin-helpers/lib/commander_action.js

@@ -0,0 +1,12 @@
+const run = require('./run');
+
+module.exports = function createCommanderAction(taskName, getOptions = () => {}) {
+  return async (command, ...args) => {
+    try {
+      await run(taskName, getOptions(...args));
+    } catch (error) {
+      process.stderr.write(`Task "${taskName}" failed:\n\n${error.stack || error.message}\n`);
+      process.exit(1);
+    }
+  };
+};

packages/kbn-plugin-helpers/lib/commander_action.test.js

@@ -0,0 +1,70 @@
+/*eslint-env jest*/
+
+const createCommanderAction = require('./commander_action');
+const run = require('./run');
+
+jest.mock('./run', () => jest.fn());
+
+const STACK_TRACE_RE = /\n(?:\s+at .+(?:\n|$))+/g;
+expect.addSnapshotSerializer({
+  print(val, serialize) {
+    return serialize(val.replace(STACK_TRACE_RE, '\n  ...stack trace...\n'));
+  },
+
+  test(val) {
+    return typeof val === 'string' && STACK_TRACE_RE.test(val);
+  },
+});
+
+beforeAll(() => {
+  jest.spyOn(process.stderr, 'write').mockImplementation(() => {});
+  jest.spyOn(process, 'exit').mockImplementation(() => {});
+});
+
+beforeEach(() => {
+  run.mockReset();
+  jest.clearAllMocks();
+});
+
+afterAll(() => {
+  jest.restoreAllMocks();
+});
+
+describe('commander action', () => {
+  it('creates a function', async () => {
+    expect(typeof createCommanderAction()).toBe('function');
+  });
+
+  it('passes args to getOptions, calls run() with taskName and options', async () => {
+    const action = createCommanderAction('taskName', (...args) => ({ args }));
+    await action('a', 'b', 'c', 'd', 'e', 'f');
+    expect(run).toHaveBeenCalledTimes(1);
+    expect(run.mock.calls).toMatchSnapshot();
+  });
+
+  it('exits with status 1 when task throws synchronously', async () => {
+    run.mockImplementation(() => {
+      throw new Error('sync error thrown');
+    });
+
+    await createCommanderAction('mockTask')();
+
+    expect(process.stderr.write).toHaveBeenCalledTimes(1);
+    expect(process.stderr.write.mock.calls).toMatchSnapshot();
+    expect(process.exit).toHaveBeenCalledTimes(1);
+    expect(process.exit).toHaveBeenCalledWith(1);
+  });
+
+  it('exits with status 1 when task throws error asynchronously', async () => {
+    run.mockImplementation(async () => {
+      throw new Error('async error thrown');
+    });
+
+    await createCommanderAction('mockTask')();
+
+    expect(process.stderr.write).toHaveBeenCalledTimes(1);
+    expect(process.stderr.write.mock.calls).toMatchSnapshot();
+    expect(process.exit).toHaveBeenCalledTimes(1);
+    expect(process.exit).toHaveBeenCalledWith(1);
+  });
+});

packages/kbn-plugin-helpers/lib/run.js

@@ -3,7 +3,9 @@ const tasks = require('./tasks');
 
 module.exports = function run(name, options) {
   const action = tasks[name];
-  if (!action) throw new Error('Invalid task: "' + name + '"');
+  if (!action) {
+    throw new Error('Invalid task: "' + name + '"');
+  }
 
   const plugin = pluginConfig();
   return action(plugin, run, options);

packages/kbn-plugin-helpers/lib/run.test.js

@@ -1,15 +1,16 @@
 /*eslint-env jest*/
 
-const testTask = jest.fn();
-const plugin = { id: 'testPlugin' };
+jest.mock('./plugin_config', () => () => (
+  { id: 'testPlugin' }
+));
 
-jest.mock('./plugin_config', () => () => plugin);
 jest.mock('./tasks', () => {
-  return { testTask };
+  return { testTask: jest.fn() };
 });
+
 const run = require('./run');
 
-describe('task runner', () => {
+describe('lib/run', () => {
   beforeEach(() => jest.resetAllMocks());
 
   it('throw given an invalid task', function () {
@@ -22,9 +23,19 @@ describe('task runner', () => {
   it('runs specified task with plugin and runner', function () {
     run('testTask');
 
+    const { testTask } = require('./tasks');
+    const plugin = require('./plugin_config')();
     const args = testTask.mock.calls[0];
     expect(testTask.mock.calls).toHaveLength(1);
-    expect(args[0]).toBe(plugin);
+    expect(args[0]).toEqual(plugin);
     expect(args[1]).toBe(run);
   });
-});
+
+  it('returns value returned by task', function () {
+    const { testTask } = require('./tasks');
+
+    const symbol = Symbol('foo');
+    testTask.mockReturnValue(symbol);
+    expect(run('testTask')).toBe(symbol);
+  });
+});

packages/kbn-plugin-helpers/tasks/build/__snapshots__/build_action.test.js.snap

@@ -0,0 +1,3 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`calling create_build rejects returned promise when build fails 1`] = `"foo bar"`;

packages/kbn-plugin-helpers/tasks/build/build_action.js

@@ -35,9 +35,6 @@ module.exports = function (plugin, run, options) {
     .then(function () {
       if (options.skipArchive) return;
       return createPackage(plugin, buildTarget, buildVersion);
-    })
-    .catch(function (err) {
-      console.log('BUILD ACTION FAILED:', err);
     });
 };
 
@@ -51,4 +48,4 @@ function askForKibanaVersion() {
   ]).then(function (answers) {
     return answers.kibanaVersion;
   });
-}
+}

packages/kbn-plugin-helpers/tasks/build/build_action.test.js

@@ -84,4 +84,12 @@ describe('calling create_build', () => {
     const [ plugin, buildTarget, buildVersion, kibanaVersion, files ] = mockBuild.mock.calls[0];
     options.files.forEach(file => expect(files).toContain(file));
   });
+
+  it('rejects returned promise when build fails', async () => {
+    mockBuild.mockImplementation(async () => {
+      throw new Error('foo bar');
+    });
+
+    await expect(buildAction(PLUGIN, noop)).rejects.toThrowErrorMatchingSnapshot();
+  });
 });