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.

There are various problems with this:

  1. GastonJS is not that much supported and doesn't have a huge userbase.
  2. We are using phantomjs, which is not necessarily maintained and stable, especially compared to a headless chrome

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 chrome in webdriver mode and phantomjs in non webdriver mode
  2. Use the mink selenium driver, instead of the gastonjs one. (this issue)
  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

Testing (chrome):

  1. brew install selenium-server-standalone
  2. brew install chromedriver
  3. selenium-server -port 4444
  4. export MINK_DRIVER_ARGS_WEBDRIVER='["chrome", null, "http://localhost:4444/wd/hub"]'
  5. sudo -u _www ./vendor/bin/phpunit -v -c ./core core/tests/Drupal/FunctionalJavascriptTests/Tests/JSWebWithWebDriverAssertTest.php
CommentFileSizeAuthor
#133 2775653-133.patch13.77 KBalexpott
#133 132-133-interdiff.txt2.37 KBalexpott
#132 2775653-132.patch12.57 KBalexpott
#132 124-132-interdiff.txt1.94 KBalexpott
#124 2775653-124.patch13.3 KBLendude
#124 interdiff-2775653-122-124.txt710 bytesLendude
#122 2775653-122.patch13.3 KBLendude
#122 interdiff-2775653-120-122.txt9.6 KBLendude
#120 2775653-120.patch13.32 KBLendude
#120 interdiff-2775653-117-120.txt3.96 KBLendude
#117 interdiff-2775653.txt1.18 KBdawehner
#117 2775653-117.patch12.61 KBdawehner
#109 interdiff-2775653.txt1.06 KBdawehner
#109 2775653-109.patch12.53 KBdawehner
#103 interdiff-2775653.txt844 bytesdawehner
#103 2775653-103.patch12.84 KBdawehner
#86 2775653-86.patch12.84 KBdawehner
#85 interdiff-2775653.txt796 bytesdawehner
#85 2775653-85.patch14.9 KBdawehner
#83 interdiff-2775653-83.patch4.01 KBdawehner
#83 2775653-83.patch14.79 KBdawehner
#80 interdiff-2775653-78-80.txt667 bytesMixologic
#80 pp_1_javascripttests-2775653-80.patch11.65 KBMixologic
#78 interdiff-2775653.txt4.08 KBdawehner
#78 2775653-78.patch11.03 KBdawehner
#76 pp_1_javascripttests-2775653-76.patch10.74 KBjibran
#76 interdiff-fc6601.txt1 KBjibran
#73 2775653-73.patch10.03 KBdawehner
#64 2775653-64.patch19.74 KBmichielnugter
#61 chrome-driver-dirty-test.patch6.92 KBdroplet
#46 javascripttests_with-2775653-46-with-statuscode.patch19.74 KBmichielnugter
#46 interdiff.txt91.03 KBmichielnugter
#40 javascripttests_with-2775653-40.patch11.56 KBjibran
#40 interdiff.txt2.52 KBjibran
#39 interdiff.txt92.96 KBmichielnugter
#36 javascripttests_with-2775653-35-with-statuscode.patch100.74 KBmichielnugter
#36 javascripttests_with-2775653-35.patch99.87 KBmichielnugter
#33 javascripttests_with-2775653-33.patch11.41 KBjibran
#29 javascripttests_with-2775653-29.patch32.09 KBjuampynr
#26 selenium-driver-2775653-26.patch32.07 KBklausi
#15 2775653-15.patch15.4 KBdroplet
#13 Phantomjs-log.txt7.49 KBdroplet
#13 Chrome-log.txt8.27 KBdroplet
#13 interdiff.patch2.37 KBdroplet
#13 2775653-13.patch15.44 KBdroplet
#12 2775653-12.patch13.49 KBdawehner
testbots.patch4.97 KBdroplet
Support from Acquia helps fund testing for Drupal Acquia logo

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 status code or response headers in functional 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 status code or response headers in functional JavaScript tests will be committed before that.

jibran’s picture

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 status code or response headers in functional 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: Leverage JS for JS testing (using nightwatch) 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...

Mixologic’s picture

Im not sure if this is the place to mention this, but I've been experimenting a little with trying out chrome headless and https://packagist.org/packages/dmore/chrome-mink-driver.

I wasn't actually able to get a test fully working, but I didnt fully attempt to do much beyond changing JavascriptTestBase to use a different mink driver and have chrome in a container.

In any case, I've been spending a lot of time attempting to repurpose some hardware that we have (old db servers, old webnodes etc) so that these can become dedicated testing resources, and Im running into a bug with phanomjs where the network fails out when running on debian in a vmware environment. Which means I cant support the daily testruns on these bots, so I cant save us $$ in repurposing this hardware.

It seems as though my only options are 1. get rid of phantomjs, and replace it with something like headless chrome. 2. figure out how to compile phantomjs 2.5.0 beta for debian jessie, which I've made some attempts, but utterly failed to do (you have to compile qtwebkit, a virtually unsupported dependency first in order to compile phantom). 3. figure out how to use one of the ubuntu binaries of phantomjs.

Anyhow, Im hoping we can come up with some sort of short term solution to doing javascript tests that will allow us to configure a different backend such that either a webdriver solution, or a direct chrome mink solution can work.

droplet’s picture

@Mixologic,

It works.

~ .\vendor\bin\phpunit.bat -c core core\tests\Drupal\FunctionalJavascriptTests\Core\MachineNameTest.php --debug -vv
PHPUnit 4.8.35 by Sebastian Bergmann and contributors.

Runtime:        PHP 7.0.8
Configuration:  C:\MyWorkTests\drupal8x\core\phpunit.xml


Starting test 'Drupal\FunctionalJavascriptTests\Core\MachineNameTest::testMachineName'.
.

Time: 33.95 seconds, Memory: 2.00MB

OK (1 test, 18 assertions)
dawehner’s picture

In any case, I've been spending a lot of time attempting to repurpose some hardware that we have (old db servers, old webnodes etc) so that these can become dedicated testing resources, and Im running into a bug with phanomjs where the network fails out when running on debian in a vmware environment. Which means I cant support the daily testruns on these bots, so I cant save us $$ in repurposing this hardware.

It seems as though my only options are 1. get rid of phantomjs, and replace it with something like headless chrome. 2. figure out how to compile phantomjs 2.5.0 beta for debian jessie, which I've made some attempts, but utterly failed to do (you have to compile qtwebkit, a virtually unsupported dependency first in order to compile phantom). 3. figure out how to use one of the ubuntu binaries of phantomjs.

Let's keep the discussion focused on using webdriver via selenium here. It solves at least one problem.

The step after that is to use chrome's webdriver implementation on the testbot. This should enable the usecase of @Mixologic!
Once we are there we could then explore the idea of nightwatch/other stuff .

  1. +++ b/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php
    @@ -0,0 +1,32 @@
    +    $cookieArray = [
    

    It'd be nice to have some documentation why why need this here ...

  2. +++ b/core/tests/Drupal/FunctionalJavascriptTests/JSWebAssert.php
    @@ -366,4 +366,26 @@ function t(r, lx, ly) {
    +  /**
    +   * Overrides statusCodeEquals function. Use Functional tests for status code validation
    +   *
    +   * @param int $code
    +   *
    +   */
    +  public function statusCodeEquals($code)
    +  {
    +    // should we throw an error?
    +  }
    +
    +  /**
    +   * Overrides statusCodeEquals function. Use Functional tests for status code validation
    +   *
    +   * @param int $code
    +   *
    +   */
    +  public function statusCodeNotEquals($code)
    +  {
    +    // should we throw an error?
    +  }
    

    We have a dedicated issue for it, don't we?

droplet’s picture

+++ b/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php
@@ -0,0 +1,32 @@
+    $cookieArray = [

I documented some in Comment #13 patch

We have a dedicated issue for it, don't we?

#2827014: Throw an exception when testing status code or response headers in functional JavaScript tests

We can reroll all missing things after above issue.

Slightly OFF-TOPIC:

The step after that is to use chrome's webdriver implementation on the testbot. This should enable the usecase of @Mixologic!

I just realize above package @Mixologic linked is not "webdriver", but binding the API to Chrome devtool API.

Strictly, this package may not an equal conversion of webdriver.
(Note: I can be wrong and very wrong. I've never dug into selenium2/fb php-webdriver source code deeply)

Take an example:

The ->visit() API:

// php-webdriver (Mink Selenium2 and Phantomjs)
  public function open($url)
    {
        $this->curl('POST', '/url', array('url' => $url));

        return $this;
    }
// chrome-mink-driver
public function visit($url)
    {
        $this->response = null;
        $this->send('Page.navigate', ['url' => $url]);
        $this->page_ready = false;
        $this->waitForPage();
        $this->waitForDom(); // droplet: this is watied for `document.readyState == "complete"`.
    }

Look at the last 2 lines. chrome-mink-driver package added extra steps for verification, which meant this chrome-mink-driver package creates an unreal environment for the testing.

The human won't wait for everything loaded before their mouse actions and our JS should work on`DOMContentLoaded` and before ideally. (Even though, we may not able to catch the diff between this 2 events. Drupal CORE has very lightweight JS usages and runs tests locally. There're only a few ms between these 2 events. But in Contrib, they can load remote resources)

Overall, it seems not the prime time to use chrome-mink-driver.

michielnugter’s picture

FileSize
19.74 KB

Reroll based on the patch 'javascripttests_with-2775653-46-with-statuscode.patch' which uses behat/mink-selenium2-driver.

Can't get it to run for now locally (errors with cookies) but haven't tried a lot right now, think my Selenium setup is currently broken.

cosmicdreams’s picture

With Chrome 59 coming out on June 5 (the day of this comment) Headless Chrome is now in stable.

Here's some guidance from Google Developers that shows how it's used via the command line and programmatically with node: https://developers.google.com/web/updates/2017/04/headless-chrome

Deeply linked in that article is a setup guide for getting Selenium working with headless chrome:
https://intoli.com/blog/running-selenium-with-headless-chrome/

It sounds like we don't think dman's chrome driver library is ready yet so perhaps we should experiment with getting selenium and headless chrome connected?

cosmicdreams’s picture

And here's an issue for gitlab where people are working through similar issues: https://gitlab.com/gitlab-org/gitlab-ce/issues/30876

michielnugter’s picture

Right, my mac just updated chrome to 59 and decided to give it a spin.

With the following:
- Selenium 3.4 (http://www.seleniumhq.org/download/)
- ChromeDriver 2.29 (https://sites.google.com/a/chromium.org/chromedriver/downloads)
- chromedriver in my PATH (FISH: set PATH $PATH /path/to/chromedriver

Starting Selenium with the following:
java -jar selenium-server-standalone-3.4.0.jar

And setting:
$this->minkDefaultDriverArgs = ['chrome'];

... It worked! JSWebAssertTest ran succesfully. Haven't tried other tests yet. Funny thing: it worked headless out of the box and can't figure out how to not run it headless just to see what's going on :)

PhantomJS keeps failing though with some cookie errors:

{"errorMessage":"Unable to set Cookie","request":{"headers":{"Accept-Encoding":"gzip,deflate","Connection":"Keep-Alive","Content-Length":"278","Content-Type":"application/json; charset=utf-8","Host":"localhost:20575","User-Agent":"Apache-HttpClient/4.5.3 (Java/1.8.0_121)"},"httpVersion":"1.1","method":"POST","post":"{\"cookie\":{\"path\":\"/\",\"domain\":\"drupal.dev\",\"name\":\"SIMPLETEST_USER_AGENT\",\"httpOnly\":false,\"hCode\":1771091788,\"secure\":false,\"value\":\"simpletest92649072%3A1496848665%3A593819192422a2.27794064%3A9nNG1kktwX06rSHHSovT3EJz7riJAV674QhZBXW6xic\",\"class\":\"org.openqa.selenium.Cookie\"}}","url":"/cookie","urlParsed":{"anchor":"","query":"","file":"cookie","directory":"/","path":"/cookie","relative":"/cookie","port":"","host":"","password":"","user":"","userInfo":"","authority":"","protocol":"","source":"/cookie","queryKey":{},"chunks":["cookie"]},"urlOriginal":"/session/74ec5da0-4b94-11e7-8263-b94bf6d69bf0/cookie"}}
Build info: version: '3.4.0', revision: 'unknown', time: 'unknown'
System info: host: 'MacBook-Pro-van-Michiel.local', ip: '192.168.30.50', os.name: 'Mac OS X', os.arch: 'x86_64', os.version: '10.12.5', java.version: '1.8.0_121'
Driver info: driver.version: unknown

Firefox complains about the following:

Could not open connection: Unrecognized channel: 9

Safari also works! (after enabling the remote control via developer tools) Broke something in my install causing some errors in the test but it starts up. Nice to see it clicking through the test :)

EDIT: I used the latest patch for testing this, nothing changed.

droplet’s picture

@michielnugter,

I guess you ran chrome with --headless before. And the test takes that instance to run. To check the Activity Monitor :)

We can pass more flags to disable plugins and use a custom profile or always open a new instance to test on a cleaner Chrome instance also.

andypost’s picture

Using --headless --disable-gpu works on Ubuntu with chrome 59

droplet’s picture

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

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

larowlan’s picture

Title: [PP-2] JavascriptTests with webDriver » [PP-1] JavascriptTests with webDriver
dawehner’s picture

FileSize
10.03 KB

Here is a reroll I'm using while trying to integrate it into the testbot.

cosmicdreams’s picture

Looks like Firefox's headless mode lands in v56. Which, at the time of this comment, is the next version of Firefox Stable. Firefox Nightly is at version 57 so we can start testing it if necessary.

Documentation for how to use it with Node.js + Selenium is here: https://developer.mozilla.org/en-US/Firefox/Headless_mode

droplet’s picture

#73 is missing DrupalSelenium2Driver file.

jibran’s picture

FileSize
1 KB
10.74 KB

Added DrupalSelenium2Driver file back.

dawehner’s picture

Assigned: Unassigned » dawehner

Thank you @jibran
I've some more changes locally, so I'm assigning the issue to myself.

dawehner’s picture

Assigned: dawehner » Unassigned
Status: Postponed » Needs review
FileSize
11.03 KB
4.08 KB
  • We continue using phantomjs by default.
  • We make it easy to swap in webdriver though
  • Once the testbot has support we slowly move over in order to detect random test failures etc.

Status: Needs review » Needs work

The last submitted patch, 78: 2775653-78.patch, failed testing. View results

Mixologic’s picture

Im getting very close on having a chromedriver container running on the testbots. But I need the patch from #2894482: MINK_DRIVER_ARGS does not support associative arrays in order to test it, so I've combined them here.

dawehner’s picture

Status: Needs work » Postponed

Thank you @Mixologic!!
Let's postpone this issue on the other one then. I posted a new patch over there. This should be easy to fix :)

dawehner’s picture

Title: [PP-1] JavascriptTests with webDriver » [PP-2] JavascriptTests with webDriver
Related issues: +#2894501: Allow for separate mink driver settings for Javascript tests
dawehner’s picture

This exposes now two additional environment variables: MINK_DRIVER_ARGS_WEBDRIVER and MINK_DRIVER_CLASS_WEBDRIVER

GrandmaGlassesRopeMan’s picture

Issue tags: +JavaScript
dawehner’s picture

Just a reroll + small documentation adaption, waiting for #2894501: Allow for separate mink driver settings for Javascript tests to be actually pushed.

dawehner’s picture

Here is a proper reroll.

dawehner’s picture

Title: [PP-2] JavascriptTests with webDriver » JavascriptTests with webDriver
Status: Postponed » Needs review

The last submitted patch, 80: pp_1_javascripttests-2775653-80.patch, failed testing. View results

The last submitted patch, 83: 2775653-83.patch, failed testing. View results

The last submitted patch, 83: interdiff-2775653-83.patch, failed testing. View results

The last submitted patch, 85: 2775653-85.patch, failed testing. View results

dawehner’s picture

I pinged @Mixologic on slack, so he can use this patch and test it with the testbot infrastructure.
The idea is to start with one test using webdriver and then move over the other one's over time. Last time I talked with @Mixologic this wasn't just a fire and go, as some tests seemed to be broken.

Status: Needs review » Needs work

The last submitted patch, 86: 2775653-86.patch, failed testing. View results

Mixologic’s picture

Hey, check that out. https://dispatcher.drupalci.org/job/drupal_patches/39407/testReport/Java...

Green patch in #86 now that I deployed chromedriver on the testbots.

cspitzlay’s picture

That's great news. Congrats!

cspitzlay’s picture

BTW: Webdriver seems to be strict about whether an element is visible if you want to fill in a value or click it.
It requires that it is not hidden, in the view if the page is scrollable and not covered by other elements.
phantomjs OTOH would just do it. I wonder if core's JS tests will show the same issues I was seeing when I made the switch for our own tests...

dawehner’s picture

BTW: Webdriver seems to be strict about whether an element is visible if you want to fill in a value or click it.
It requires that it is not hidden, in the view if the page is scrollable and not covered by other elements.

Indeed, I've had annoying times in my life with modals in tests. I think though this makes sense as design limitation, given we test user interaction.

In case we need to still fill in other fields, maybe we could do something like #2917885: Add drupalPostFormWithInvalidOptions() to BrowserTestBase aka. provide some helpers for hacky tests.

+++ b/core/tests/Drupal/FunctionalJavascriptTests/JavascriptTestBase.php
@@ -176,4 +194,12 @@ protected function getDrupalSettings() {
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function getHtmlOutputHeaders() {
+    // The webdriver API does not support fetching headers.
+    return '';
+  }
+

Should we throw some @trigger_error as well?

cspitzlay’s picture

  • Hidden fields can also be set using injected jQuery code. That's how I worked around one issue we had.
  • Most scrolling issues we had could be solved with code like this:
      /**
       * Scrolls the first element found by the selector into view.
       * @param string $selector The jQuery selector to find the element.
       * @param bool $opt_center TRUE means to center element inside the scrollable parent element,
       *                         FALSE means scroll only to the closest border inside the scrollable parent element.
       */
      protected function scrollIntoViewIfNeeded(string $selector, bool $opt_center = TRUE) {
        $opt_center_string = $opt_center ? 'true' : 'false';
        $this->getSession()->executeScript("jQuery('${selector}')[0].scrollIntoViewIfNeeded(${opt_center_string})");
      }
    
dawehner’s picture

Status: Needs work » Needs review

There is no reason that this is on needs work at this point in time.

xjm’s picture

For #97, dawehner asked about BC for getHtmlOutputHeaders(). Did it ever work with Phantom? Maybe it should actually raise a fail or exception that that is not allowed in that type of test though? For more explicit debugging.

dawehner’s picture

@xjm
Yes, phantomjs included commands to fetch the HTTP errors as well HTTP headers. Due to webdriver being basically an abstraction over a normal user in a browser, things like HTTP status and headers no longer exist, at least in the view of whoever designed webdriver :)

Maybe it should actually raise a fail or exception that that is not allowed in that type of test though? For more explicit debugging.

For the assertions we have right now:

  public function statusCodeEquals($code) {
    @trigger_error('Support for statusCodeEquals is to be dropped from Javascript tests. See https://www.drupal.org/node/2857562.');
    parent::statusCodeEquals($code);
  }

so I guess doing the same makes sense here.

droplet’s picture

+++ b/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php
@@ -0,0 +1,35 @@
+      // Unlike \Behat\Mink\Driver\Selenium2Driver::setCookie we set a domain
+      // and an expire date, as otherwise cookies leak from one test into
+      // another.

anyone mind to review this doc again? Because when I read it I don't fully understand why did it.

I first introduce this for phantomJS only
https://www.drupal.org/project/drupal/issues/2775653#comment-11646983

dawehner’s picture

Does this make it easier to understand?

mtodor’s picture

I checked this a bit since in Thunder we are using WebDriver for a long time with Selenium and Chrome driver. It works fine with few method overwrites. With refactoring here, it looks way better, on first glance - only $minkDefaultDriverClass and $minkDefaultDriverArgs should be provided and that's it. Nice job!

There is the only thing that I would like to ask related to this part:

+++ b/core/tests/Drupal/FunctionalJavascriptTests/JavascriptTestBase.php
@@ -22,14 +26,19 @@
+    if ($this->minkDefaultDriverClass === DrupalSelenium2Driver::class) {
+      $this->minkDefaultDriverArgs = ['phantomjs'];
+    }
+    elseif ($this->minkDefaultDriverClass === PhantomJSDriver::class) {
+      // Set up the template cache used by the PhantomJS mink driver.
+      $path = $this->tempFilesDirectory . DIRECTORY_SEPARATOR . 'browsertestbase-templatecache';
+      $this->minkDefaultDriverArgs = [
+        'http://127.0.0.1:8510',
+        $path,
+      ];
+      if (!file_exists($path)) {
+        mkdir($path);
+      }
     }

We are setting $minkDefaultDriverArgs in JavascriptTestBase::initMink(), then the parent is called and in the parent after a few up and down function calls getMinkDriverArgs() is executed and result of that replaces $minkDefaultDriverArgs. So my question is, why we don't move defaults in function minkDefaultDriverArgs() and return them only if getenv('MINK_DRIVER_ARGS_WEBDRIVER') ?: getenv('MINK_DRIVER_ARGS_PHANTOMJS') ?: parent::getMinkDriverArgs(); and so on - is empty. Then it would be more readable and understandable what is going on there. Maybe I'm missing something, is there some reason for not doing that? And also it would be nice that getMinkDriverArgs() returns array instead of string then. That would also remove some checks if empty value from minkDefaultDriverArgs() and also assumption that function will return Json string.

dawehner’s picture

We (@mixologic, @justafish, @drpal, @dawehner) discussed a bit about JS testing in general

a) We agreed that testing JS in JS is the obvious choice
b) We agreed that we will not be able to get rid of JS testing in PHP given semantic versioning
c) @drpal especially made the point that in order to get the Drupal community into the mindset of using JS having a PHP based testing being around is a bad thing. Once we figured out JS testing we want to actively deprecate to test JS in PHP
d) We agreed on proceeding on the following steps:

  1. #2775653: JavascriptTests with webDriver
  2. #2928962: Run yarn-based core tests on the testbot
  3. #2869825: Leverage JS for JS testing (using nightwatch)
GrandmaGlassesRopeMan’s picture

~

cspitzlay’s picture

I hope d) 1. means that support for driving a browser with JS enabled will stay around (non-deprecated) even after testing JS in PHP is "actively deprecated". In the tests of the application we have the focus is not on testing JS but in a few spots the JS support is needed.

alexpott’s picture

Status: Needs review » Needs work
+++ b/core/tests/Drupal/FunctionalJavascriptTests/LegacyJavascriptTestBase.php
@@ -10,6 +12,20 @@
+  /**
+   * {@inheritdoc}
+   */
+  protected function getMinkDriverClass() {
+    return getenv('MINK_DRIVER_CLASS_PHANTOMJS') ?: BrowserTestBase::getMinkDriverClass();
+  }

I think this is not necessary anymore.

re #97 I don't think we should add an @trigger_error to getHtmlOutputHeaders() since \Drupal\Tests\BrowserTestBase::drupalGet() will call it.

dawehner’s picture

Status: Needs work » Needs review
FileSize
12.53 KB
1.06 KB

I hope d) 1. means that support for driving a browser with JS enabled will stay around (non-deprecated) even after testing JS in PHP is "actively deprecated". In the tests of the application we have the focus is not on testing JS but in a few spots the JS support is needed.

I totally think so, and if its just for semantic version reasons :)

@alexpott
Good point. I totally think we can remove the arguments code as well.

Mixologic’s picture

Status: Needs review » Reviewed & tested by the community

This LGTM. +1

alexpott’s picture

Status: Reviewed & tested by the community » Needs review

I've tried to test this patch and have not been at all successful.

I can run javascript tests with the old PhantomJSDriver no problem.

If I try to run the new webdriver test \Drupal\FunctionalJavascriptTests\Tests\JSWebWithWebDriverAssertTest using phantomjs --debug=true --webdriver=4444 I have a problem setting cookies... the first one that fails is the xdebug cookie but also the simpletest cookie fails too...

2017-12-15T16:35:59 [DEBUG] HTTP Request - Content Body: {"cookie":{"name":"XDEBUG_SESSION","value":"PHPSTORM","secure":false,"domain":"drupal8alt.dev"}}
[DEBUG - 2017-12-15T16:35:59.868Z] RouterReqHand - _handle - {"headers":{"Accept":"application/json;charset=UTF-8","Content-Length":"96","Content-Type":"application/json;charset=UTF-8","Host":"localhost:4444"},"httpVersion":"1.1","method":"POST","post":"{\"cookie\":{\"name\":\"XDEBUG_SESSION\",\"value\":\"PHPSTORM\",\"secure\":false,\"domain\":\"drupal8alt.dev\"}}","url":"/session/06a762a0-e1b6-11e7-ad6b-6f1152ea8817/cookie","urlParsed":{"anchor":"","query":"","file":"cookie","directory":"/session/06a762a0-e1b6-11e7-ad6b-6f1152ea8817/","path":"/session/06a762a0-e1b6-11e7-ad6b-6f1152ea8817/cookie","relative":"/session/06a762a0-e1b6-11e7-ad6b-6f1152ea8817/cookie","port":"","host":"","password":"","user":"","userInfo":"","authority":"","protocol":"","source":"/session/06a762a0-e1b6-11e7-ad6b-6f1152ea8817/cookie","queryKey":{},"chunks":["session","06a762a0-e1b6-11e7-ad6b-6f1152ea8817","cookie"]}}
2017-12-15T16:35:59 [DEBUG] CookieJar - Saved "XDEBUG_SESSION=PHPSTORM; domain=.drupal8alt.dev; path=/"
2017-12-15T16:35:59 [DEBUG] CookieJar - Rejected Cookie "XDEBUG_SESSION=PHPSTORM; domain=drupal8alt.dev"
2017-12-15T16:36:00 [DEBUG] HTTP Response - Status Code 500 Internal Server Error

If I download selenium server 2 or 3 I can not get either to work with Firefox - this might be because there is no combination that is compatible with the latest Firefox and the PHP webdriver implementation.

Going to try chrome via selenium now.

I think I might be doing something wrong but regardless I think we need better instructions.

alexpott’s picture

So chrome worked fine with Selenium 3.

My steps to make it work:

  1. brew install selenium-server-standalone
  2. brew install chromedriver
  3. export MINK_DRIVER_ARGS_WEBDRIVER='["chrome", null, "http://localhost:4444/wd/hub"]'
  4. sudo -u _www ./vendor/bin/phpunit -v -c ./core core/tests/Drupal/FunctionalJavascriptTests/Tests/JSWebWithWebDriverAssertTest.php

Firefox does not work... here's what I do extra:

  1. brew install geckodriver
  2. export MINK_DRIVER_ARGS_WEBDRIVER='["firefox", [], "http://localhost:4444/wd/hub"]'
    (note the null is replaced with a [] - with a null firefox does not even start
  3. sudo -u _www ./vendor/bin/phpunit -v -c ./core core/tests/Drupal/FunctionalJavascriptTests/Tests/JSWebWithWebDriverAssertTest.php

This so nearly works but the findButton doesn't work because it looks like \WebDriver\Container::webDriverElement doesn't play nice with what firefox is returning... it expects an array with an ELEMENT key but we have an array with a key like element-6066-11e4-a52e-4f735466cecf

Mile23’s picture

Status: Needs review » Needs work
$ git apply 2775653-109.patch
$ composer install
$ ~/Downloads/phantomjs-2.1.1-macosx/bin/phantomjs --webdriver=4444 &
$ ./vendor/bin/phpunit -c core --testsuite functional-javascript --group Ajax -vv
PHPUnit 4.8.36 by Sebastian Bergmann and contributors.

Runtime:	PHP 7.0.8 with Xdebug 2.4.0
Configuration:	/Users/paul/pj2/drupal/core/phpunit.xml

Testing 
SSSSSSSS

Time: 3.32 minutes, Memory: 10.00MB

There were 8 skipped tests:

1) Drupal\FunctionalJavascriptTests\Ajax\AjaxCallbacksTest::testDateAjaxCallback
PhantomJS is either not installed or not running. Start it via phantomjs --ssl-protocol=any --ignore-ssl-errors=true vendor/jcalderonzumba/gastonjs/src/Client/main.js 8510 1024 768&

[..etc.. All the tests were skipped in this way. Also this: ]

Remaining deprecation notices (8)

drupal_set_message() is deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead. See https://www.drupal.org/node/2774931: 8x
    1x in AjaxCallbacksTest::testDateAjaxCallback from Drupal\FunctionalJavascriptTests\Ajax
    1x in AjaxCallbacksTest::testDateTimeAjaxCallback from Drupal\FunctionalJavascriptTests\Ajax
    1x in AjaxFormImageButtonTest::testAjaxImageButton from Drupal\FunctionalJavascriptTests\Ajax
    1x in AjaxFormPageCacheTest::testSimpleAJAXFormValue from Drupal\FunctionalJavascriptTests\Ajax
    1x in AjaxFormPageCacheTest::testAjaxElementValidation from Drupal\FunctionalJavascriptTests\Ajax
    1x in AjaxTest::testAjaxWithAdminRoute from Drupal\FunctionalJavascriptTests\Ajax
    1x in AjaxTest::testDrupalSettingsCachingRegression from Drupal\FunctionalJavascriptTests\Ajax
    1x in BackwardCompatibilityTest::testAjaxBackwardCompatibility from Drupal\FunctionalJavascriptTests\Ajax

......

+++ b/core/tests/README.md
@@ -46,3 +41,30 @@ export SIMPLETEST_BASE_URL='http://d8.dev'
+## Functional javascript tests
+
+* Start PhantomJS:
+  ```
+  phantomjs --webdriver=4444
+  ```
+
+* Run the functional javascript tests:
+
+```
+  ./vendor/bin/phpunit -c core --testsuite functional-javascript
+```

Basically these instructions are not correct.

Mixologic’s picture

Needs issue summary update badly.

Where things are at now is we're aiming to support *both* Mink->gastonjs->phantomjs and deprectate it, alongside adding Mink->webdriver->chromedriver(geckodriver/edgedriver/etc).

Additionally, we don't want to suggest running phantomjs in webdriver mode at all, because we're trying to avoid anybody using phantomjs for anything. Its deprecated abandonware at this point.

geckodriver might not be completely webdriver api compliant. I've installed chromedriver on the testbots, and am using that as the baseline for now.

core/tests/Drupal/FunctionalJavascriptTests/Tests/JSWebWithWebDriverAssertTest.php

is the only test that should actually use webdriver as of this patch, all other javascript tests would continue to use phantom until we convert them over. (many of them pass simply by changing the class, others might require some investigation)

Berdir’s picture

I recently also switched from phantomjs to chromedriver in one of our custom distributions with about 3500 behat assertions. Created about 3 issues/merge requests, the biggest problem was basically that chrome/ChromeDrive are much stricter in regards to visibility of elements when writing into fields/clicking things but the driver does not yet ensure that, so a lot of things fail silently/in unexpected ways. https://gitlab.com/DMore/chrome-mink-driver/issues/42 addresses some of that but there are more cases.

Another tricky problem is timing, Chrome is a lot faster with JS execution and things, so a common problem I had is that ajax was basically finished but the behaviors were still running or elements like ckeditor/dropzone weren't fully initialized yet.

But overall, I'm pretty happy, seeing a lot less random fails now.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.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

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update
FileSize
12.61 KB
1.18 KB
  • rerolled
  • Updated the issue summary
  • Adapted the readme
Lendude’s picture

Ahhh I got it to work!

+++ b/core/tests/README.md
@@ -46,3 +41,25 @@ export SIMPLETEST_BASE_URL='http://d8.dev'
+sudo -u _www ./vendor/bin/phpunit -v -c  --testsuite functional-javascript
...
+  ./vendor/bin/phpunit -c core --testsuite functional-javascript

It would be nice if these instructions were the same. The sudo -u _www only applies when you need to run as the _www user. So that might need some additional comment for those that just want to copy/paste the instructions
Also, when I run ./vendor/bin/phpunit -v -c --testsuite functional-javascript, it throws:
Could not read "--testsuite".
Because it's missing the 'core' part.

I followed these steps again, but one essential part is starting the selenium server, so we might want to add the step:
selenium-server -port 4444

Or you just get skipped tests.

Ok so I didn't feel like running the entire testsuite, so I tested with:
./vendor/bin/phpunit -v -c core --filter JSWebWithWebDriverAssertTest

Result:

PHPUnit 4.8.36 by Sebastian Bergmann and contributors.

Runtime:        PHP 7.1.6 with Xdebug 2.5.0
Configuration:  /Applications/MAMP/htdocs/d8/drupal/core/phpunit.xml

Testing 
.

Time: 52.98 seconds, Memory: 160.00MB

OK (1 test, 22 assertions)

Remaining deprecation notices (1)

drupal_set_message() is deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead. See https://www.drupal.org/node/2774931: 1x
    1x in JSWebWithWebDriverAssertTest::testJsWebAssert from Drupal\FunctionalJavascriptTests\Tests

But it pops a chrome window and runs everything! Pretty cool.

Starting PhantomJS (the old fashioned way, not using webdriver) and running:
./vendor/bin/phpunit -v -c core --filter JSWebAssertTest

Gives the same deprecation notice but works too.

So instead of removing the PhantomJS start instruction would it be better to move that to the ## Functional javascript tests part of the readme and expand it with starting selenium-server?

droplet’s picture

Lendude’s picture

Redid the README, I think it flows a bit better now, but let me know if it needs more tuning.

Edit: the interdiff is a bit weird, interdiff didn't want to play nice for some reason....

Status: Needs review » Needs work

The last submitted patch, 120: 2775653-120.patch, failed testing. View results

Lendude’s picture

Status: Needs work » Needs review
FileSize
9.6 KB
13.3 KB

pull before you roll....

dawehner’s picture

--- b/composer.lock
+++ a/composer.lock
@@ -2742,67 +2742,6 @@
             ],
...
--- a/composer.lock
+++ b/composer.lock

Nice!

+++ b/core/tests/README.md
@@ -37,6 +32,58 @@ User <your-user>
+
+```
+brew install selenium-server-standalone
+brew install chromedriver
+```
+
+* Before running tests make sure that selenium-server is running
+```
+selenium-server -port 4444
+```
+
+* Set the correct driver args and run the tests:
+```
+export MINK_DRIVER_ARGS_WEBDRIVER='["chrome", null, "http://localhost:4444/wd/hub"]'
+./vendor/bin/phpunit -v -c  --testsuite functional-javascript
+```

That's a much nicer flow! Thank you

Lendude’s picture

Missed the -c changed, but updated the phpunit commands to be the same now.

jibran’s picture

Status: Needs review » Reviewed & tested by the community

Docs look much better now. Thank you!

droplet’s picture

This is even neater and for every platform than MacOS:
https://github.com/vvo/selenium-standalone

If we run out of time, simply 2 lines:

npm install selenium-standalone@latest -g
selenium-standalone install

If we have little time to polish it:
- Add a npm script (install part only). Even a single line adding to package.json

npm install selenium-standalone@latest -g && selenium-standalone install

In the future follow up:
- A npm script for config
- A npm script for test runner

Thanks

claudiu.cristea’s picture

Yesterday I tried to write a JS test for #2921810: Allow TimestampFormatter to show as a fully cacheable time difference with JS and I could not run it with PhantomJS because it seems that PhantomJS has a limitation/bug when the scripts are using setTimeout() & Friends. Applying this patch, I managed to run the test with Selenium. So, +1 to see this in.

alexpott’s picture

Issue summary: View changes
Issue tags: +Needs change record

The patch looks fantastic and works great with the instructions on OS X. I think we should file a follow up add instructions for Linux and OS X. Plus this issue definitely needs a change record. Once we have those things we should commit. It is fabulous watching the test run in chrome! Nice work everyone.

alexpott’s picture

Made a start on the CR https://www.drupal.org/node/2941464 - it needs work.

larowlan’s picture

  1. +++ b/core/tests/Drupal/FunctionalJavascriptTests/JavascriptTestBase.php
    @@ -38,6 +47,9 @@ protected function initMink() {
    +    catch (DriverException $e) {
    +      $this->markTestSkipped($e->getMessage());
    +    }
    

    Can we add a comment here as to why we're doing this?

  2. +++ b/core/tests/README.md
    @@ -37,6 +32,58 @@ User <your-user>
    +* Perform the required installs once:
    +
    +```
    +brew install selenium-server-standalone
    +brew install chromedriver
    +```
    +
    +* Before running tests make sure that selenium-server is running
    ...
    +selenium-server -port 4444
    

    should we point out that these brew commands are for OSX and provide links for Windows/Linux users. We know they are, but not everyone will.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: -Needs change record
Related issues: +#2941560: Add testing with webdriver instructions for other operating systems

I've finished the change record and added the follow-up #2941560: Add testing with webdriver instructions for other operating systems. We still need to added #130

alexpott’s picture

Status: Needs work » Needs review
FileSize
1.94 KB
12.57 KB

Addressed #130. I don't think specifically catching the DriverException is required. The next catch is:

    catch (\Exception $e) {
      $this->markTestSkipped('An unexpected error occurred while starting Mink: ' . $e->getMessage());
    }

Which I think addresses why the test is skipped. Something failed in starting Mink / Webdriver and the test itself has not failed.

Also fixed a minor coding standards issue.

alexpott’s picture

I just ran through the instructions again and got confused because all but 1 of the tests were skipped. That's because they are legacy. I think we should include instructions about how to force all BTBs to use webdriver.

I've also added docs to phpunit.xml.dist since this is a good place to configure stuff.

Lendude’s picture

Status: Needs review » Reviewed & tested by the community

Nice additions to the docs, looks great. Some small tweaks to the CR.

Looks ready again.

droplet’s picture

alexpott’s picture

Adding credit for patch review, patch testing and discussion about the wider issue.

alexpott’s picture

Version: 8.6.x-dev » 8.5.x-dev

Committed 6341df3 and pushed to 8.6.x. Thanks!

Moving this back to 8.5.x and going to ask other committers about backporting this to 8.5.x. I think we should as it allows better JS testing and to keep tests in-sync across 8.5.x and 8.6.x.

  • alexpott committed 6341df3 on 8.6.x
    Issue #2775653 by dawehner, droplet, michielnugter, Lendude, jibran,...

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 133: 2775653-133.patch, failed testing. View results

Mixologic’s picture

Status: Needs work » Reviewed & tested by the community

patch failed because it was already applied.

One word of caution on setting MINK_DRIVER_CLASS -> that will override all mink classes at this point, not just phantomjs->selenium, i.e. all the BTB tests that rely on Goutte will also switch over to whatever that variable is set to.

tstoeckler’s picture

Awesome work! Glad that we got this in.

Found #2941862: Overriding both Mink driver class *and* args does not work for JS tests while playing around with this. That's a related, but pre-existing bug with JS-testing so should not hold up backport in any way, though.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Discussed backporting with other committers. @catch and @webchick are +1.

Committed 10ee6eb and pushed to 8.5.x. Thanks!

  • alexpott committed 10ee6eb on 8.5.x
    Issue #2775653 by dawehner, droplet, michielnugter, Lendude, jibran,...
jibran’s picture

What is the next step here? Do we have an issue to switch over core JS testing from PhantomJSDriver to DrupalSelenium2Driver on CI?

alexpott’s picture

@jibran swap the existing tests to use webdriver. However some fail so will need adjusting.

Mixologic’s picture

It's like Christmas all over again. So happy this is in both!

droplet’s picture

Only 2 or 3 fails in the old day:
https://www.drupal.org/project/drupal/issues/2775653#comment-11646973

not sure if anyone has a powerful machine can help to re-run it again for us or let's find a workaround for everyone:
#2937380: Maintain popular CI presets to improve developer experience

- PhantomJS used a different way to measure windows size. (@see: https://www.drupal.org/files/issues/testbots.patch)
- CKEditor error: #2903623: BigPipe test hardening: check that CKEDITOR exists to prevent script throws error immediately

If you let me pick one, I will close the following issue first for a long run in the future:
#1988968: Drupal.ajax does not guarantee that "add new JS file to page" commands have finished before calling said JS

Mixologic’s picture

@droplet : I started some conversion work here #2942900: Convert JavascriptTestBase Tests to use DrupalSelenium2Driver, to see how much we can convert for free: my first result shows that 48/63 tests are passing, so there are only 15 tests that need work to convert core entirely.

The JS tests do not require that powerful of a machine as they are currently running serially on the testbots because phantom cannot be run in parallel. i.e. you're not going to see a significant speed boost on another CI system vs locally, other than you can set it and forget it remotely and come back to it later.

jibran’s picture

Did anyone try this on travis?

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

heddn’s picture

Shouldn't we updated https://www.drupal.org/node/2116263 as well?

Mixologic’s picture

@heddn: indeed. Many places need docs updating: https://www.drupal.org/project/drupal/issues/2941560