Console errors and warning plugin #1916

el-davo · 2015-03-10T22:02:33Z

My first attempt at writing a plugin. Its usage is meant to fail when there are errors and warnings on the console of the browser.. Let me know what you guys think

googlebot · 2015-03-10T22:02:34Z

Thanks for your pull request.

It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits.

Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name.

tullmann · 2015-03-10T22:23:15Z

plugins/console/index.js

+ */
+ConsolePlugin.logMessages = function (warnings, errors) {
+    warnings.map(function (warning) {
+        console.error(warning.level.name + ': ' + warning.message);


Use "console.warn" here?

tullmann · 2015-03-10T22:31:47Z

This looks handy to me. We do something similar with scraping through the browser logs.

I'm also not entirely confident about the q/protractor.promise thing, so you might wait for confirmation from someone with a clue about that.

In my experience Firefox browser often spits out a pile of odd/useless log messages at startup, so I need to suppress the first batch of messages. (Otherwise the first test gets blamed for a bunch of stuff it didn't do.)

And, I also have a list of messages that I'm not interested in seeing from Firefox. (We see a lot of "Ruleset ignored due to bad selector" messages, which is probably our app). So, it might be worth adding the ability to provide the plugin with a list of messages to ignore.

juliemr · 2015-03-10T23:15:21Z

I said this above, but I know that 'comments on outdated changes' get hard to see, so copying:

It's super confusing, but Protractor actually uses both q and the webdriver promises. I think it's OK to use q in this situation, because it has no need to sync up with the webdriver promises - and it's actually probably safer, because it can't accidentally interfere with the webdriver control flow.

juliemr · 2015-03-10T23:16:03Z

plugins/console/index.js

+var testOut = {failedCount: 0, specResults: []};
+
+/**
+ * This plugin scans the log after each test and can fail on error and warning messages


Clarify that it scans the 'browser log'

juliemr · 2015-03-10T23:45:02Z

Thanks, I think this is generally useful and should definitely be included! I've added a couple comments about the output and some general style nitpicks, could you take a look at those?

el-davo · 2015-03-11T08:39:38Z

Thanks for the great feedback guys. Will get the updates in asap

googlebot · 2015-03-11T08:39:40Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

googlebot · 2015-03-11T15:14:02Z

CLAs look good, thanks!

sjelin · 2015-03-11T21:05:30Z

plugins/console/index.js

+ */
+ConsolePlugin.parseLog = function(config) {
+  var self = this;
+  var deferred = q.defer();


You don't need this deferred. Just return the promise you're currently resolving it inside. That is:

return this.getBrowserLog().then(function(log) { ... });

Good idea will give it a shot

sjelin · 2015-03-11T21:50:47Z

plugins/console/index.js

+    testOut.failedCount += (warnings.length > 0 && failOnWarning) ? 1 : 0;
+    testOut.failedCount += (errors.length > 0 && failOnError) ? 1 : 0;
+
+    if(testOut.failedCount > 0) {


I think you want to output the log regardless don't you?

sjelin · 2015-03-12T21:28:35Z

Nice! I'll merge this in later today

el-davo · 2015-03-13T08:13:19Z

Awesome and thanks for the help

sjelin · 2015-03-16T22:26:18Z

Merged in at 0351a10

jimmyeisenhauer · 2015-03-18T22:29:50Z

Curious if you will be adding documentation for this plugin to the plugin docs. Also does this work with all browsers protractor works with?

sjelin · 2015-03-18T22:46:43Z

Good point James! I'll add documentation ASAP. Not sure if Travis automatically tested this in all browsers, but I imagine that it did and everything works fine.

alecxe · 2015-03-22T18:09:01Z

Thank you very much for the feature.

From what I understand, the plugin would not work on Internet Explorer since browser.manage().logs() is not yet supported, see [IE] Add support for fetching logs using the webdriver.manage().logs() mechanism.

I've done smth similar before and I had to skip IE, see Handling Unknown Errors in protractor.

alecxe · 2015-03-22T18:41:51Z

Also How can I get hold of the browser's console? section of the FAQ should be updated and mention the new console plugin.

el-davo · 2015-03-23T12:59:26Z

Hi Alex. That's interesting to know that it doesn't work on Internet explorer. I did not realize that. In our company we don't currently run protractor tests against ie, only chrome and firefox.I had a lot of headaches trying to make the tests run stable on ie. Hopefully support will be added soon for i.e. I could put in the suggested fix you mentioned here - http://stackoverflow.com/questions/27587571/handling-unknown-errors-in-protractor But is there any point as you could just leave the plugin out of your internet explorer config?.

alecxe · 2015-03-23T13:22:09Z

@el-davo thanks for the quick response. For us it is not a big deal too at the current project, since currently we are not automating IE, but we'll do that in the future - as a workaround, we can apply the fix suggested in that SO topic, or have a separate config. We should at least document that the plugin would not work on IE, or implicitly skip checking the logs if running against IE, or may be issue a warning..what do you think?

Also, let me know if I should create a separate issue for the following suggestion: is it possible to group the console errors and warnings that have the same text and stack trace? At the moment the test output I get has about a hundred repeated stack trace blocks, since every test I run detects the same error message on the console. Hope it is clear what I'm trying to say. Thanks.

el-davo · 2015-03-23T13:37:05Z

Hi Alex

I like that suggestion of not having the same stack traces. No need to create a seperate issue. I can look into that in the next few days. Maybe we can do what they do in the chrome dev tools and have a counter that increases if the message is the same. For example if i have an error

WARNING (2 times) this is a warning message, line 32, column 4

Will try and add a readme also explicitly stating that ie. is a no go for now :)

alecxe · 2015-03-23T17:31:05Z

@el-davo exactly! Would definitely make the plugin better. Thank you, let me know if you need any help.

Speaking about the IE logging problems, the timeline plugin has it documented here and here. We can have smth similar for console plugin.

cscleison · 2015-03-26T11:43:48Z

From what i could see all tests fail in the event of a console error. Don't you think would be great to check console changes after each test in order to find out which interaction caused the error?

Right now we have a website with a lot of fields not localized yet so we have a lot of INFO entries in console. It gets so huge that I'm getting Java heap space errors when the plugin tries to print it. Maybe a parameter to avoid the print...

[launcher] Error: UnknownError: Java heap space
    at new bot.Error (C:\Users\USER\AppData\Roaming\npm\node_modules\protracto
r\node_modules\selenium-webdriver\lib\atoms\error.js:113:18)
    at Object.bot.response.checkResponse (C:\Users\USER\AppData\Roaming\npm\no
de_modules\protractor\node_modules\selenium-webdriver\lib\atoms\response.js:106:
9)
    at C:\Users\USER\AppData\Roaming\npm\node_modules\protractor\node_modules\
selenium-webdriver\lib\webdriver\webdriver.js:377:20
    at [object Object].promise.ControlFlow.runInFrame_ (C:\Users\USER\AppData\
Roaming\npm\node_modules\protractor\node_modules\selenium-webdriver\lib\webdrive
r\promise.js:1877:20)
    at [object Object].promise.Callback_.goog.defineClass.notify (C:\Users\CSTEI
N\AppData\Roaming\npm\node_modules\protractor\node_modules\selenium-webdriver\li
b\webdriver\promise.js:2464:25)
    at [object Object].promise.Promise.notify_ (C:\Users\USER\AppData\Roaming\
npm\node_modules\protractor\node_modules\selenium-webdriver\lib\webdriver\promis
e.js:563:12)
    at Array.forEach (native)
    at Object.goog.array.forEach (C:\Users\USER\AppData\Roaming\npm\node_modul
es\protractor\node_modules\selenium-webdriver\lib\goog\array\array.js:203:43)
    at [object Object].promise.Promise.notifyAll_ (C:\Users\USER\AppData\Roami
ng\npm\node_modules\protractor\node_modules\selenium-webdriver\lib\webdriver\pro
mise.js:552:16)
    at goog.async.run.processWorkQueue (C:\Users\USER\AppData\Roaming\npm\node
_modules\protractor\node_modules\selenium-webdriver\lib\goog\async\run.js:125:21
)
[launcher] Process exited with error code 100

el-davo and others added 4 commits March 6, 2015 08:25

Added new console plugin

b65159f

Added specs

0fd503d

Added docs

3e9d669

Reverted port change

1fb5b46

googlebot added the cla: no label Mar 10, 2015

tullmann reviewed Mar 10, 2015
View reviewed changes

juliemr reviewed Mar 10, 2015
View reviewed changes

juliemr self-assigned this Mar 10, 2015

el-davo added 2 commits March 11, 2015 08:50

Removed .idea from git ignore file

200eae8

Updated formatting as suggested in pull request

8436f83

googlebot added cla: yes and removed cla: no labels Mar 11, 2015

el-davo added 3 commits March 11, 2015 16:05

Updated to log to the specResults instead of console.log

f953412

Reverted change to the port

41590bb

Fixed logic to make the tests pass and fail

2ff7192

sjelin reviewed Mar 11, 2015
View reviewed changes

el-davo added 2 commits March 11, 2015 21:47

Updates from pull request

e72bd90

Updates from pull request

65ba682

sjelin reviewed Mar 11, 2015
View reviewed changes

el-davo added 4 commits March 11, 2015 22:02

Added exact failed count number

af97467

Added specs

1bf7ab9

Revert to environment.js formatting

a4d4d10

Update to conform to spec results

8ea8466

sjelin added pr: LGTM and removed pr: needs review labels Mar 12, 2015

sjelin closed this Mar 16, 2015

juliemr mentioned this pull request Mar 17, 2015

Add option to automatically redirect browser logs #1234

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Console errors and warning plugin #1916

Console errors and warning plugin #1916

el-davo commented Mar 10, 2015

googlebot commented Mar 10, 2015

tullmann Mar 10, 2015

tullmann commented Mar 10, 2015

juliemr commented Mar 10, 2015

juliemr Mar 10, 2015

juliemr commented Mar 10, 2015

el-davo commented Mar 11, 2015

googlebot commented Mar 11, 2015

googlebot commented Mar 11, 2015

sjelin Mar 11, 2015

el-davo Mar 11, 2015

sjelin Mar 11, 2015

sjelin commented Mar 12, 2015

el-davo commented Mar 13, 2015

sjelin commented Mar 16, 2015

jimmyeisenhauer commented Mar 18, 2015

sjelin commented Mar 18, 2015

alecxe commented Mar 22, 2015

alecxe commented Mar 22, 2015

el-davo commented Mar 23, 2015

alecxe commented Mar 23, 2015

el-davo commented Mar 23, 2015

alecxe commented Mar 23, 2015

cscleison commented Mar 26, 2015

Console errors and warning plugin #1916

Console errors and warning plugin #1916

Conversation

el-davo commented Mar 10, 2015

googlebot commented Mar 10, 2015

tullmann Mar 10, 2015

Choose a reason for hiding this comment

tullmann commented Mar 10, 2015

juliemr commented Mar 10, 2015

juliemr Mar 10, 2015

Choose a reason for hiding this comment

juliemr commented Mar 10, 2015

el-davo commented Mar 11, 2015

googlebot commented Mar 11, 2015

googlebot commented Mar 11, 2015

sjelin Mar 11, 2015

Choose a reason for hiding this comment

el-davo Mar 11, 2015

Choose a reason for hiding this comment

sjelin Mar 11, 2015

Choose a reason for hiding this comment

sjelin commented Mar 12, 2015

el-davo commented Mar 13, 2015

sjelin commented Mar 16, 2015

jimmyeisenhauer commented Mar 18, 2015

sjelin commented Mar 18, 2015

alecxe commented Mar 22, 2015

alecxe commented Mar 22, 2015

el-davo commented Mar 23, 2015

alecxe commented Mar 23, 2015

el-davo commented Mar 23, 2015

alecxe commented Mar 23, 2015

cscleison commented Mar 26, 2015