Problem/Motivation

We are using phantomjs for testing our javascript behaviour.
For the communication with phantomjs we are leveraging gastonjs, which is a port of poltergeist, a quite common library in the ruby and java world.

Poltergeist uses direct communication with phantomjs to control the browser.

An alternative approach is to leverage webdriver, a W3C API, which allows you to
control both phantomjs as well as actual browsers like firefox/chrome using selenium.

This would have the following advantages:

  • You can easier see what is happening in a real browser
  • It is more common than gastonJS to use

Proposed resolution

  1. Open an issue in the testbot issue queue to still use phantomjs, but using the --webdriver command #2857788: Patch the testbot to start both phantomjs in webdriver mode and phantomjs in non webdriver mode
  2. Use the mink selenium driver, instead of the gastonjs one.
  3. More details below
  4. Profit?

Remaining tasks

User interface changes

API changes

Data model changes

Testing if Drupal can run with webDriver [1] which allowed running tests under real Browsers.[2]

[1] Bahat official webDriver: http://mink.behat.org/en/latest/drivers/selenium2.html
[2] Testing with different browsers: http://www.seleniumhq.org/download/

Pre-setup:
1. Apply patch
2. `composer require behat/mink-selenium2-driver`

Testing (Firefox):
1. You can follow http://mink.behat.org/en/latest/drivers/selenium2.html to start selenium2

2. Make sure you have Firefox installed in system DEFAULT dir

3. Run PHPUnit test ( same as current system )
e.g.

.\vendor\bin\phpunit.bat -c core core\modules\simpletest\tests\src\FunctionalJavascript\BrowserWithJavascriptTest.php

@droplet created a new simple standard test to avoid Drupal issue

.\vendor\bin\phpunit.bat -c core core\modules\simpletest\tests\src\FunctionalJavascript\StandardTest.php

4. It started firefox and running tests without any problems.

Testing (Phantomjs):
1. Start `phantomjs --webdriver=4444`
(If you have selenium2, you can run via selenium2 also)

2. Change `firefox` to `phantomjs` in $this->minkDefaultDriverArgs
(@see my patch's comment)

3. Same as above step 3 ~ 4

@droplet reports:
Chrome & Firefox:
It run with current testbots system smoothly.

Phantomjs:
Current testbot framework:
It can run but crashed

Unknown errors with Session

$this->drupalGet('<front>');

Patched:
@see FunctionalJavascript\StandardTest.php

It worked!!

I don't familiar with Drupal testbots framework. Looking forward to your feedback from who worked for Drupal JavaScript testbots.

Comments

droplet created an issue. See original summary.

droplet’s picture

Issue summary: View changes
dawehner’s picture

Of course it works, this is the entire point of not using phantomJS directly, but rather a browser abstraction layer (mink).

Personally I just don't want to deal with fighting the testbots, they aren't really a moving target, nor are they particularly easy to deal wth

dawehner’s picture

Component: base system » phpunit

I don't want to sound bitter, but I think the time would be better invested in extending our test coverage in the javascript space first.

droplet’s picture

Of course it works, this is the entire point of not using phantomJS directly, but rather a browser abstraction layer (mink).

It would be more interesting after seeing the comments. Wasn't Selenium2Driver available at that point? I'm surprised how easy to run it with Drupal is.

With webDriver, it's easier to inspect via browser dev tools instead of many screenshots to guess how the `document` is. We will know how the test engine performs tests, not how we using Mouse & KB to simulate it when you write test cases..etc. For example in this case #2717629: Add filter group for a View fails. @Lendude told that my test case was wrong. But looking at my code, it's pretty straightforward and no errors. With browsers, we can see the visual result of each step in real-time. Dropping a breakpoint and inspect it and move to next point. It speeds up to write ( & review) testcase actually.

In another side, we will get better supports from active Behat & PHP WebDriver (Facebook) communities.

dawehner’s picture

It would be more interesting after seeing the comments. Wasn't Selenium2Driver available at that point? I'm surprised how easy to run it with Drupal is.

One reason we went with phantomjs: We wanted to check for HTTP status codes. This is not exposed by the selenium API but just by phantomjs directly.
As written before, the other reason is that getting a real browser onto the testbots would be a major effort.

But sure, feel free to spend the time and trying it out :) It would be great to use selenium actually, because selenium provides a selenium grid we need at
point to scale up the javascript tests.

dawehner’s picture

So yeah please open up an issue in the testbot queue. I would like to see what happens :)

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dawehner’s picture

@droplet
If you want to make this change, can you open up the corresponding testbot issue?

chx’s picture

I tried chromedriver and I can report this works to an extent.

I believe the click method might be slightly broken with chromedriver because #divFormBody > div.clsJspFormBody > div > table > tbody > tr:nth-child(3) > td > table:nth-child(1) > tbody > tr > td > a doesn't work but using //*[@id="divFormBody"]/div[2]/div/table/tbody/tr[3]/td/table[1]/tbody/tr/td/a does. It might be broken with more than chromedriver but my time debugging this is up. A separate issue might be in order.

dawehner’s picture

Issue summary: View changes

Here is a more constructive issue summary

dawehner’s picture

Status: Active » Needs review
FileSize
13.49 KB

I adapted the patch to be a bit more minimal, like removing gastonjs, adding selenium etc. and also adapting the documentation.

droplet’s picture

Fixed:
- Fixing set Cookie in PhantomJS
- Mute StatusCode in JSTests @see comments in interdiff.patch

Known issues:
- StatusCode needs further discussion.
- 2 or 3 tests failing. @see: Attached logs.txt

Notes:
- All errors are fixable from my reviews. Those should be improved to current testbots system also. I will send Patches to each Component issues threads soon.

The last submitted patch, 13: 2775653-13.patch, failed testing.

droplet’s picture

Same as #13. (Fixing the new file index)

dawehner’s picture

Note: The missing feature to be able to retrieve the HTTP status code was one of the reasons to use gastonjs instead of selenium. The same is true for HTTP headers
Isn't there a way to execute some JS to detect that information? IMHO this is something we should try to discuss, can we live with that missing feature, before we invest too much work.

droplet’s picture

Do you still remember the issue for "HTTP status code" discussion? Would like to understand the reason & background info. Thanks.
Is it because we use it as UnitTests (in future)?

Isn't there a way to execute some JS to detect that information?

Impossible since it isn't requested from JS. PhantomJS HTTP Status provided by PhantomJS API. Selenium rejected to implement the HTTP Status API in WebDriver. Use Proxy is an option..

dawehner’s picture

Not sure this was actually documented exactly, but this was just part of the research phase.
Well, the question is, is it good/right to not request the header/status code?

droplet’s picture

With HTTPStatus in JSTests, we may reduce duplicated / similar tests. (Lazy mode, hehe)

Umm. My first question is:

Can we assume the following conditions?
- Status 200 === Return good content, visible
- Status 400 === Error page, no visible

There're 13 usages in all CORE tests. (Less than I thoughts. It doesn't validate every page requests) IMO, most of them should replace with Text Asserts (even now). I meant we should verify "What We See". (better security)

And there're only 3 usages involved the JSTests:

- drupalLogin & drupalLogout (this is helper function, not testing Login & Logout API)
No idea why it's checking the status code before submission.

- SessionTest
"Status 200" === "session doesn't expire". Is it good?

Note:
- If we request from JS code, it can access the status code from XMLHttpRequest.
- REST-like API should use UnitTests that able to access StatusCode.
- getResponseHeader(), no usages in JSTests.

dawehner’s picture

We could also just simply ignore those assertions.

klausi’s picture

  1. +++ b/composer.json
    @@ -40,5 +40,8 @@
    +    },
    +    "require-dev": {
    +        "behat/mink-selenium2-driver": "^1.3"
         }
    

    this should go into core/composer.json

  2. +++ b/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php
    @@ -0,0 +1,33 @@
    +/**
    + * Phantomjs requried domain in setCookie.
    + */
    

    "requires a domain"

  3. +++ b/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php
    @@ -0,0 +1,33 @@
    +class DrupalSelenium2Driver extends Selenium2Driver
    +{
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public function setCookie($name, $value = null)
    +  {
    

    { should be on the same line

  4. +++ b/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php
    @@ -0,0 +1,33 @@
    +    if (null === $value) {
    

    we don't use yoda conditions in drupal, so the null should be on the other side.

  5. +++ b/core/tests/Drupal/FunctionalJavascriptTests/JSWebAssert.php
    @@ -28,4 +28,26 @@ public function assertWaitOnAjaxRequest($timeout = 10000, $message = 'Unable to
    +  public function statusCodeEquals($code)
    +  {
    +    // should we throw an error?
    +  }
    

    Yes, I think we should throw a UnsupportedDriverActionException here. Which means we will have to implement our own drupalLogin() method on JavascriptTestBase.

    But silently leaving this method empty is a bad idea because then the test will pass although the developer wanted to assert the status code. The test should rather fail instead and tell the developer that this does not work.

    Which means this is a potential breaking change for all people using JavascriptTestBase right now.

    Which means we need to create a new JavascriptSeleniumTestBase class which has this behavior, while the old JavascriptTestBase stays as is.

    Then we would need to document 2 ways how people need to run PhantomJS, which is confusing. And the testbot would have to run PhantomJS in 2 variants?

dawehner’s picture

Which means we need to create a new JavascriptSeleniumTestBase class which has this behavior, while the old JavascriptTestBase stays as is.

Then we would need to document 2 ways how people need to run PhantomJS, which is confusing. And the testbot would have to run PhantomJS in 2 variants?

Well, we could silently ignore failures in the status code ... given that they should not have been tested using JTB in the first place.

alexpott’s picture

@dawehner pinged me for my thoughts on this issue.

  • I'm +1 to the direction - it makes perfect sense
  • I think we should do a hard break on statusCodeEquals and throw an exception JTB is experimental and supporting multiple versions is way more expensive. We could change drupalLogin to not use it but assert something else or just remove it since if the form fails it fails.
dawehner’s picture

I think we should do a hard break on statusCodeEquals and throw an exception JTB is experimental and supporting multiple versions is way more expensive. We could change drupalLogin to not use it but assert something else or just remove it since if the form fails it fails.

Ideally we would do that already I'd say: #2827014: Throw an exception when testing non response body in javascript tests

klausi’s picture

Status: Needs review » Needs work

Patch does not apply anymore, would be nice to have something working here for testing.

klausi’s picture

Status: Needs work » Needs review
FileSize
32.07 KB

After having another bad day with PhantomJS having a real browser sounds more and more appealing. Just a reroll for now, I can execute a JS test locally with this.

We should figure out how easy it is to install firefox or chrome on the testbot and how to get PhantomJS running in webdriver mode on port 4444 there.

dawehner’s picture

how to get PhantomJS running in webdriver mode on port 4444 there.

I guess a viable approach would be:

  • patch the testbot to start both phantomjs in webdriver mode and phantomjs in non webdriver mode
  • Deploy this patch and wait until 8.2.x is not longer supported
  • Remove the old phantomjs instance

Status: Needs review » Needs work

The last submitted patch, 26: selenium-driver-2775653-26.patch, failed testing.

juampynr’s picture

Status: Needs work » Needs review
FileSize
32.09 KB

Re-rolling.

Status: Needs review » Needs work

The last submitted patch, 29: javascripttests_with-2775653-29.patch, failed testing.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jibran’s picture

+++ b/core/tests/Drupal/FunctionalJavascriptTests/JavascriptTestBase.php
@@ -16,27 +16,19 @@
-  protected $minkDefaultDriverClass = PhantomJSDriver::class;
+  protected $minkDefaultDriverClass = DrupalSelenium2Driver::class;

Should we make this configurable?

jibran’s picture

Can I use it on github.com/jibran/dynamic_entity_reference? Here is a re-roll.

jibran’s picture

Status: Needs work » Needs review
droplet’s picture

#32,

Yes if we can. Before it's fully integrated into testbot. It's still useful for CORE testing coding.

Core using the system theme for testing, we never know what's going on behind the scene:
https://www.drupal.org/node/2832672#comment-11913964

michielnugter’s picture

I got it to work on PhantomJS, I started the selenium-server and then ran the tests. As long as phantomJS is in your path that is.

I had some problems settings it up mainly because of the supported versions. I added notes on this in the README.md. Still couldn't get it to run in firefox because I haven't been able to figure out yet on how to run 2 versions in parallel as I want to keep my default firefox on the latest version.

Attached patch fixes the issues mentioned by klausi. I split the patch in two, 1 containing only the fixes that are in scope for this issue and 1 with the headers stuff so the patch can pass..

The last submitted patch, 36: javascripttests_with-2775653-35.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 36: javascripttests_with-2775653-35-with-statuscode.patch, failed testing.

michielnugter’s picture

FileSize
92.96 KB

Forgot the interdiff, attached now. Nothing special changed, a lot of stuff in the composer.lock not sure why it failed here..

jibran’s picture

droplet’s picture

+++ b/core/tests/README.md
@@ -59,6 +59,9 @@ sudo -u www-data -E ./vendor/bin/phpunit -c core --testsuite functional-javascri
+Note: Use the 2.x range for selenium standalone server as 3.x is not yet supported by Mink.

Is this line incorrect? I'm running all latest version to test.

+++ b/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php
@@ -5,32 +5,28 @@
- * Phantomjs requried domain in setCookie.
+ * Provides a driver for Selenium testing.
...
-      'domain' => parse_url($this->getWebDriverSession()->url(), PHP_URL_HOST), // drupal-fixes: phantomjs hack
...
+      'domain' => parse_url($this->getWebDriverSession()->url(), PHP_URL_HOST),

keep a comment indicate this is a PhantomJS hack.

dawehner’s picture

  1. +++ b/composer.lock
    +++ b/composer.lock
    @@ -1099,12 +1099,12 @@
    
    @@ -1099,12 +1099,12 @@
                 "source": {
                     "type": "git",
                     "url": "https://github.com/php-fig/log.git",
    -                "reference": "fe0936ee26643249e916849d48e3a51d5f5e278b"
    +                "reference": "1.0.0"
                 },
                 "dist": {
                     "type": "zip",
    -                "url": "https://api.github.com/repos/php-fig/log/zipball/fe0936ee26643249e916849d48e3a51d5f5e278b",
    -                "reference": "fe0936ee26643249e916849d48e3a51d5f5e278b",
    +                "url": "https://api.github.com/repos/php-fig/log/zipball/1.0.0",
    +                "reference": "1.0.0",
                     "shasum": ""
                 },
                 "type": "library",
    @@ -2988,6 +2988,67 @@
    

    This seems to be an unrelated change.

  2. +++ b/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php
    @@ -0,0 +1,32 @@
    +    if ($value === null) {
    +      $this->getWebDriverSession()->deleteCookie($name);
    +      return;
    +    }
    +
    +    $cookieArray = [
    +      'name' => $name,
    +      'value' => urlencode($value),
    +      'secure' => FALSE,
    +      'domain' => parse_url($this->getWebDriverSession()->url(), PHP_URL_HOST),
    +      'expires' => time() + 80000,
    +    ];
    +
    +    $this->getWebDriverSession()->setCookie($cookieArray);
    +  }
    

    IMHO this should provide some documentation why we are doing this.

  3. +++ b/core/tests/Drupal/FunctionalJavascriptTests/JSWebAssert.php
    @@ -366,4 +366,26 @@ function t(r, lx, ly) {
    +    // should we throw an error?
    +  }
    +
    ...
    +  public function statusCodeNotEquals($code)
    +  {
    +    // should we throw an error?
    

    Let's ensure that #2827014: Throw an exception when testing non response body in javascript tests will be committed before that.

jibran’s picture

Title: JavascriptTests with webDriver » [PP-2] JavascriptTests with webDriver
Status: Needs review » Postponed
droplet’s picture

michielnugter’s picture

Rerolled to update to the latest composer files.

Removed the notes on Selenium 2.x, I got something to work on 3.x. PhantomJS keeps failing, don't know yet. testJavascript() keeps failing in firefox, don't know that either..

If I do get a proper setup with firefox, chrome and phantomjs working I'll update the issue.

Kept the status methods for now to make the patch workable, will rerole after #2827014: Throw an exception when testing non response body in javascript tests gets committed.

droplet’s picture

testJavascript() keeps failing in firefox, don't know that either..

It will, @see #13 logs. Real broswers & PhantomJS using diff way to measure dimensions. My very first patch has a fix, I think we can fix it in a new issue thread also. ( https://www.drupal.org/files/issues/testbots.patch )

droplet’s picture

dawehner’s picture

Headless Mode In CHROME!!!

I recommend you to calm down :)

Mixologic’s picture

https://groups.google.com/forum/#!topic/phantomjs/9aI5d-LDuNE phantomJS is going to go unsupported as well.

UNSUPPORTED!

dawehner’s picture

cosmicdreams’s picture

Came here to also report on headless mode in Chrome (which works in Chrome canary and could be used to fine tune things now) and the planned shutdown of phantomjs. Cool to see you're all in the know.

Given that knowledge how do we cover the gap? Do we just need to change the directives that the test runner uses to launch web pages during testing?

dawehner’s picture

This is tricky now. In a BOF during Drupalcon we talked about using nightwatch, see #2869825: Experiment: Get nightwatch working to functional test Drupal JS via JS for a detailed explanation.

droplet’s picture

Just some random thoughts...

I think we need a prediction at the moment. I don't think webdriver will be the future since Chrome Headless come out now. Chrome will push the whole eco forward a big step. webdriver is missing a lot of cool APIs compare to RDP.

https://github.com/cyrus-and/chrome-remote-interface#implementations
http://remotedebug.org/

On the other side, I think a solid API designs is more important NOW. For example,

Codeception (PHP) & CodeceptJS is almost sharing same APIs. It's easier for all developers. Adopting a new rockstar is also easy: https://github.com/Codeception/CodeceptJS/blob/master/lib/helper/Nightma...

Also, take a look at:
https://github.com/laravel/dusk

justafish’s picture

@droplet Chrome Headless is indeed super cool, but it's not a replacement for Web Driver (which runs on top of it). You still need the Selenium/Web Driver protocol to actually drive the browser and click around etc. See https://chromium.googlesource.com/chromium/src/+/lkgr/headless/README.md

dawehner’s picture

On the other side, I think a solid API designs is more important NOW. For example,

Codeception (PHP) & CodeceptJS is almost sharing same APIs. It's easier for all developers. Adopting a new rockstar is also easy: https://github.com/Codeception/CodeceptJS/blob/master/lib/helper/Nightma...

Also, take a look at:
https://github.com/laravel/dusk

Thank you for providing all those links. I have a bit of a problem to see why we should push to use codeception, given that it would require us another HUGE rewrite of all the tests we have. Are our tests actually that simple can we can use the syntax/magic codeception provides? It could be nice.

I agree it offers a lot of nicety, but I believe writing tests is not hard because of the syntax. The trick is to actually know what you want to test.

It also seems to be the case that the actual usage rates of codeception aren't that high (https://www.npmjs.com/package/codeceptjs has 25000 downloads vs. https://www.npmjs.com/package/nightwatch has 225000 downloads)

droplet’s picture

@justafish,

You're also correct. WebDriver is one of the communicating channels. It runs Chrome via Chromedriver. Only Safari 10 is supported WebDriver natively.

Chrome:
Selenium -> WebDriver -> Chromdriver -> Chrome DevTools Protocol -> Chrome

Edge
Selenium -> WebDriver -> Edgedriver -> (Simiilar to Chrome DevTools Protocol, or exactly using it) -> Edge

Firefox (Latest version)
Selenium -> WebDriver -> geckodriver -> Marionette proxy -> Firefox

Safari 10
Selenium -> WebDriver -> Safari

I've read a lot of articles in the past and recently. Most of them stated Selenium supporting these drivers development and many bugs around that. They made the WebDriver protocol actually. And some more interesting things, e.g.:
https://news.ycombinator.com/item?id=14240402

And it seems some features able to do with Selenium only, not WebDriver itself.

justafish’s picture

@droplet You don't need to use Selenium at all when using Nightwatch for Chrome and Edge, the standalone application can be used: http://nightwatchjs.org/gettingstarted#chromedriver

There's even an npm package that's super easy to install https://www.npmjs.com/package/chromedriver

droplet’s picture

@justafish,
That just replaced Selenium with another packages only. It's very diff with communicating with browser natively. Whatever, it isn't the point here.

"Chrome DevTools Protocol" vs "WebDriver". "Chrome DevTools Protocol" is better.

To be clarified, I didn't say no to nightwatch.js or any other E2E testings. Just try if we able to find a better and solid workaround for the future and fit into Drupal. If it's a private project or other JS libs, to change a test engine almost seamless usually. But in Drupal, it takes years:
#2809161: Convert Javascript/AJAX testing to use JavascriptTestBase

I asked the maintainer of nightwatch in an issue thread and will see if we have more info.
https://github.com/nightwatchjs/nightwatch/issues/1439#issuecomment-2989...