[kbn-test] use slightly more debug-friendly error output (#21985)

A couple times while debugging failures in `functional_tests(_servers)` with people I've asked for stack traces and received responses like "that's all there is", and it turns out that's right, because the cli's are passing the error object directly to `chalk.red()`, which converts it into a string that only includes the message. This pr moves the common operations from `run_tests/cli.js` and `run_servers/cli.js` into `lib/run_cli` and includes test for the common functionality there, as well as a common error printing logic that still includes the red message, but also includes a stack trace that will help out a lot in debugging.
This commit is contained in:
Spencer 2018-08-15 09:50:42 -07:00 committed by GitHub
parent 25e5a1e1d4
commit 595476bcf2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 325 additions and 111 deletions

View file

@ -1,9 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`run tests CLI options accepts help option even if invalid options passed 1`] = `
Array [
Array [
"Run Functional Tests
"Run Functional Tests
Usage:
node scripts/functional_tests --help
@ -21,55 +19,48 @@ Options:
--verbose Log everything.
--debug Run in debug mode.
--quiet Only log errors.
--silent Log nothing.",
],
]
--silent Log nothing.
"
`;
exports[`run tests CLI options rejects boolean config value 1`] = `
Array [
Array [
"Error: functional_tests: invalid argument [true] to option [config]",
],
]
"
functional_tests: invalid argument [true] to option [config]
...stack trace...
"
`;
exports[`run tests CLI options rejects boolean value for kibana-install-dir 1`] = `
Array [
Array [
"Error: functional_tests: invalid argument [true] to option [kibana-install-dir]",
],
]
"
functional_tests: invalid argument [true] to option [kibana-install-dir]
...stack trace...
"
`;
exports[`run tests CLI options rejects empty config value if no default passed 1`] = `
Array [
Array [
"Error: functional_tests: config is required",
],
]
"
functional_tests: config is required
...stack trace...
"
`;
exports[`run tests CLI options rejects invalid options even if valid options exist 1`] = `
Array [
Array [
"Error: functional_tests: invalid option [aintnothang]",
],
]
"
functional_tests: invalid option [aintnothang]
...stack trace...
"
`;
exports[`run tests CLI options rejects non-boolean value for bail 1`] = `
Array [
Array [
"Error: functional_tests: invalid argument [peanut] to option [bail]",
],
]
"
functional_tests: invalid argument [peanut] to option [bail]
...stack trace...
"
`;
exports[`run tests CLI options rejects non-enum value for esFrom 1`] = `
Array [
Array [
"Error: functional_tests: invalid argument [butter] to option [esFrom]",
],
]
"
functional_tests: invalid argument [butter] to option [esFrom]
...stack trace...
"
`;

View file

@ -17,9 +17,8 @@
* under the License.
*/
import chalk from 'chalk';
import getopts from 'getopts';
import { runTests } from '../../tasks';
import { runCli } from '../../lib';
import { processOptions, displayHelp } from './args';
/**
@ -31,17 +30,8 @@ import { processOptions, displayHelp } from './args';
* if no config option is passed
*/
export async function runTestsCli(defaultConfigPaths) {
try {
const userOptions = getopts(process.argv.slice(2)) || {};
if (userOptions.help) {
console.log(displayHelp());
return undefined;
}
await runCli(displayHelp, async userOptions => {
const options = processOptions(userOptions, defaultConfigPaths);
await runTests(options);
} catch (err) {
console.log(chalk.red(err));
process.exit(1);
}
});
}

View file

@ -18,6 +18,7 @@
*/
import { runTestsCli } from './cli';
import { checkMockConsoleLogSnapshot } from '../../test_helpers';
// Note: Stub the runTests function to keep testing only around the cli
// method and arguments.
@ -62,7 +63,7 @@ describe('run tests CLI', () => {
await runTestsCli();
expect(exitMock).toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
it('rejects empty config value if no default passed', async () => {
@ -71,7 +72,7 @@ describe('run tests CLI', () => {
await runTestsCli();
expect(exitMock).toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
it('accepts empty config value if default passed', async () => {
@ -88,7 +89,7 @@ describe('run tests CLI', () => {
await runTestsCli(['foo']);
expect(exitMock).toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
it('accepts string value for kibana-install-dir', async () => {
@ -105,7 +106,7 @@ describe('run tests CLI', () => {
await runTestsCli(['foo']);
expect(exitMock).toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
it('accepts boolean value for updateBaselines', async () => {
@ -130,7 +131,7 @@ describe('run tests CLI', () => {
await runTestsCli(['foo']);
expect(exitMock).toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
it('accepts value for grep', async () => {
@ -187,7 +188,7 @@ describe('run tests CLI', () => {
await runTestsCli(['foo']);
expect(exitMock).not.toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
it('rejects invalid options even if valid options exist', async () => {
@ -196,7 +197,7 @@ describe('run tests CLI', () => {
await runTestsCli(['foo']);
expect(exitMock).toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
});
});

View file

@ -1,57 +1,50 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`start servers CLI options accepts boolean value for updateBaselines 1`] = `
Array [
Array [
"Error: functional_tests_server: invalid option [updateBaselines]",
],
]
"
functional_tests_server: invalid option [updateBaselines]
...stack trace...
"
`;
exports[`start servers CLI options rejects bail 1`] = `
Array [
Array [
"Error: functional_tests_server: invalid option [bail]",
],
]
"
functional_tests_server: invalid option [bail]
...stack trace...
"
`;
exports[`start servers CLI options rejects boolean config value 1`] = `
Array [
Array [
"Error: functional_tests_server: invalid argument [true] to option [config]",
],
]
"
functional_tests_server: invalid argument [true] to option [config]
...stack trace...
"
`;
exports[`start servers CLI options rejects boolean value for kibana-install-dir 1`] = `
Array [
Array [
"Error: functional_tests_server: invalid argument [true] to option [kibana-install-dir]",
],
]
"
functional_tests_server: invalid argument [true] to option [kibana-install-dir]
...stack trace...
"
`;
exports[`start servers CLI options rejects empty config value if no default passed 1`] = `
Array [
Array [
"Error: functional_tests_server: config is required",
],
]
"
functional_tests_server: config is required
...stack trace...
"
`;
exports[`start servers CLI options rejects invalid options even if valid options exist 1`] = `
Array [
Array [
"Error: functional_tests_server: invalid option [grep]",
],
]
"
functional_tests_server: invalid option [grep]
...stack trace...
"
`;
exports[`start servers CLI options rejects non-enum value for esFrom 1`] = `
Array [
Array [
"Error: functional_tests_server: invalid argument [butter] to option [esFrom]",
],
]
"
functional_tests_server: invalid argument [butter] to option [esFrom]
...stack trace...
"
`;

View file

@ -17,9 +17,8 @@
* under the License.
*/
import chalk from 'chalk';
import getopts from 'getopts';
import { startServers } from '../../tasks';
import { runCli } from '../../lib';
import { processOptions, displayHelp } from './args';
/**
@ -28,17 +27,8 @@ import { processOptions, displayHelp } from './args';
* if no config option is passed
*/
export async function startServersCli(defaultConfigPath) {
try {
const userOptions = getopts(process.argv.slice(2)) || {};
if (userOptions.help) {
console.log(displayHelp());
return undefined;
}
await runCli(displayHelp, async userOptions => {
const options = processOptions(userOptions, defaultConfigPath);
await startServers(options);
} catch (err) {
console.log(chalk.red(err));
process.exit(1);
}
});
}

View file

@ -18,6 +18,7 @@
*/
import { startServersCli } from './cli';
import { checkMockConsoleLogSnapshot } from '../../test_helpers';
// Note: Stub the startServers function to keep testing only around the cli
// method and arguments.
@ -62,7 +63,7 @@ describe('start servers CLI', () => {
await startServersCli();
expect(exitMock).toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
it('rejects empty config value if no default passed', async () => {
@ -71,7 +72,7 @@ describe('start servers CLI', () => {
await startServersCli();
expect(exitMock).toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
it('accepts empty config value if default passed', async () => {
@ -88,7 +89,7 @@ describe('start servers CLI', () => {
await startServersCli('foo');
expect(exitMock).toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
it('accepts string value for kibana-install-dir', async () => {
@ -105,7 +106,7 @@ describe('start servers CLI', () => {
await startServersCli('foo');
expect(exitMock).toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
it('accepts boolean value for updateBaselines', async () => {
@ -114,7 +115,7 @@ describe('start servers CLI', () => {
await startServersCli('foo');
expect(exitMock).toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
it('accepts source value for esFrom', async () => {
@ -131,7 +132,7 @@ describe('start servers CLI', () => {
await startServersCli('foo');
expect(exitMock).toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
it('accepts debug option', async () => {
@ -188,7 +189,7 @@ describe('start servers CLI', () => {
await startServersCli('foo');
expect(exitMock).toHaveBeenCalledWith(1);
expect(logMock.mock.calls).toMatchSnapshot();
checkMockConsoleLogSnapshot(logMock);
});
});
});

View file

@ -0,0 +1,26 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`does right thing when non-error is thrown 1`] = `
"
'foo bar' thrown!
...stack trace...
"
`;
exports[`logs no stack trace then exits when stack missing 1`] = `
"
foo error
(no stack trace)
"
`;
exports[`logs the stack then exits when run function throws an error 1`] = `
"
foo error
stack 1
stack 2
stack 3
"
`;

View file

@ -21,3 +21,4 @@ export { runKibanaServer } from './run_kibana_server';
export { runElasticsearch } from './run_elasticsearch';
export { runFtr } from './run_ftr';
export { KIBANA_ROOT, KIBANA_FTR_SCRIPT, FUNCTIONAL_CONFIG_PATH, API_CONFIG_PATH } from './paths';
export { runCli } from './run_cli';

View file

@ -0,0 +1,58 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { inspect } from 'util';
import chalk from 'chalk';
import getopts from 'getopts';
export async function runCli(getHelpText, run) {
try {
const userOptions = getopts(process.argv.slice(2)) || {};
if (userOptions.help) {
console.log(getHelpText());
return;
}
await run(userOptions);
} catch (error) {
if (!(error instanceof Error)) {
error = new Error(`${inspect(error)} thrown!`);
}
console.log();
console.log(chalk.red(error.message));
// first line in the stack trace is the message, skip it as we log it directly and color it red
if (error.stack) {
console.log(
error.stack
.split('\n')
.slice(1)
.join('\n')
);
} else {
console.log(' (no stack trace)');
}
console.log();
process.exit(1);
}
}

View file

@ -0,0 +1,133 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { runCli } from './run_cli';
import { checkMockConsoleLogSnapshot } from '../test_helpers';
const mockProcessExit = jest.spyOn(process, 'exit').mockImplementation(() => {});
const mockConsoleLog = jest.spyOn(console, 'log').mockImplementation(() => {});
const actualProcessArgv = process.argv;
const sleep = ms => new Promise(resolve => setTimeout(resolve, ms));
beforeEach(() => {
process.argv = actualProcessArgv.slice(0, 2);
jest.clearAllMocks();
});
afterAll(() => {
process.argv = actualProcessArgv;
});
it('accepts help option even if invalid options passed', async () => {
process.argv.push('--foo', '--bar', '--help');
const mockGetHelpText = jest.fn().mockReturnValue('mock help text');
const mockRun = jest.fn();
await runCli(mockGetHelpText, mockRun);
expect(mockProcessExit).not.toHaveBeenCalled();
expect(mockGetHelpText).toHaveBeenCalledTimes(1);
expect(mockConsoleLog).toHaveBeenCalledTimes(1);
expect(mockConsoleLog).toHaveBeenCalledWith('mock help text');
expect(mockRun).not.toHaveBeenCalled();
});
it('passes parsed argv to run function', async () => {
process.argv.push('--foo', 'bar', '--baz=box', '--', 'a', 'b', 'c');
const mockGetHelpText = jest.fn();
const mockRun = jest.fn();
await runCli(mockGetHelpText, mockRun);
expect(mockGetHelpText).not.toHaveBeenCalled();
expect(mockConsoleLog).not.toHaveBeenCalled();
expect(mockProcessExit).not.toHaveBeenCalled();
expect(mockRun).toHaveBeenCalledTimes(1);
expect(mockRun).toHaveBeenCalledWith({
foo: 'bar',
baz: 'box',
_: ['a', 'b', 'c'],
});
});
it('waits for promise returned from run function to resolve before resolving', async () => {
let resolveMockRun;
const mockRun = jest.fn().mockImplementation(
() =>
new Promise(resolve => {
resolveMockRun = resolve;
})
);
const onResolved = jest.fn();
const promise = runCli(null, mockRun).then(onResolved);
expect(mockRun).toHaveBeenCalled();
expect(onResolved).not.toHaveBeenCalled();
await sleep(500);
expect(onResolved).not.toHaveBeenCalled();
resolveMockRun();
await promise;
expect(onResolved).toHaveBeenCalled();
});
it('logs the stack then exits when run function throws an error', async () => {
await runCli(null, () => {
const error = new Error('foo error');
error.stack = 'foo error\n stack 1\n stack 2\n stack 3';
throw error;
});
expect(mockProcessExit).toHaveBeenCalledTimes(1);
expect(mockProcessExit).toHaveBeenCalledWith(1);
expect(mockConsoleLog).toHaveBeenCalled();
checkMockConsoleLogSnapshot(mockConsoleLog);
});
it('logs no stack trace then exits when stack missing', async () => {
await runCli(null, () => {
const error = new Error('foo error');
error.stack = undefined;
throw error;
});
expect(mockProcessExit).toHaveBeenCalledTimes(1);
expect(mockProcessExit).toHaveBeenCalledWith(1);
expect(mockConsoleLog).toHaveBeenCalled();
checkMockConsoleLogSnapshot(mockConsoleLog);
});
it('does right thing when non-error is thrown', async () => {
await runCli(null, () => {
throw 'foo bar';
});
expect(mockProcessExit).toHaveBeenCalledTimes(1);
expect(mockProcessExit).toHaveBeenCalledWith(1);
expect(mockConsoleLog).toHaveBeenCalled();
checkMockConsoleLogSnapshot(mockConsoleLog);
});

View file

@ -0,0 +1,30 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
/* eslint-env jest */
import { format } from 'util';
export function checkMockConsoleLogSnapshot(logMock) {
const output = logMock.mock.calls
.reduce((acc, args) => `${acc}${format(...args)}\n`, '')
.replace(/(^ at.+[>)\d]$\n?)+/m, ' ...stack trace...');
expect(output).toMatchSnapshot();
}