Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(debugger): fix issue where output does not display circular dep and ... #1889

Closed
wants to merge 1 commit into from
Closed

fix(debugger): fix issue where output does not display circular dep and ... #1889

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
22 changes: 15 additions & 7 deletions lib/debugger/modes/commandRepl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,13 @@ var CommandRepl = function(client) {
CommandRepl.prototype.stepEval = function(expression, callback) {
expression = expression.replace(/"/g, '\\\"');
var expr = 'browser.dbgCodeExecutor_.execute("' + expression + '")';
this.evaluate_(expr, callback);
this.evaluate_(expr, function(err, res) {
// Result is a string representation of the evaluation.
if (res !== undefined) {
console.log(res);
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? Do we need an extra log here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before I was using JSON.stringify to serialize the data between protractor and the commandRepl, which would parse the string. However, this was not good because you can't serialize functions and objects with circular dependencies.

The only way to stringify the full object mimic what console.log does, which is to use util.format (see https://github.com/joyent/node/blob/v0.12.0/lib/console.js#L55).

So then, lastly, why I can't do callback(err, res), but must instead do console.log(res); callback(err, undefined). It's because during callback, nodejs doesn't console.log the result, but will rather print the result as is.

Here are some outputs if I do callback(err, res):

> browser
'{ controlFlow: [Function],\n  schedule: [Function],\n  getSession: [Function],\n  getCapabilities: [Function],\n  quit: [Function],\n  actions: [Function],\n  executeScript: [Function],\n  executeAsyncScript: [Function],\n  call: [Function],\n  wait: [Function],\n  sleep: [Function],\n  getWindowHandle: [Function],\n  getAllWindowHandles: [Function],\n  getPageSource: [Function],\n  close: [Function],\n  getCurrentUrl: [Function],\n  getTitle: [Function],\n  findElementInternal_: [Function],\n  findDomElement_: [Function],\n  findElementsInternal_: [Function],\n  takeScreenshot: [Function],\n  manage: [Function],\n  switchTo: [Function],\n  driver: \n   { session_: \n      { then: [Function: then],\n        cancel: [Function: cancel],\n        isPending: [Function: isPending] },\n     executor_: { execute: [Function] },\n     flow_: \n      { events_: {},\n        timer: [Object],\n        history_: [],\n        activeFrame_: [Object],\n        schedulingFrame_: [Object],\n        shutdownId_: null,\n        eventLoopId_: [Object],\n        pendingRejections_: 0,\n        numAbortedFrames_: 0 } },\n  element: { [Function] all: [Function] },\n  \'$\': [Function],\n  \'$$\': [Function],\n  baseUrl: \'http://www.angularjs.org\',\n  rootEl: \'body\',\n  ignoreSynchronization: false,\n  getPageTimeout: 10000,\n  params: {},\n  resetUrl: \'data:text/html,<html></html>\',\n  mockModules_: [ { name: \'protractorBaseModule_\', script: [Function], args: [] } ],\n  getProcessedConfig: [Function],\n  forkNewDriverInstance: [Function],\n  dbgCodeExecutor_: \n   { execPromise_: \n      { then: [Function: then],\n        cancel: [Function: cancel],\n        isPending: [Function: isPending] },\n     execPromiseResult_: undefined,\n     execPromiseError_: undefined,\n     replServer_: \n      { domain: null,\n        _events: {},\n        _maxListeners: 10,\n        useGlobal: false,\n        ignoreUndefined: false,\n        eval: [Function],\n        inputStream: [Object],\n        outputStream: [Object],\n        lines: [Object],\n        context: [Object],\n        bufferedCommand: \'\',\n        prompt: \'> \',\n        rli: [Object],\n        commands: [Object],\n        writer: [Object],\n        useColors: false },\n     execute_: [Function],\n     execute: [Function],\n     complete: [Function],\n     resultReady: [Function],\n     getResult: [Function] } }'
> element
'function (locator) {\n    return new ElementArrayFinder(ptor).all(locator).toElementFinder_();\n  }'

}
callback(err, undefined);
});
};

/**
Expand All @@ -47,7 +53,11 @@ CommandRepl.prototype.complete = function(line, callback) {
} else {
line = line.replace(/"/g, '\\\"');
var expr = 'browser.dbgCodeExecutor_.complete("' + line + '")';
this.evaluate_(expr, callback);
this.evaluate_(expr, function(err, res) {
// Result is a JSON representation of the autocomplete response.
var result = res === undefined ? undefined : JSON.parse(res);
callback(err, result);
});
}
};

Expand Down Expand Up @@ -77,16 +87,14 @@ CommandRepl.prototype.evaluate_ = function(expression, callback) {
command: 'evaluate',
arguments: {
frame: 0,
maxStringLength: 2000,
maxStringLength: -1,
expression: 'browser.dbgCodeExecutor_.getResult()'
}
}, function(err, res) {
try {
var result = res.value === undefined ?
undefined : JSON.parse(res.value);
callback(err, result);
callback(err, res.value);
} catch (e) {
callback(e, null);
callback(e, undefined);
}
self.client.removeListener('break', onbreak_);
});
Expand Down
37 changes: 25 additions & 12 deletions lib/protractor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var util = require('util');
var url = require('url');
var webdriver = require('selenium-webdriver');
var helper = require('./util');
Expand Down Expand Up @@ -698,14 +699,7 @@ Protractor.prototype.initDebugger_ = function(debuggerClientPath, opt_debugPort)
self.execPromiseResult_ = self.execPromiseError_ = undefined;

self.execPromise_ = self.execPromise_.
then(function() {
var result = execFn_();
if (webdriver.promise.isPromise(result)) {
return result.then(function(val) {return val;});
} else {
return result;
}
}).then(function(result) {
then(execFn_).then(function(result) {
self.execPromiseResult_ = result;
}, function(err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the reason for this in the past? It doesn't appear to do anything

Copy link
Contributor Author

Choose a reason for hiding this comment

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

element(by.xyz).getText() is an elementFinder/promise.
When someone enters element(by.xyz).getText(), we need to return the text value of the elementFinder (i.e. by calling then) instead of the text representation of the elementFinder.

self.execPromiseError_ = err;
Expand All @@ -720,22 +714,42 @@ Protractor.prototype.initDebugger_ = function(debuggerClientPath, opt_debugPort)
},

// Execute a piece of code.
// Result is a string representation of the evaluation.
execute: function(code) {
var execFn_ = function() {
// Run code through vm so that we can maintain a local scope which is
// isolated from the rest of the execution.
return vm_.runInThisContext(code);
var res = vm_.runInThisContext(code);
if (!webdriver.promise.isPromise(res)) {
res = webdriver.promise.fulfilled(res);
}

return res.then(function(res) {
if (res === undefined) {
return undefined;
} else {
// The '' forces res to be expanded into a string instead of just
// '[Object]'. Then we remove the extra space caused by the '' using
// substring.
return util.format.apply(this, ['', res]).substring(1);
}
});
};
this.execute_(execFn_);
},

// Autocomplete for a line.
// Result is a JSON representation of the autocomplete response.
complete: function(line) {
var self = this;
var execFn_ = function() {
var deferred = webdriver.promise.defer();
self.replServer_.complete(line, function(err, res) {
deferred.fulfill(res, err);
if (err) {
deferred.reject(err);
} else {
deferred.fulfill(JSON.stringify(res));
}
});
return deferred;
};
Expand All @@ -756,8 +770,7 @@ Protractor.prototype.initDebugger_ = function(debuggerClientPath, opt_debugPort)
if (this.execPromiseError_) {
throw this.execPromiseError_;
}

return JSON.stringify(this.execPromiseResult_);
return this.execPromiseResult_;
}
};

Expand Down