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