Skip to content

Commit

Permalink
[ftr] remove digdug, use chromedriver directly (#11558)
Browse files Browse the repository at this point in the history
* [ftr] remove digdug, use chromedriver directly

why?
 - digdug is meant to be used by intern, and expects a certain lifecycle that the FTR has tried and continuously fails to mimic
 - rather than continue trying to force digdug into the stack, just spawn chromedriver ourselves. We know how to handle it
 - cleans up verbose remote logging while we're in there, since selenium-standalone-server went with digdug, which was previously doing the verbose logging
 - deprecate config.servers.webdriver
 - add config.chromedriver.url
 - if url at config.chromedriver.url points to a server that responds to http requests use it as the chromedriver instance, enables running chrome in a VM or container
 - if pings to config.chromedriver.url fail a local chromedriver is started

* address review requests
  • Loading branch information
spalger authored May 4, 2017
1 parent e5e939f commit f76bef4
Show file tree
Hide file tree
Showing 21 changed files with 280 additions and 145 deletions.
5 changes: 2 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
"test:browser": "grunt test:browser",
"test:ui": "grunt test:ui",
"test:ui:server": "grunt test:ui:server",
"test:ui:runner": "grunt test:ui:runner",
"test:server": "grunt test:server",
"test:coverage": "grunt test:coverage",
"test:visualRegression": "grunt test:visualRegression",
Expand Down Expand Up @@ -213,9 +212,8 @@
"chance": "1.0.6",
"cheerio": "0.22.0",
"chokidar": "1.6.0",
"chromedriver": "2.28.0",
"chromedriver": "^2.29.0",
"classnames": "2.2.5",
"digdug": "1.6.3",
"enzyme": "2.7.0",
"enzyme-to-json": "1.4.5",
"eslint": "3.11.1",
Expand Down Expand Up @@ -278,6 +276,7 @@
"source-map-support": "0.2.10",
"supertest": "1.2.0",
"supertest-as-promised": "2.0.2",
"tree-kill": "^1.1.0",
"webpack-dev-server": "1.14.1"
},
"engines": {
Expand Down
4 changes: 2 additions & 2 deletions src/cli/cluster/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ module.exports = class Worker extends EventEmitter {
// we don't need to react to process.exit anymore
this.processBinder.destroy();

// wait until the cluster reports this fork has exitted, then resolve
// wait until the cluster reports this fork has exited, then resolve
await new Promise(resolve => this.once('fork:exit', resolve));
}
}
Expand Down Expand Up @@ -153,7 +153,7 @@ module.exports = class Worker extends EventEmitter {
this.forkBinder.on('online', () => this.onOnline());
this.forkBinder.on('disconnect', () => this.onDisconnect());

// when the cluster says a fork has exitted, check if it is ours
// when the cluster says a fork has exited, check if it is ours
this.clusterBinder.on('exit', (fork, code) => this.onExit(fork, code));

// when the process exits, make sure we kill our workers
Expand Down
5 changes: 4 additions & 1 deletion src/functional_test_runner/lib/config/read_config_file.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { defaultsDeep } from 'lodash';

import { Config } from './config';
import { transformDeprecations } from './transform_deprecations';

export async function readConfigFile(log, configFile, settingOverrides = {}) {
log.debug('Loading config file from %j', configFile);
Expand All @@ -24,5 +25,7 @@ export async function readConfigFile(log, configFile, settingOverrides = {}) {
})
);

return new Config(settings);
return new Config(transformDeprecations(settings, msg => {
log.error(msg);
}));
}
5 changes: 4 additions & 1 deletion src/functional_test_runner/lib/config/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,14 @@ export const schema = Joi.object().keys({
),

servers: Joi.object().keys({
webdriver: urlPartsSchema(),
kibana: urlPartsSchema(),
elasticsearch: urlPartsSchema(),
}).default(),

chromedriver: Joi.object().keys({
url: Joi.string().uri({ scheme: /https?/ }).default('http://localhost:9515')
}).default(),

// definition of apps that work with `common.navigateToApp()`
apps: Joi.object().pattern(
ID_PATTERN,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { createTransform, Deprecations } from '../../../deprecation';

export const transformDeprecations = createTransform([
Deprecations.unused('servers.webdriver')
]);
18 changes: 9 additions & 9 deletions src/utils/tooling_log/__tests__/log_levels.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import expect from 'expect.js';
import Chance from 'chance';
import { createLogLevelFlags } from '../log_levels';
import { parseLogLevel } from '../log_levels';

const chance = new Chance();

describe('createLogLevelFlags()', () => {
describe('parseLogLevel(logLevel).flags', () => {
describe('logLevel=silent', () => {
it('produces correct map', () => {
expect(createLogLevelFlags('silent')).to.eql({
expect(parseLogLevel('silent').flags).to.eql({
silent: true,
error: false,
warning: false,
Expand All @@ -20,7 +20,7 @@ describe('createLogLevelFlags()', () => {

describe('logLevel=error', () => {
it('produces correct map', () => {
expect(createLogLevelFlags('error')).to.eql({
expect(parseLogLevel('error').flags).to.eql({
silent: true,
error: true,
warning: false,
Expand All @@ -33,7 +33,7 @@ describe('createLogLevelFlags()', () => {

describe('logLevel=warning', () => {
it('produces correct map', () => {
expect(createLogLevelFlags('warning')).to.eql({
expect(parseLogLevel('warning').flags).to.eql({
silent: true,
error: true,
warning: true,
Expand All @@ -46,7 +46,7 @@ describe('createLogLevelFlags()', () => {

describe('logLevel=info', () => {
it('produces correct map', () => {
expect(createLogLevelFlags('info')).to.eql({
expect(parseLogLevel('info').flags).to.eql({
silent: true,
error: true,
warning: true,
Expand All @@ -59,7 +59,7 @@ describe('createLogLevelFlags()', () => {

describe('logLevel=debug', () => {
it('produces correct map', () => {
expect(createLogLevelFlags('debug')).to.eql({
expect(parseLogLevel('debug').flags).to.eql({
silent: true,
error: true,
warning: true,
Expand All @@ -72,7 +72,7 @@ describe('createLogLevelFlags()', () => {

describe('logLevel=verbose', () => {
it('produces correct map', () => {
expect(createLogLevelFlags('verbose')).to.eql({
expect(parseLogLevel('verbose').flags).to.eql({
silent: true,
error: true,
warning: true,
Expand All @@ -89,7 +89,7 @@ describe('createLogLevelFlags()', () => {
// by specifying a long length
const level = chance.word({ length: 10 });

expect(() => createLogLevelFlags(level))
expect(() => parseLogLevel(level))
.to.throwError(level);
});
});
Expand Down
15 changes: 8 additions & 7 deletions src/utils/tooling_log/log_levels.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,21 @@ const LEVELS = [
'verbose',
];

export function createLogLevelFlags(levelLimit) {
const levelLimitI = LEVELS.indexOf(levelLimit);
export function parseLogLevel(name) {
const i = LEVELS.indexOf(name);

if (levelLimitI === -1) {
if (i === -1) {
const msg = (
`Invalid log level "${levelLimit}" ` +
`Invalid log level "${name}" ` +
`(expected one of ${LEVELS.join(',')})`
);
throw new Error(msg);
}

const flags = {};
LEVELS.forEach((level, i) => {
flags[level] = i <= levelLimitI;
LEVELS.forEach((level, levelI) => {
flags[level] = levelI <= i;
});
return flags;

return { name, flags };
}
27 changes: 19 additions & 8 deletions src/utils/tooling_log/tooling_log.js
Original file line number Diff line number Diff line change
@@ -1,37 +1,40 @@
import { format } from 'util';
import { PassThrough } from 'stream';

import { createLogLevelFlags } from './log_levels';
import { parseLogLevel } from './log_levels';
import { magenta, yellow, red, blue, brightBlack } from 'ansicolors';

export function createToolingLog(logLevel = 'silent') {
const logLevelFlags = createLogLevelFlags(logLevel);
export function createToolingLog(initialLogLevelName = 'silent') {
// current log level (see logLevel.name and logLevel.flags) changed
// with ToolingLog#setLevel(newLogLevelName);
let logLevel = parseLogLevel(initialLogLevelName);

// current indentation level, changed with ToolingLog#indent(delta)
let indentString = '';

class ToolingLog extends PassThrough {
verbose(...args) {
if (!logLevelFlags.verbose) return;
if (!logLevel.flags.verbose) return;
this.write(' %s ', magenta('sill'), format(...args));
}

debug(...args) {
if (!logLevelFlags.debug) return;
if (!logLevel.flags.debug) return;
this.write(' %s ', brightBlack('debg'), format(...args));
}

info(...args) {
if (!logLevelFlags.info) return;
if (!logLevel.flags.info) return;
this.write(' %s ', blue('info'), format(...args));
}

warning(...args) {
if (!logLevelFlags.warning) return;
if (!logLevel.flags.warning) return;
this.write(' %s ', yellow('warn'), format(...args));
}

error(err) {
if (!logLevelFlags.error) return;
if (!logLevel.flags.error) return;

if (typeof err !== 'string' && !(err instanceof Error)) {
err = new Error(`"${err}" thrown`);
Expand All @@ -46,6 +49,14 @@ export function createToolingLog(logLevel = 'silent') {
return indentString.length;
}

getLevel() {
return logLevel.name;
}

setLevel(newLogLevelName) {
logLevel = parseLogLevel(newLogLevelName);
}

write(...args) {
format(...args).split('\n').forEach((line, i) => {
const subLineIndent = i === 0 ? '' : ' ';
Expand Down
30 changes: 1 addition & 29 deletions tasks/config/run.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { format } from 'url';
import { resolve } from 'path';
import chromedriver from 'chromedriver';

module.exports = function (grunt) {
const platform = require('os').platform();
const root = p => resolve(__dirname, '../../', p);
Expand Down Expand Up @@ -155,34 +155,6 @@ module.exports = function (grunt) {
]
},

chromeDriver: {
options: {
wait: false,
ready: /Starting ChromeDriver/,
quiet: false,
failOnError: false
},
cmd: chromedriver.path,
args: [
`--port=${uiConfig.servers.webdriver.port}`,
'--url-base=wd/hub',
]
},

devChromeDriver: {
options: {
wait: false,
ready: /Starting ChromeDriver/,
quiet: false,
failOnError: false
},
cmd: chromedriver.path,
args: [
`--port=${uiConfig.servers.webdriver.port}`,
'--url-base=wd/hub',
]
},

optimizeBuild: {
options: {
wait: false,
Expand Down
6 changes: 0 additions & 6 deletions tasks/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,6 @@ module.exports = function (grunt) {
'run:testUIDevServer:keepalive'
]);

grunt.registerTask('test:ui:runner', [
'checkPlugins',
'clean:screenshots',
'functionalTestRunner'
]);

grunt.registerTask('test:api', [
'esvm:ui',
'run:apiTestServer',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { EventEmitter } from 'events';

import { createLocalChromedriverApi } from './chromedriver_local_api';
import { createRemoteChromedriverApi } from './chromedriver_remote_api';
import { ping } from './ping';

const noop = () => {};

/**
* Api for interacting with a local or remote instance of Chromedriver
*
* @type {Object}
*/
export class ChromedriverApi extends EventEmitter {
static async factory(log, url) {
return (await ping(url))
? createRemoteChromedriverApi(log, url)
: createLocalChromedriverApi(log, url);
}

constructor(options = {}) {
super();

const {
url,
start = noop,
stop = noop,
} = options;

if (!url) {
throw new TypeError('url is a required parameter');
}

this._url = url;
this._state = undefined;
this._callCustomStart = () => start(this);
this._callCustomStop = () => stop(this);
this._beforeStopFns = [];
}

getUrl() {
return this._url;
}

beforeStop(fn) {
this._beforeStopFns.push(fn);
}

isStopped() {
return this._state === 'stopped';
}

async start() {
if (this._state !== undefined) {
throw new Error('Chromedriver can only be started once');
}

this._state = 'started';
await this._callCustomStart();
}

async stop() {
if (this._state !== 'started') {
throw new Error('Chromedriver can only be stopped after being started');
}

this._state = 'stopped';

for (const fn of this._beforeStopFns.splice(0)) {
await fn();
}

await this._callCustomStop();
}
}
Loading

0 comments on commit f76bef4

Please sign in to comment.