Skip to content
This repository has been archived by the owner on Nov 16, 2021. It is now read-only.

Replacing selenium with phantomjs. #80

Merged
merged 3 commits into from Jan 6, 2016

Conversation

grasmash
Copy link
Contributor

@grasmash grasmash commented Jan 5, 2016

No description provided.

@grasmash
Copy link
Contributor Author

grasmash commented Jan 5, 2016

This test failure appears to be unrelated. Is the media feature broken upstream?

@phenaproxima
Copy link
Collaborator

Judging by Travis' output, it looks like there might be a syntax error in composer.json?

@grasmash
Copy link
Contributor Author

grasmash commented Jan 5, 2016

@phenaproxima There was, yes. It's been fixed. There is now a failing media test in this build: https://travis-ci.org/acquia/lightning/builds/100441364

@phenaproxima
Copy link
Collaborator

That is one hell of a weird error; as far as I can tell, the x-factor appears to be the introduction of PhantomJS. Maybe PhantomJS is not compatible with the tests, or with our JS? (We are using a few relatively new techniques, like ES6 Promises -- although we have a polyfill for that.)

@grasmash
Copy link
Contributor Author

grasmash commented Jan 6, 2016

Yeah, it's totally possible that PhantomJS doesn't play well with that. My main goal in this PR is to reduce the noise in the logs created by Selenium. A secondary goal is to make testing faster by making it headless.

@balsama balsama merged commit a4a0fc4 into acquia:8.x-1.x Jan 6, 2016
@balsama
Copy link
Contributor

balsama commented Jan 6, 2016

I think we had a few things going on here. Foremost, the media_library.feature was failing periodically (~20%). When we introduced phantom, it was failing ~80% of the time. Introducing a pause before trying to click on the media WYSIWYG button fixed this so I think we were runninginto an issue where CKEditor wasn't fully loaded some of the time (more often with phantom because it's faster(?)).

Once I introduced that 1-second delay, everything passed locally 100% of the time using both selenium or phantomjs. But Travis was failing with a new error on the media_library.feature:

And I execute the "media_library" command in CKEditor "edit-body-0-value" # LightningSubContext::iExecuteTheCommandInCkeditor()
      {"errorMessage":"'undefined' is not a function (evaluating 'this.onTabActivate.bind(this)')","request":{"headers":{"Accept":"application/json;charset=UTF-8","Content-Length":"99","Content-Type":"application/json;charset=UTF-8","Host":"localhost:4444"},"httpVersion":"1.1","method":"POST","post":"{\"script\":\"return CKEDITOR.instances['edit-body-0-value'].execCommand('media_library');\",\"args\":[]}","url":"/execute","urlParsed":{"anchor":"","query":"","file":"execute","directory":"/","path":"/execute","relative":"/execute","port":"","host":"","password":"","user":"","userInfo":"","authority":"","protocol":"","source":"/execute","queryKey":{},"chunks":["execute"]},"urlOriginal":"/session/50d1c440-b4ab-11e5-afcd-4d40947bd52d/execute"}} (WebDriver\Exception\UnknownError)

I swapped selenium back in and travis is happy.

I'm happy to switch to phantomjs still (agree the cleaner logs are nice) but I'm not sure what's causing the above error. So, for now, we;re back to selenium.

@grasmash
Copy link
Contributor Author

grasmash commented Feb 3, 2016

We're looking at including Lightning in all of our project by default, but we need resolve the issue with media_library that you encountered, since we use PhantomJS in PS.

@balsama
Copy link
Contributor

balsama commented Feb 3, 2016

Great. We can re-prioritize this. It was a dead end for me if I remember correctly - couldn't find any clues as to why it was failing on travis specifically. Maybe some fresh eyes will help. I or @phenaproxima will take another look before the end of the week. Thanks.

@grasmash
Copy link
Contributor Author

grasmash commented Feb 3, 2016

It looks like this is due to PhantomJS not having the prototype.bind() function that CKEditor relies on. This has been solved with non-PHP implementation of PhantomJS, gotta look into a solution for us.

See:

Possible solution:
https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_objects/Function/bind#Polyfill

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants