From cb38ed0a8aae2cb862001e0b6f076aa9972f4489 Mon Sep 17 00:00:00 2001 From: Julie Ralph Date: Wed, 28 Dec 2016 12:04:59 -0800 Subject: [PATCH] Refactor element explorer to work with selenium-webdriver 3 (#3828) This implementation now relies mostly on promises explicitly, so the control flow is only used to add one large task to the queue. This should pave the way for the eventual removal of the control flow, as well as getting element explorer to work immediately. BREAKING CHANGE You can no longer use the `repl` command from within `browser.pause()`. Instead, use `broser.explore()` to directly enter the repl. --- lib/breakpointhook.js | 10 +++ lib/browser.ts | 43 +++++++------ lib/debugger.ts | 99 ++++++++++++++++-------------- lib/debugger/clients/explorer.js | 22 +++++-- lib/debugger/clients/wddebugger.js | 61 ++++-------------- lib/debugger/debuggerCommons.js | 47 +++++++++----- lib/debugger/modes/commandRepl.js | 7 ++- lib/debugger/modes/debuggerRepl.js | 58 +++++++++++------ lib/frameworks/explorer.js | 3 +- scripts/test.js | 1 + 10 files changed, 198 insertions(+), 153 deletions(-) create mode 100644 lib/breakpointhook.js diff --git a/lib/breakpointhook.js b/lib/breakpointhook.js new file mode 100644 index 000000000..a8aac8ed7 --- /dev/null +++ b/lib/breakpointhook.js @@ -0,0 +1,10 @@ +module.exports = function() { + return true; +}; + +/** + * The reason this file exists is so that we can set a breakpoint via + * script name, and then control when that breakpoint is set in + * our library code by importing and calling this function. The + * breakpoint will always be on line 2. + */ \ No newline at end of file diff --git a/lib/browser.ts b/lib/browser.ts index 6c646ecb4..51c4bf623 100644 --- a/lib/browser.ts +++ b/lib/browser.ts @@ -972,37 +972,46 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { } /** - * Beta (unstable) enterRepl function for entering the repl loop from - * any point in the control flow. Use browser.enterRepl() in your test. + * See browser.explore(). + */ + enterRepl(opt_debugPort?: number) { + return this.explore(opt_debugPort); + } + + /** + * Beta (unstable) explore function for entering the repl loop from + * any point in the control flow. Use browser.explore() in your test. * Does not require changes to the command line (no need to add 'debug'). * Note, if you are wrapping your own instance of Protractor, you must * expose globals 'browser' and 'protractor' for pause to work. * * @example * element(by.id('foo')).click(); - * browser.enterRepl(); + * browser.explore(); * // Execution will stop before the next click action. * element(by.id('bar')).click(); * * @param {number=} opt_debugPort Optional port to use for the debugging * process */ - enterRepl(opt_debugPort?: number) { + explore(opt_debugPort?: number) { let debuggerClientPath = __dirname + '/debugger/clients/explorer.js'; - let onStartFn = () => { - logger.info(); - logger.info('------- Element Explorer -------'); - logger.info( - 'Starting WebDriver debugger in a child process. Element ' + - 'Explorer is still beta, please report issues at ' + - 'github.com/angular/protractor'); - logger.info(); - logger.info('Type to see a list of locator strategies.'); - logger.info('Use the `list` helper function to find elements by strategy:'); - logger.info(' e.g., list(by.binding(\'\')) gets all bindings.'); + let onStartFn = (firstTime: boolean) => { logger.info(); + if (firstTime) { + logger.info('------- Element Explorer -------'); + logger.info( + 'Starting WebDriver debugger in a child process. Element ' + + 'Explorer is still beta, please report issues at ' + + 'github.com/angular/protractor'); + logger.info(); + logger.info('Type to see a list of locator strategies.'); + logger.info('Use the `list` helper function to find elements by strategy:'); + logger.info(' e.g., list(by.binding(\'\')) gets all bindings.'); + logger.info(); + } }; - this.debugHelper.init(debuggerClientPath, onStartFn, opt_debugPort); + this.debugHelper.initBlocking(debuggerClientPath, onStartFn, opt_debugPort); } /** @@ -1040,8 +1049,6 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { logger.info(); logger.info('press c to continue to the next webdriver command'); logger.info('press ^D to detach debugger and resume code execution'); - logger.info('type "repl" to enter interactive mode'); - logger.info('type "exit" to break out of interactive mode'); logger.info(); } }; diff --git a/lib/debugger.ts b/lib/debugger.ts index 6a87d8261..300af0b89 100644 --- a/lib/debugger.ts +++ b/lib/debugger.ts @@ -7,6 +7,7 @@ import {Locator} from './locators'; import {Logger} from './logger'; import {Ptor} from './ptor'; import * as helper from './util'; +let breakpointHook = require('./breakpointhook.js'); declare var global: any; declare var process: any; @@ -25,32 +26,36 @@ export class DebugHelper { constructor(private browserUnderDebug_: ProtractorBrowser) {} + + initBlocking(debuggerClientPath: string, onStartFn: Function, opt_debugPort?: number) { + this.init_(debuggerClientPath, true, onStartFn, opt_debugPort); + } + + init(debuggerClientPath: string, onStartFn: Function, opt_debugPort?: number) { + this.init_(debuggerClientPath, false, onStartFn, opt_debugPort); + } + /** * 1) Set up helper functions for debugger clients to call on (e.g. - * getControlFlowText, execute code, get autocompletion). + * execute code, get autocompletion). * 2) Enter process into debugger mode. (i.e. process._debugProcess). * 3) Invoke the debugger client specified by debuggerClientPath. * * @param {string} debuggerClientPath Absolute path of debugger client to use. + * @param {boolean} blockUntilExit Whether to block the flow until process exit or resume + * immediately. * @param {Function} onStartFn Function to call when the debugger starts. The * function takes a single parameter, which represents whether this is the * first time that the debugger is called. * @param {number=} opt_debugPort Optional port to use for the debugging * process. + * + * @return {Promise} If blockUntilExit, a promise resolved when the debugger process + * exits. Otherwise, resolved when the debugger process is ready to begin. */ - init(debuggerClientPath: string, onStartFn: Function, opt_debugPort?: number) { - (wdpromise.ControlFlow as any).prototype.getControlFlowText = function() { - let controlFlowText = this.getSchedule(/* opt_includeStackTraces */ true); - // This filters the entire control flow text, not just the stack trace, so - // unless we maintain a good (i.e. non-generic) set of keywords in - // STACK_SUBSTRINGS_TO_FILTER, we run the risk of filtering out non stack - // trace. The alternative though, which is to reimplement - // webdriver.promise.ControlFlow.prototype.getSchedule() here is much - // hackier, and involves messing with the control flow's internals / - // private variables. - return helper.filterStackTrace(controlFlowText); - }; - + init_( + debuggerClientPath: string, blockUntilExit: boolean, onStartFn: Function, + opt_debugPort?: number) { const vm_ = require('vm'); let flow = wdpromise.controlFlow(); @@ -75,8 +80,11 @@ export class DebugHelper { } let sandbox = vm_.createContext(context); - let debuggerReadyPromise = wdpromise.defer(); - flow.execute(() => { + let debuggingDone = wdpromise.defer(); + + // We run one flow.execute block for the debugging session. All + // subcommands should be scheduled under this task. + let executePromise = flow.execute(() => { process['debugPort'] = opt_debugPort || process['debugPort']; this.validatePortAvailability_(process['debugPort']).then((firstTime: boolean) => { onStartFn(firstTime); @@ -93,34 +101,30 @@ export class DebugHelper { .on('message', (m: string) => { if (m === 'ready') { - debuggerReadyPromise.fulfill(); + breakpointHook(); + if (!blockUntilExit) { + debuggingDone.fulfill(); + } } }) .on('exit', () => { - logger.info('Debugger exiting'); // Clear this so that we know it's ok to attach a debugger // again. this.dbgCodeExecutor = null; + debuggingDone.fulfill(); }); }); - }); - - let pausePromise = flow.execute(() => { - return debuggerReadyPromise.promise.then(() => { - // Necessary for backward compatibility with node < 0.12.0 - return this.browserUnderDebug_.executeScriptWithDescription('', 'empty debugger hook'); - }); - }); + return debuggingDone.promise; + }, 'debugging tasks'); // Helper used only by debuggers at './debugger/modes/*.js' to insert code - // into the control flow. - // In order to achieve this, we maintain a promise at the top of the control + // into the control flow, via debugger 'evaluate' protocol. + // In order to achieve this, we maintain a task at the top of the control // flow, so that we can insert frames into it. // To be able to simulate callback/asynchronous code, we poll this object - // for a result at every run of DeferredExecutor.execute. - let browserUnderDebug = this.browserUnderDebug_; + // whenever `breakpointHook` is called. this.dbgCodeExecutor = { - execPromise_: pausePromise, // Promise pointing to current stage of flow. + execPromise_: undefined, // Promise pointing to currently executing command. execPromiseResult_: undefined, // Return value of promise. execPromiseError_: undefined, // Error from promise. @@ -137,20 +141,19 @@ export class DebugHelper { execute_: function(execFn_: Function) { this.execPromiseResult_ = this.execPromiseError_ = undefined; - this.execPromise_ = this.execPromise_.then(execFn_).then( + this.execPromise_ = execFn_(); + // Note: This needs to be added after setting execPromise to execFn, + // or else we cause this.execPromise_ to get stuck in pending mode + // at our next breakpoint. + this.execPromise_.then( (result: Object) => { this.execPromiseResult_ = result; + breakpointHook(); }, (err: Error) => { this.execPromiseError_ = err; + breakpointHook(); }); - - // This dummy command is necessary so that the DeferredExecutor.execute - // break point can find something to stop at instead of moving on to the - // next real command. - this.execPromise_.then(() => { - return browserUnderDebug.executeScriptWithDescription('', 'empty debugger hook'); - }); }, // Execute a piece of code. @@ -159,7 +162,12 @@ export class DebugHelper { let execFn_ = () => { // Run code through vm so that we can maintain a local scope which is // isolated from the rest of the execution. - let res = vm_.runInContext(code, sandbox); + let res; + try { + res = vm_.runInContext(code, sandbox); + } catch (e) { + res = 'Error while evaluating command: ' + e; + } if (!wdpromise.isPromise(res)) { res = wdpromise.fulfilled(res); } @@ -190,14 +198,14 @@ export class DebugHelper { deferred.fulfill(JSON.stringify(res)); } }); - return deferred; + return deferred.promise; }; this.execute_(execFn_); }, // Code finished executing. resultReady: function() { - return !this.execPromise_.isPending(); + return !(this.execPromise_.state_ === 'pending'); }, // Get asynchronous results synchronously. @@ -213,7 +221,7 @@ export class DebugHelper { } }; - return pausePromise; + return executePromise; } /** @@ -227,7 +235,7 @@ export class DebugHelper { * is done. The promise will resolve to a boolean which represents whether * this is the first time that the debugger is called. */ - private validatePortAvailability_(port: number): wdpromise.Promise { + private validatePortAvailability_(port: number): wdpromise.Promise { if (this.debuggerValidated_) { return wdpromise.fulfilled(false); } @@ -256,8 +264,9 @@ export class DebugHelper { }); return doneDeferred.promise.then( - () => { + (firstTime: boolean) => { this.debuggerValidated_ = true; + return firstTime; }, (err: string) => { console.error(err); diff --git a/lib/debugger/clients/explorer.js b/lib/debugger/clients/explorer.js index 1fd9f0ab9..b0dee20c2 100644 --- a/lib/debugger/clients/explorer.js +++ b/lib/debugger/clients/explorer.js @@ -38,6 +38,10 @@ WdRepl.prototype.initServer_ = function(port) { // Intentionally blank. }); sock.end(); + // TODO(juliemr): Investigate why this is necessary. At this point, there + // should be no active listeners so this process should just exit + // by itself. + process.exit(0); } else if (input[input.length - 1] === '\t') { // If the last character is the TAB key, this is an autocomplete // request. We use everything before the TAB as the init data to feed @@ -98,15 +102,17 @@ WdRepl.prototype.initRepl_ = function() { output: process.stdout, eval: stepEval, useGlobal: false, - ignoreUndefined: true + ignoreUndefined: true, + completer: cmdRepl.complete.bind(cmdRepl) }); - replServer.complete = cmdRepl.complete.bind(cmdRepl); - replServer.on('exit', function() { - console.log('Exiting...'); + console.log('Element Explorer Exiting...'); self.client.req({command: 'disconnect'}, function() { - // Intentionally blank. + // TODO(juliemr): Investigate why this is necessary. At this point, there + // should be no active listeners so this process should just exit + // by itself. + process.exit(0); }); }); }; @@ -137,6 +143,12 @@ WdRepl.prototype.init = function() { var self = this; this.client = debuggerCommons.attachDebugger(process.argv[2], process.argv[3]); this.client.once('ready', function() { + debuggerCommons.setEvaluateBreakpoint(self.client, function() { + process.send('ready'); + self.client.reqContinue(function() { + // Intentionally blank. + }); + }); self.initReplOrServer_(); }); }; diff --git a/lib/debugger/clients/wddebugger.js b/lib/debugger/clients/wddebugger.js index 767e05af3..f082e376d 100644 --- a/lib/debugger/clients/wddebugger.js +++ b/lib/debugger/clients/wddebugger.js @@ -1,24 +1,16 @@ var repl = require('repl'); var debuggerCommons = require('../debuggerCommons'); -var CommandRepl = require('../modes/commandRepl'); var DebuggerRepl = require('../modes/debuggerRepl'); /** - * BETA BETA BETA - * Custom protractor debugger which steps through one control flow task - * at a time. + * Custom protractor debugger which steps through one control flow task at a time. * * @constructor */ var WdDebugger = function() { this.client; this.replServer; - - // repl is broken into 'command repl' and 'debugger repl'. - this.cmdRepl; this.dbgRepl; - // currentRepl is a pointer to one of them. - this.currentRepl; }; /** @@ -37,36 +29,7 @@ WdDebugger.prototype.stepEval_ = function(cmd, context, filename, callback) { // Think about whether this is a better pattern. cmd = debuggerCommons.trimReplCmd(cmd); - - if (this.currentRepl === this.dbgRepl && cmd === 'repl' || - this.currentRepl === this.cmdRepl && cmd === 'exit') { - // switch repl mode - this.currentRepl = - this.currentRepl === this.dbgRepl ? this.cmdRepl : this.dbgRepl; - // For node backward compatibility. In older versions of node `setPrompt` - // does not exist, and we set the prompt by overwriting `replServer.prompt` - // directly. - if (this.replServer.setPrompt) { - this.replServer.setPrompt(this.currentRepl.prompt); - } else { - this.replServer.prompt = this.currentRepl.prompt; - } - this.replServer.complete = this.currentRepl.complete.bind(this.currentRepl); - callback(); - } else if (this.currentRepl === this.cmdRepl) { - // If we are currently in command repl mode. - this.cmdRepl.stepEval(cmd, function(err, res) { - // Result is a string representation of the evaluation, so we console.log - // the result to print it properly. Then we callback with undefined so - // that the result isn't printed twice. - if (res !== undefined) { - console.log(res); - } - callback(err, undefined); - }); - } else { - this.dbgRepl.stepEval(cmd, callback); - } + this.dbgRepl.stepEval(cmd, callback); }; /** @@ -75,28 +38,20 @@ WdDebugger.prototype.stepEval_ = function(cmd, context, filename, callback) { */ WdDebugger.prototype.initRepl_ = function() { var self = this; - this.cmdRepl = new CommandRepl(this.client); this.dbgRepl = new DebuggerRepl(this.client); - this.currentRepl = this.dbgRepl; // We want the prompt to show up only after the controlflow text prints. this.dbgRepl.printControlFlow_(function() { - // Backward compatibility: node version 0.8.14 has a number of built in - // libraries for repl, and the keyword 'repl' clashes with our usage. - if (repl._builtinLibs && repl._builtinLibs.indexOf('repl') > -1) { - repl._builtinLibs.splice(repl._builtinLibs.indexOf('repl'), 1); - } self.replServer = repl.start({ - prompt: self.currentRepl.prompt, + prompt: self.dbgRepl.prompt, input: process.stdin, output: process.stdout, eval: self.stepEval_.bind(self), useGlobal: false, - ignoreUndefined: true + ignoreUndefined: true, + completer: self.dbgRepl.complete.bind(self.dbgRepl) }); - self.replServer.complete = self.currentRepl.complete.bind(self.currentRepl); - self.replServer.on('exit', function() { console.log('Resuming code execution'); self.client.req({command: 'disconnect'}, function() { @@ -114,6 +69,12 @@ WdDebugger.prototype.init = function() { var self = this; this.client = debuggerCommons.attachDebugger(process.argv[2], process.argv[3]); this.client.once('ready', function() { + debuggerCommons.setWebDriverCommandBreakpoint(self.client, function() { + process.send('ready'); + self.client.reqContinue(function() { + // Intentionally blank. + }); + }); self.initRepl_(); }); }; diff --git a/lib/debugger/debuggerCommons.js b/lib/debugger/debuggerCommons.js index f0e039eed..7575fb20f 100644 --- a/lib/debugger/debuggerCommons.js +++ b/lib/debugger/debuggerCommons.js @@ -2,7 +2,6 @@ var baseDebugger = require('_debugger'); /** * Create a debugger client and attach to a running protractor process. - * Set a break point at webdriver executor. * @param {number} pid Pid of the process to attach the debugger to. * @param {number=} opt_port Port to set up the debugger connection over. * @return {!baseDebugger.Client} The connected debugger client. @@ -14,19 +13,6 @@ exports.attachDebugger = function(pid, opt_port) { // Call this private function instead of sending SIGUSR1 because Windows. process._debugProcess(pid); - client.once('ready', function() { - client.setBreakpoint({ - type: 'scriptRegExp', - target: 'lib/http\.js', //jshint ignore:line - line: 432 - }, function() { - process.send('ready'); - client.reqContinue(function() { - // Intentionally blank. - }); - }); - }); - // Connect to debugger on port with retry 200ms apart. var connectWithRetry = function(attempts) { client.connect(port, 'localhost') @@ -45,6 +31,39 @@ exports.attachDebugger = function(pid, opt_port) { return client; }; + +/** + * Set a breakpoint for evaluating REPL statements. + */ +exports.setEvaluateBreakpoint = function(client, cb) { + client.setBreakpoint({ + type: 'scriptRegExp', + target: 'built/breakpointhook\.js', //jshint ignore:line + line: 2 + }, function(err, response) { + if (err) { + throw new Error(err); + } + cb(response.breakpoint); + }); +}; + +/** + * Set a breakpoint for moving forward by one webdriver command. + */ +exports.setWebDriverCommandBreakpoint = function(client, cb) { + client.setBreakpoint({ + type: 'scriptRegExp', + target: 'lib/http\.js', //jshint ignore:line + line: 433 + }, function(err, response) { + if (err) { + throw new Error(err); + } + cb(response.breakpoint); + }); +}; + /** * Trim excess symbols from the repl command so that it is consistent with * the user input. diff --git a/lib/debugger/modes/commandRepl.js b/lib/debugger/modes/commandRepl.js index 311779664..6608e5970 100644 --- a/lib/debugger/modes/commandRepl.js +++ b/lib/debugger/modes/commandRepl.js @@ -9,7 +9,7 @@ var REPL_INITIAL_SUGGESTIONS = [ ]; /** - * Repl to interactively run code. + * Repl to interactively run commands in the context of the test. * * @param {Client} node debugger client. * @constructor @@ -46,6 +46,7 @@ CommandRepl.prototype.complete = function(line, callback) { if (line === '') { callback(null, [REPL_INITIAL_SUGGESTIONS, '']); } else { + // TODO(juliemr): This is freezing the program! line = line.replace(/"/g, '\\\"'); var expr = 'browser.debugHelper.dbgCodeExecutor.complete("' + line + '")'; this.evaluate_(expr, function(err, res) { @@ -76,6 +77,10 @@ CommandRepl.prototype.evaluate_ = function(expression, callback) { expression: 'browser.debugHelper.dbgCodeExecutor.resultReady()' } }, function(err, res) { + if (err) { + throw new Error('Error while checking if debugger expression result was ready.' + + 'Expression: ' + expression + ' Error: ' + err); + } // If code finished executing, get result. if (res.value) { self.client.req({ diff --git a/lib/debugger/modes/debuggerRepl.js b/lib/debugger/modes/debuggerRepl.js index 55592c7ad..0d1c46266 100644 --- a/lib/debugger/modes/debuggerRepl.js +++ b/lib/debugger/modes/debuggerRepl.js @@ -4,14 +4,14 @@ var DBG_INITIAL_SUGGESTIONS = ['repl', 'c', 'frame', 'scopes', 'scripts', 'source', 'backtrace']; /** - * Repl to step through code. + * Repl to step through webdriver test code. * * @param {Client} node debugger client. * @constructor */ var DebuggerRepl = function(client) { this.client = client; - this.prompt = 'wd-debug> '; + this.prompt = '>>> '; }; /** @@ -25,11 +25,19 @@ var DebuggerRepl = function(client) { DebuggerRepl.prototype.stepEval = function(cmd, callback) { switch (cmd) { case 'c': - this.printControlFlow_(callback); + this.printNextStep_(callback); this.client.reqContinue(function() { // Intentionally blank. }); break; + case 'repl': + console.log('Error: using repl from browser.pause() has been removed. ' + + 'Please use browser.enterRepl instead.'); + callback(); + break; + case 'schedule': + this.printControlFlow_(callback); + break; case 'frame': this.client.req({command: 'frame'}, function(err, res) { console.log(util.inspect(res, {colors: true})); @@ -75,20 +83,21 @@ DebuggerRepl.prototype.stepEval = function(cmd, callback) { * @param {string} line Initial user entry * @param {function} callback */ -DebuggerRepl.prototype.complete = function(line, callback) { +DebuggerRepl.prototype.complete = function(line, callback) { var suggestions = DBG_INITIAL_SUGGESTIONS.filter(function(suggestion) { return suggestion.indexOf(line) === 0; }); + console.log('suggestions'); callback(null, [suggestions, line]); }; /** - * Print the controlflow. + * Print the next command and setup the next breakpoint. * * @private * @param {function} callback */ -DebuggerRepl.prototype.printControlFlow_ = function(callback) { +DebuggerRepl.prototype.printNextStep_ = function(callback) { var self = this; var onBreak_ = function() { self.client.req({ @@ -99,25 +108,36 @@ DebuggerRepl.prototype.printControlFlow_ = function(callback) { expression: 'command.getName()' } }, function(err, res) { + // We ignore errors here because we'll get one from the initial break. if (res.value) { console.log('-- Next command: ' + res.value); } - self.client.req({ - command: 'evaluate', - arguments: { - frame: 0, - maxStringLength: 4000, - expression: 'protractor.promise.controlFlow().getControlFlowText()' - } - }, function(err, controlFlowResponse) { - if (controlFlowResponse.value) { - console.log(controlFlowResponse.value); - } - callback(); - }); + callback(); }); }; this.client.once('break', onBreak_); }; +/** + * Print the controlflow. + * + * @private + * @param {function} callback + */ +DebuggerRepl.prototype.printControlFlow_ = function(callback) { + this.client.req({ + command: 'evaluate', + arguments: { + frame: 0, + maxStringLength: 4000, + expression: 'protractor.promise.controlFlow().getSchedule()' + } + }, function(err, controlFlowResponse) { + if (controlFlowResponse.value) { + console.log(controlFlowResponse.value); + } + callback(); + }); +}; + module.exports = DebuggerRepl; diff --git a/lib/frameworks/explorer.js b/lib/frameworks/explorer.js index f2608dc33..4a60adb4a 100644 --- a/lib/frameworks/explorer.js +++ b/lib/frameworks/explorer.js @@ -13,8 +13,9 @@ exports.run = function(runner) { if (runner.getConfig().baseUrl) { browser.get(runner.getConfig().baseUrl); } + browser.executeScriptWithDescription('var e = 0', 'starting explorer hook'); browser.enterRepl(); - browser.executeScriptWithDescription('', 'empty debugger hook').then(function() { + browser.executeScriptWithDescription('var e = 1', 'done with explorer hook').then(function() { resolve({ failedCount: 0 }); diff --git a/scripts/test.js b/scripts/test.js index 82a00b850..f544eb445 100755 --- a/scripts/test.js +++ b/scripts/test.js @@ -38,6 +38,7 @@ var passingTests = [ 'node built/cli.js spec/hybridConf.js', 'node scripts/driverProviderAttachSession.js', 'node scripts/errorTest.js', + // Interactive Element Explorer tasks 'node scripts/interactive_tests/interactive_test.js', 'node scripts/interactive_tests/with_base_url.js', // Unit tests