-
Notifications
You must be signed in to change notification settings - Fork 669
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.
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
Add the capability to get browser console messages (closes #1738) #1818
Add the capability to get browser console messages (closes #1738) #1818
Conversation
d965b8e
to
568cb32
Compare
❌ Tests for the commit 568cb32 have failed. See details: |
ts-defs/index.d.ts
Outdated
interface ConsoleMessagesCollection { | ||
/** | ||
* TODO: | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/**
* Messages output to the browser console by the console.log() method.
*/
ts-defs/index.d.ts
Outdated
log: string[], | ||
/** | ||
* TODO: | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/**
* Warning messages output to the browser console by the console.warn() method.
*/
ts-defs/index.d.ts
Outdated
/** | ||
* TODO: | ||
*/ | ||
error: string[], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/**
* Error messages output to the browser console by the console.error() method.
*/
ts-defs/index.d.ts
Outdated
/** | ||
* TODO: | ||
*/ | ||
info: string[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/**
* Information messages output to the browser console by the console.info() method.
*/
ts-defs/index.d.ts
Outdated
@@ -1005,6 +1024,10 @@ interface TestController { | |||
*/ | |||
getNativeDialogHistory(): Promise<NativeDialogHistoryItem[]>; | |||
/** | |||
* TODO: Returns a collection of browser console messages. | |||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/**
* Returns an object that contains messages output to the browser console.
*
* This object exposes the following properties.
*
* -- `log` contains log messages written by the console.log() method
* -- `error` contains error messages written by the console.error() method
* -- `warn` contains warning messages written by the console.warn() method
* -- `info` contains information messages written by the console.info() method
*/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to list the return object's properties here? If not, just remove them.
❌ Tests for the commit e70563e have failed. See details: |
@@ -0,0 +1,39 @@ | |||
fixture `Double Click`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's rename fixture
@@ -1,6 +1,6 @@ | |||
/// <reference path="../../../../../ts-defs/index.d.ts" /> | |||
import { Selector, ClientFunction } from 'testcafe'; | |||
import { expect } from 'chai'; | |||
import {Selector, ClientFunction} from 'testcafe'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong formating
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
❌ Tests for the commit 9889217 have failed. See details: |
src/client/driver/driver.js
Outdated
} | ||
|
||
set consoleMessages (messages) { | ||
return this.contextStorage.setItem(CONSOLE_MESSAGES, messages); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ehmm, we can run out of storage quite quickly with this approach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought about this.. Now the max size of the session storage in browsers is 5-10MB, so it looks not so crusial since we reset the session storage for each test run. But I agree, it would be better to keep the messages on the server.
✅ Tests for the commit 9889217 have passed. See details: |
✅ Tests for the commit e970116 have passed. See details: |
6b90e32
to
c4358a6
Compare
FPR /cc @helen-dikareva @AndreyBelym @inikulin |
❌ Tests for the commit c4358a6 have failed. See details: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
❌ Tests for the commit c4358a6 have failed. See details: |
Thanks for that adjustment 👍 |
…#1738) (DevExpress#1818) * Add the capability to get browser console messages (closes DevExpress#1738) * Rename to `getBrowserConsoleMessages` * Address Helen's remarks * Fix the test for node 4 * Store messages on the server * Integrating with Roles. Refactoring. * Fix tests.
Feature summary
API
Some browsers can write messages to the console with a little bit different formatting. We just call
toString()
for arguments that were passed to the console methods (see an example in the test)Real-World Use Case
Check your react application doesn't have prop type errors: