Skip to content
This repository was archived by the owner on Jul 29, 2024. It is now read-only.

feat(debugger): make element explorer work with node 0.12.0 #1898

Closed
wants to merge 1 commit into from
Closed

feat(debugger): make element explorer work with node 0.12.0 #1898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 6 additions & 18 deletions lib/debugger/clients/explorer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
var repl = require('repl');
var baseDebugger = require('_debugger');
var debuggerCommons = require('../debuggerCommons');
var CommandRepl = require('../modes/commandRepl');

/**
Expand All @@ -9,28 +9,16 @@ var CommandRepl = require('../modes/commandRepl');
* @constructor
*/
var WdRepl = function() {
this.client = new baseDebugger.Client();
this.client;
};

/**
* Initiate debugger client.
* @private
*/
WdRepl.prototype.initClient_ = function() {
var client = this.client;

client.once('ready', function() {

client.setBreakpoint({
type: 'scriptRegExp',
target: '.*executors\.js', //jshint ignore:line
line: 37
}, function() {});
});

var host = 'localhost';
var port = process.argv[2] || 5858;
client.connect(port, host); // TODO - might want to add retries here.
this.client =
debuggerCommons.attachDebugger(process.argv[2], process.argv[3]);
};

/**
Expand Down Expand Up @@ -103,7 +91,7 @@ WdRepl.prototype.initRepl_ = function() {
var stepEval = function(cmd, context, filename, callback) {
// The command that eval feeds is of the form '(CMD\n)', so we trim the
// double quotes and new line.
cmd = cmd.slice(1, cmd.length - 2);
cmd = debuggerCommons.trimReplCmd(cmd);
cmdRepl.stepEval(cmd, function(err, res) {
// Result is a string representation of the evaluation.
if (res !== undefined) {
Expand Down Expand Up @@ -142,7 +130,7 @@ WdRepl.prototype.initReplOrServer_ = function() {
// advantage is that the process becomes much more realistic because now we're
// using the normal repl. However, it was not possible to test autocomplete
// this way since we cannot immitate the TAB key over the wire.
var debuggerServerPort = process.argv[3];
var debuggerServerPort = process.argv[4];
if (debuggerServerPort) {
this.initServer_(debuggerServerPort);
} else {
Expand Down
51 changes: 26 additions & 25 deletions lib/debugger/clients/wddebugger.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
var repl = require('repl');
var baseDebugger = require('_debugger');
var debuggerCommons = require('../debuggerCommons');
var CommandRepl = require('../modes/commandRepl');
var DebuggerRepl = require('../modes/debuggerRepl');

Expand All @@ -11,7 +11,7 @@ var DebuggerRepl = require('../modes/debuggerRepl');
* @constructor
*/
var WdDebugger = function() {
this.client = new baseDebugger.Client();
this.client;
this.replServer;

// repl is broken into 'command repl' and 'debugger repl'.
Expand All @@ -26,28 +26,17 @@ var WdDebugger = function() {
* @private
*/
WdDebugger.prototype.initClient_ = function() {
var client = this.client;

client.once('ready', function() {
this.client =
debuggerCommons.attachDebugger(process.argv[2], process.argv[3]);
this.client.once('ready', function() {
console.log(' ready\n');

client.setBreakpoint({
type: 'scriptRegExp',
target: '.*executors\.js', //jshint ignore:line
line: 37
}, function() {
console.log('press c to continue to the next webdriver command');
console.log('press d to continue to the next debugger statement');
console.log('type "repl" to enter interactive mode');
console.log('type "exit" to break out of interactive mode');
console.log('press ^C to exit');
console.log();
});
console.log('press c to continue to the next webdriver command');
console.log('press d to continue to the next debugger statement');
console.log('type "repl" to enter interactive mode');
console.log('type "exit" to break out of interactive mode');
console.log('press ^C to exit');
console.log();
});

var host = 'localhost';
var port = process.argv[2] || 5858;
client.connect(port, host); // TODO - might want to add retries here.
};

/**
Expand All @@ -60,19 +49,26 @@ WdDebugger.prototype.initClient_ = function() {
*/
WdDebugger.prototype.stepEval_ = function(cmd, context, filename, callback) {
// The loop won't come back until 'callback' is called.
// Strip out the () which the REPL adds and the new line.
// Note - node's debugger gets around this by adding custom objects
// named 'c', 's', etc to the REPL context. They have getters which
// perform the desired function, and the callback is stored for later use.
// Think about whether this is a better pattern.
cmd = cmd.slice(1, cmd.length - 2);

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;
this.replServer.prompt = this.currentRepl.prompt;
// 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) {
Expand Down Expand Up @@ -103,6 +99,11 @@ WdDebugger.prototype.initRepl_ = function() {

// 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to worry about versions <10 anymore, feel free to remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping this, as discussed offline.

// 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,
input: process.stdin,
Expand Down
60 changes: 60 additions & 0 deletions lib/debugger/debuggerCommons.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
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.
*/
exports.attachDebugger = function(pid, opt_port) {
var client = new baseDebugger.Client();
var port = opt_port || process.debugPort;

// Call this private function instead of sending SIGUSR1 because Windows.
process._debugProcess(pid);

client.once('ready', function() {
client.setBreakpoint({
type: 'scriptRegExp',
target: '.*executors\.js', //jshint ignore:line
line: 37
}, function() {
client.reqContinue(function() {
// Intentionally blank.
});
});
});

// Connect to debugger on port with retry 200ms apart.
var connectWithRetry = function(attempts) {
client.connect(port, 'localhost')
.on('error', function(e) {
if (attempts === 1) {
throw e;
} else {
setTimeout(function() {
connectWithRetry(attempts - 1);
}, 200);
}
});
};
connectWithRetry(10);

return client;
};

/**
* Trim excess symbols from the repl command so that it is consistent with
* the user input.
* @param {string} cmd Cmd provided by the repl server.
* @return {string} The trimmed cmd.
*/
exports.trimReplCmd = function(cmd) {
// Given user input 'foobar', some versions of node provide '(foobar\n)',
// while other versions of node provide 'foobar\n'.
if (cmd.length >= 2 && cmd[0] === '(' && cmd[cmd.length - 1] === ')') {
cmd = cmd.substring(1, cmd.length - 1);
}
return cmd.slice(0, cmd.length - 1);
};
75 changes: 47 additions & 28 deletions lib/protractor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,34 +661,30 @@ Protractor.prototype.initDebugger_ = function(debuggerClientPath, opt_debugPort)
return asString;
};

if (opt_debugPort) {
process.debugPort = opt_debugPort;
}

// Call this private function instead of sending SIGUSR1 because Windows.
process._debugProcess(process.pid);

var flow = webdriver.promise.controlFlow();
var self = this;
var pausePromise = flow.execute(function() {
log.puts('Starting WebDriver debugger in a child process. Pause is ' +
'still beta, please report issues at github.com/angular/protractor\n');
var args = [process.debugPort];
if (self.debuggerServerPort_) {
args.push(self.debuggerServerPort_);
}
var nodedebug = require('child_process').fork(debuggerClientPath, args);
process.on('exit', function() {
nodedebug.kill('SIGTERM');
// Invoke fn if port is available.
var onPortAvailable = function(port, fn) {
var net = require('net');
var tester = net.connect({port: port}, function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check that the error is EADDRINUSE?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not an error. I'm testing that I can connect to it and fail if so.

I tried something like https://gist.github.com/timoxley/1689041 before, where I'm test creating a server to check for EADDRINUSE. However, that does not fail for some reason, until I try to create the debug process.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, I meant something like this: https://github.com/SeleniumHQ/selenium/blob/master/javascript/node/selenium-webdriver/net/portprober.js#L159

Except we don't actually want to create a server, so the method you had is the best I can think of.

console.error('Port ' + port + ' is already in use. Please specify ' +
'another port to debug.');
process.exit(1);
});
});
tester.once('error', function (err) {
if (err.code === 'ECONNREFUSED') {
tester.once('close', fn).end();
} else {
console.error('Unexpected failure testing for port ' + port + ': ',
err);
process.exit(1);
}
});
};

var vm_ = require('vm');
var flow = webdriver.promise.controlFlow();

var context = { require: require };
for (var key in global) {
context[key] = global[key];
}
context.list = function(locator) {
global.list = function(locator) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? I think it's safer to avoid polluting global.list

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise autocomplete doesn't suggest it, since autocomplete doesn't know about context.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I don't think we need autocomplete to suggest it, since it's all of 4 characters, but I don't really care either way.

/* globals browser */
return browser.findElements(locator).then(function(arr) {
var found = [];
Expand All @@ -700,9 +696,33 @@ Protractor.prototype.initDebugger_ = function(debuggerClientPath, opt_debugPort)
return found;
});
};
for (var key in global) {
context[key] = global[key];
}
var sandbox = vm_.createContext(context);

var browserUnderDebug = this;
flow.execute(function() {
log.puts('Starting WebDriver debugger in a child process. Pause is ' +
'still beta, please report issues at github.com/angular/protractor\n');
process.debugPort = opt_debugPort || process.debugPort;
onPortAvailable(process.debugPort, function() {
var args = [process.pid, process.debugPort];
if (browserUnderDebug.debuggerServerPort_) {
args.push(browserUnderDebug.debuggerServerPort_);
}
var nodedebug = require('child_process').fork(debuggerClientPath, args);
process.on('exit', function() {
nodedebug.kill('SIGTERM');
});
});
});

var pausePromise = flow.timeout(1000, 'waiting for debugger to attach')
.then(function() {
// Necessary for backward compatibility with node < 0.12.0
browserUnderDebug.executeScript_('', 'empty debugger hook');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wasn't this necessary before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because the way node debugger changed in 0.12.
Before, when you start the debug process, it starts a server but the program continues (that's why you needed the sleep() in there to manually stop execution while waiting for the debugger to attach). However, now, the moment you start debug, the program stops and waits for the reqContinue signal.

As a result of that, before, we call debug in this process. In the client, we take control once we attach.
Now, we call debug from the client. Because the program is paused, reqContinue is the first call we make. After that, we can take control as before.

However, if we're using node 0.10, the program isn't paused when issued the debug, so the first reqContinue actually makes the program jump to the next break point.

});

// Helper used only by debuggers at './debugger/modes/*.js' to insert code
// into the control flow.
Expand All @@ -718,7 +738,8 @@ Protractor.prototype.initDebugger_ = function(debuggerClientPath, opt_debugPort)
// A dummy repl server to make use of its completion function.
replServer_: require('repl').start({
input: {on: function() {}, resume: function() {}}, // dummy readable stream
output: {write: function() {}} // dummy writable stream
output: {write: function() {}}, // dummy writable stream
useGlobal: true
}),

// Execute a function, which could yield a value or a promise,
Expand Down Expand Up @@ -802,8 +823,6 @@ Protractor.prototype.initDebugger_ = function(debuggerClientPath, opt_debugPort)
return this.execPromiseResult_;
}
};

flow.timeout(1000, 'waiting for debugger to attach');
};

/**
Expand Down
6 changes: 6 additions & 0 deletions scripts/interactive_tests/interactive_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ test.addCommandExpectation('element(by.binding("nonexistent")).getText()',
'ERROR: NoSuchElementError: No element found using locator: ' +
'by.binding("nonexistent")');

// Check global `list` works.
test.addCommandExpectation('list(by.binding("greeting"))', '[ \'Hiya\' ]');
test.addCommandExpectation('list(by.binding("nonexistent"))', '[]');

// Check complete calls
test.addCommandExpectation('\t',
'[["element(by.id(\'\'))","element(by.css(\'\'))",' +
Expand All @@ -42,6 +46,8 @@ test.addCommandExpectation('\t',
'"element(by.className(\'\'))"],""]');
test.addCommandExpectation('ele\t', '[["element"],"ele"]');
test.addCommandExpectation('br\t', '[["break","","browser"],"br"]');
// Make sure the global 'list' we added shows up.
test.addCommandExpectation('li\t', '[["list"],"li"]');

test.run();

3 changes: 2 additions & 1 deletion scripts/interactive_tests/interactive_test_util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ exports.InteractiveTest = function(interactiveServerStartCmd, port) {
};
return verifyAll(0);
}).fin(function() {
client.sendCommand(0x1D); // '^]' This is the term signal.
// '^]' This is the term signal.
client.sendCommand(String.fromCharCode(0x1D));
client.disconnect();
});
}).done();
Expand Down