Implement `ConsoleSandbox`. Add the `consoleMethCalled` event. #1312

AlexanderMoskovkin · 2017-09-14T13:57:02Z

miherlosev · 2017-09-14T14:17:56Z

src/client/sandbox/console.js

+        const sandbox = this;
+
+        const proxyConsoleMeth = meth => {
+            sandbox.window.console[meth] = function () {


In a new code we use the spread operator for an argument cloning
see
https://github.com/DevExpress/testcafe-hammerhead/blob/master/src/client/sandbox/node/document/index.js#L107
https://github.com/DevExpress/testcafe-hammerhead/blob/master/src/client/sandbox/node/window.js#L300
https://github.com/DevExpress/testcafe-hammerhead/blob/master/src/client/sandbox/event/index.js#L98

So, rewrite as

sandbox.window.console[meth] = (...args) => { sandbox.emit(sandbox.CONSOLE_METH_CALLED, { meth, args: args}); sandbox.nativeMethods.consoleMeths[meth].apply(sandbox.nativeMethods.console, args); }

miherlosev · 2017-09-14T14:25:19Z

src/client/index.js

@@ -54,7 +54,8 @@ class Hammerhead {
            beforeXhrSend:           this.sandbox.xhr.BEFORE_XHR_SEND_EVENT,
            fetchSent:               this.sandbox.fetch.FETCH_REQUEST_SENT_EVENT,
            pageNavigationTriggered: this.pageNavigationWatch.PAGE_NAVIGATION_TRIGGERED_EVENT,
-            scriptElementAdded:      this.sandbox.node.element.SCRIPT_ELEMENT_ADDED_EVENT
+            scriptElementAdded:      this.sandbox.node.element.SCRIPT_ELEMENT_ADDED_EVENT,
+            consoleMethCalled:       this.sandbox.console.CONSOLE_METH_CALLED


this.sandbox.console.CONSOLE_METH_CALLED -> this.sandbox.console.CONSOLE_METH_CALLED_EVENT

miherlosev · 2017-09-14T14:26:50Z

src/client/sandbox/console.js

+    constructor () {
+        super();
+
+        this.CONSOLE_METH_CALLED = 'hammerhead|console|console-meth-called-event';


this.CONSOLE_METH_CALLED_EVENT = 'hammerhead|event|console-meth-called

miherlosev · 2017-09-14T14:32:29Z

src/client/sandbox/native-methods.js

+        // Console
+        this.console = win.console;
+
+        this.consoleMeths = {


We don't use an additional hierarchy level for similar cases.
See
https://github.com/DevExpress/testcafe-hammerhead/blob/master/src/client/sandbox/native-methods.js#L121
https://github.com/DevExpress/testcafe-hammerhead/blob/master/src/client/sandbox/native-methods.js#L176

Rewrite as

this.consoleLog = ... this.consoleWarn = ... etc

Why? Is there a significant reason for this? I believe it's better to leave it as is to use

sandbox.nativeMethods.consoleMeths[meth]

instead of

const nativeMeth = console + meth[0].toUpperCase() + meth.slice(1); sandbox.nativeMethods[nativeMeth];

Ok. If you use a console method as key then let you leave it as is.

miherlosev · 2017-09-14T14:39:46Z

test/client/fixtures/sandbox/console-test.js

@@ -0,0 +1,66 @@
+var consoleSandbox = hammerhead.sandbox.console;
+
+function argsToArray (args) {


Array.prototype.slice.call ?

miherlosev · 2017-09-14T15:04:43Z

test/client/fixtures/sandbox/console-test.js

+        originMethods[meth].apply(console, args);
+    }
+
+    window.console = {


To capture console trace you override methods that already overriden in sandbox.
Usually, we use an another way - we override the appropriate native methods.
See tests in https://github.com/DevExpress/testcafe-hammerhead/blob/master/test/client/fixtures/sandbox/node/methods-test.js
Use this way too.

miherlosev · 2017-09-14T15:08:42Z

test/client/fixtures/sandbox/console-test.js

+    window.console.info('7', '8');
+    /* eslint-enable no-console */
+
+    deepEqual(handledEvents, ['log', 'warn', 'error', 'info']);


handledConsoleMethodNames

miherlosev · 2017-09-14T15:09:24Z

test/client/fixtures/sandbox/console-test.js

+
+    deepEqual(handledEvents, ['log', 'warn', 'error', 'info']);
+    deepEqual(log, '12345678');
+    deepEqual(logFromEvents, '12345678');


handledConsoleMethodArgs

LavrovArtem · 2017-09-15T08:17:07Z

src/client/sandbox/console.js

+
+        const sandbox = this;
+
+        const proxyConsoleMeth = meth => {


Make this function as private member of ConsoleSandbox, I think it is more redable.

testcafe-build-bot · 2017-09-21T16:36:35Z

✅ Tests for the commit f87debf have passed. See details:

LavrovArtem · 2017-09-22T06:15:08Z

src/client/sandbox/console.js

+    attach (window) {
+        super.attach(window);
+
+        const sandbox = this;


remove this variable

AlexanderMoskovkin · 2017-09-22T11:18:21Z

FPR

miherlosev · 2017-09-22T11:28:06Z

src/client/sandbox/console.js

+    constructor () {
+        super();
+
+        this.CONSOLE_METH_CALLED_EVENT = 'hammerhead|console|console-meth-called';


Rename to 'hammerhead|event|console-meth-called'

miherlosev · 2017-09-22T11:31:22Z

src/client/sandbox/console.js

+            this._proxyConsoleMeth('info');
+            this._proxyConsoleMeth('error');
+            this._proxyConsoleMeth('warn');
+        }


Should we override the console.dir method too?

I guess not.

https://developer.mozilla.org/en-US/docs/Web/API/Console/dir

Non-standard
This feature is non-standard and is not on a standards track. Do not use it on production sites facing the Web: it will not work for every user.

miherlosev · 2017-09-22T11:36:30Z

src/client/sandbox/index.js

@@ -198,6 +200,7 @@ export default class Sandbox extends SandboxBase {
        this.node.attach(window);
        this.upload.attach(window);
        this.cookie.attach(window);
+        this.console.attach(window);


Should we reattach ConsoleSandbox after document.write is called?
See reattach method and DOCUMENT_CLEANED_EVENT event.

It seems, we should

miherlosev · 2017-09-22T14:10:11Z

test/client/fixtures/sandbox/console-test.js

@@ -47,4 +47,16 @@ if (window.console && typeof window.console.log !== 'undefined') {
            info:  originMethods.info
        };
    });
+
+    test('consoleMethCalled event after document.write', function () {


'consoleMethod' event should be raised after document.write

'consoleMethCalled' event...

AlexanderMoskovkin · 2017-09-26T10:52:22Z

FPR

testcafe-build-bot · 2017-09-26T11:21:14Z

❌ Tests for the commit d5afa2a have failed. See details:

AlexanderMoskovkin · 2017-09-26T11:46:19Z

@testcafe-build-bot \retest

testcafe-build-bot · 2017-09-26T12:10:51Z

✅ Tests for the commit d5afa2a have passed. See details:

AlexanderMoskovkin · 2017-09-26T12:13:08Z

I've rewritten things about iframes, take a look again
/cc @miherlosev @LavrovArtem

…press#1312) * Implement ConsoleSandbox. Add the consoleMethCalled event. * Address remarks * Address Artem's remark * Address Miher's remark * Update test title * Reattach sandbox on the document cleaned event * Send the consoleMethCalled method from iframes to the top window. * Fix the test for IE * Comment the test for IE

Implement ConsoleSandbox. Add the consoleMethCalled event.

1845fff

AlexanderMoskovkin requested review from miherlosev and LavrovArtem September 14, 2017 13:57

miherlosev requested changes Sep 14, 2017

View reviewed changes

LavrovArtem suggested changes Sep 15, 2017

View reviewed changes

Address remarks

f87debf

LavrovArtem suggested changes Sep 22, 2017

View reviewed changes

Address Artem's remark

5102dce

LavrovArtem approved these changes Sep 22, 2017

View reviewed changes

miherlosev requested changes Sep 22, 2017

View reviewed changes

Address Miher's remark

cf706f2

miherlosev requested changes Sep 22, 2017

View reviewed changes

AlexanderMoskovkin added 2 commits September 22, 2017 17:16

Update test title

8a32e28

Reattach sandbox on the document cleaned event

f65f9b3

LavrovArtem mentioned this pull request Sep 25, 2017

Hammerhead is not initialized in the rewrited iframe with same-domain url in IE #1325

Closed

AlexanderMoskovkin added 3 commits September 26, 2017 12:48

Send the consoleMethCalled method from iframes to the top window.

4266863

Fix the test for IE

8b679fc

Comment the test for IE

d5afa2a

LavrovArtem approved these changes Sep 26, 2017

View reviewed changes

miherlosev approved these changes Sep 26, 2017

View reviewed changes

miherlosev merged commit 47908e6 into DevExpress:master Sep 26, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `ConsoleSandbox`. Add the `consoleMethCalled` event. #1312

Implement `ConsoleSandbox`. Add the `consoleMethCalled` event. #1312

AlexanderMoskovkin commented Sep 14, 2017

miherlosev Sep 14, 2017 •

edited

Loading

miherlosev Sep 14, 2017

miherlosev Sep 14, 2017

miherlosev Sep 14, 2017

AlexanderMoskovkin Sep 20, 2017

miherlosev Sep 20, 2017

miherlosev Sep 14, 2017

miherlosev Sep 14, 2017

miherlosev Sep 14, 2017 •

edited

Loading

miherlosev Sep 14, 2017

LavrovArtem Sep 15, 2017

testcafe-build-bot commented Sep 21, 2017

LavrovArtem Sep 22, 2017

AlexanderMoskovkin Sep 22, 2017

AlexanderMoskovkin commented Sep 22, 2017

miherlosev Sep 22, 2017

miherlosev Sep 22, 2017

AlexanderMoskovkin Sep 22, 2017

miherlosev Sep 22, 2017

AlexanderMoskovkin Sep 22, 2017

miherlosev Sep 22, 2017

AlexanderMoskovkin Sep 22, 2017

miherlosev Sep 22, 2017

AlexanderMoskovkin commented Sep 26, 2017

testcafe-build-bot commented Sep 26, 2017

AlexanderMoskovkin commented Sep 26, 2017

testcafe-build-bot commented Sep 26, 2017

AlexanderMoskovkin commented Sep 26, 2017 •

edited

Loading

		@@ -0,0 +1,66 @@
		var consoleSandbox = hammerhead.sandbox.console;

		function argsToArray (args) {

Implement ConsoleSandbox. Add the consoleMethCalled event. #1312

Implement ConsoleSandbox. Add the consoleMethCalled event. #1312

Conversation

AlexanderMoskovkin commented Sep 14, 2017

miherlosev Sep 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miherlosev Sep 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

testcafe-build-bot commented Sep 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexanderMoskovkin commented Sep 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexanderMoskovkin commented Sep 26, 2017

testcafe-build-bot commented Sep 26, 2017

AlexanderMoskovkin commented Sep 26, 2017

testcafe-build-bot commented Sep 26, 2017

AlexanderMoskovkin commented Sep 26, 2017 • edited Loading

Implement `ConsoleSandbox`. Add the `consoleMethCalled` event. #1312

Implement `ConsoleSandbox`. Add the `consoleMethCalled` event. #1312

miherlosev Sep 14, 2017 •

edited

Loading

miherlosev Sep 14, 2017 •

edited

Loading

AlexanderMoskovkin commented Sep 26, 2017 •

edited

Loading