Convert Javascript/AJAX testing to use JavascriptTestBase

I'll work on creating the necessary subissues.

I took a quick look at JavascriptTestBase.

So I am currently using simpletest's WebTestBase to make automated screenshots for the User Guide. There are two screenshots in there that use drupalPostAjaxForm, which is currently not part of JavascriptTestBase. They are taking screenshots of settings areas of the Field UI's Manage Fields page -- after you click the gear icon, these areas appear in blue and let you set the settings.

I am a bit confused as to whether JavascriptTestBase would serve for this application. I don't want to test whether the JavaScript or Ajax is working. What I want to do is have a scripted way to get the current page in my WebTestBase internal browser into a certain state, so that I can verbose-output the page and take a screenshot of it. Currently, drupalPostAjaxForm allows me to do this, because after posting the Ajax request, it fixes up the page in a way that is at least hopefully similar to what ajax.js does.

I realize this is not the intended use of WebTestBase or BrowserTestBase or JavascriptTestBase... but it sure is a handy way to automate screenshots for the User Guide, in English and other languages.

I plan to also put some more testing code in the test class I'm using for this, so that we can test that UI things we mention in the User Guide are still present as Drupal evolves. This will let use basically have a regression test for "Does the User Guide still match the current Drupal user interface?"... at least, as much as possible. In this case, we would also want to have the Ajax processed, so that after the Ajax request, we could see that the field labels are present, etc.

Anyway... Are there plans to have JavascriptTestBase actually process the Ajax in the internal browser using the ajax.js file?

Anyway... Are there plans to have JavascriptTestBase actually process the Ajax in the internal browser using the ajax.js file?

At least the agreement seem to be that there is no usecase for that. On the other hand I strongly believe that with the abstraction layers of BTB you can achieve whatever you need.

I have one use case for it (#3), but of course I could get around this by adopting something similar to WebTestBase::drupalProcessAjaxResponse() in my "test" class (which is obviously pushing the definitions of "test" a bit and going beyond the intended use of any *Test classes)...

Is there no value in making tests that do assertText() and friends after an Ajax request to verify that the page looks as expected though? I would think that Field UI and Views UI would want to have tests like that, if it were possible to do. But... I guess I am unsure as to what the intention of JavascriptTestBase is, if it's not to be able to test that the JavaScript on the page is working correctly?

Are there any guidelines/instructions for converting existing WebTestBase tests using drupalPostAjaxForm() to use JavascriptTestBase? With just the class docs for JavascriptTestBase to go on, it's not at all obvious. They only say:

Runs a browser test using PhantomJS.

Base class for testing browser interaction implemented in JavaScript.

And then there are all of 4 methods added to BrowserTestBase.

@jhodgdon
I opened up an issue for that: #2810621: Improve core.api.php documentation about browsertests and javascript tests

Added also #2820079: Convert \Drupal\book\Tests\BookTest::testBookOrdering() to a JavascriptTestBase test, split from #2757007: Convert all book web tests to BrowserTestBase.

Thank you @claudiu.cristea!

#2787923: Add a getDrupalSettings() method to JavascriptTestBase is a blocker for many of the child conversions because some Ajax requests are changing the page settings.

For many of the Ajax tests, I don't think they should be converted but rather extended. I think the actual tests are important because they prove how the functionality works with disabled Javascript. So, in such cases they should not removed but expanded with a sibling JTB test. Opinions?

@claudiu.cristea
Are you sure they really test the non JS behaviour? These ajax tests do use the ajax request format, but then implement this weird JS in php logic. Given that this is just triggering the JS code path, and not the non JS code path. In general though I agree, we should evaluate the individual tests and see whether they might have specific test coverage we don't want to loose.

@dawehner, I'm converting now \Drupal\ckeditor\Tests\CKEditorAdminTest::testExistingFormat(). That test is posting the form using .js-hide button but, true, using $this->drupalPostAjaxForm(). However, we need that kind of testing while it assures the non-JS functionality.

Probably we need to keep them but convert them to pure non-JS testing.

I've been working on several tests in this issue and on several other issues requiring functional tests.

One thing I'm now wondering if we should create a functional test to cover ajax.js itself. Currently it only seems to have limited tests and there is no test module to use in these tests. All coverage so far is indirect by testing various parts of Drupal.
This is quite a large undertaking but then again it might just make several of the tests here redundant.

There already is 1 issue requiring a good test module for ajax.js here: #2215857: Behaviors get attached to removed forms.

One thing I'm now wondering if we should create a functional test to cover ajax.js itself.

OH that would be totally great! On the longrun I could imagine ajax.js would be more of a unit test, but that's something to be considered in the future.

I think in the long term both should be required as you also want to test the interaction between the page and the server via ajax.js.

A unit test is quite hard with the current code base, only some parts (Ajax commands) are unit testable I think.

I propose to open an issue for the conversion which contains the plan with sub issues for each domain within ajax.js. This way we can cover one small part of ajax.js now and get something in to base other tests on. Do you agree?

A unit test is quite hard with the current code base, only some parts (Ajax commands) are unit testable I think.

Oh yeah no question, there is other work happening which might make that feasible.

I propose to open an issue for the conversion which contains the plan with sub issues for each domain within ajax.js. This way we can cover one small part of ajax.js now and get something in to base other tests on. Do you agree?

Sure why not. Please ping also @nod_ about it, he might have plans/more wisdom already.

I pinged @nod_ about it and asked if he could take a look at this issue.

I've been working some more on the tests and am wondering if we're using the right approach. Some tests on the list to be converted only use the core JavaScript functionality, one example is #2809489: Convert \Drupal\field\Tests\NestedFormTest to BrowserTestBase.

Am I right in assuming that the testcoverage can be dropped in specific places if we have test coverage for the core files themselves? For example, drag-n-drop and ajax command usage won't be functionally tested in specific places. The #2809467: Convert \Drupal\comment\Tests\CommentPagerTest to BrowserTestBase is an example of this. The Functional tests in the comment module should only test the Javascript provided in these modules. We could keep the non-js test parts as a browsertest or kernel test and strip the javascript parts.

Some of the core tests will require a lot of work but I think it will save a lot of work for the conversions.

#17 ~ #21

I think it's a generic question in Drupal Tests than JavaScriptTest only. (The tests strategy)

Am I right in assuming that the testcoverage can be dropped in specific places if we have test coverage for the core files themselves?

No? In simple word, I think Drupal has almost no identical features. It does not simply use a JS lib, CSS framework or calling a single line PHP API with identical pattern markups (e.g. Bootstrap framework). But it's extended with custom coding & applying different CSS styles. For example, the drag-n-drop you mentioned, the UIs have differences. (and custom implementation, e.g. Drupal.behaviors.blockDrag. Again, even we forget the JS & PHP, CSS also matter. @see below link)

To me, a good Acceptance test (ATDD) is helping to catch this kind of errors: https://www.drupal.org/node/2789381#comment-11547375

We can't be lazy (https://www.drupal.org/node/2775653#comment-11647431) and cheat yourself (in Phantomjs-like simulate browser env). e.g.,
#2773791: Clicking elements with children in Javascript tests throws a GastonJS exception
#2820079: Convert \Drupal\book\Tests\BookTest::testBookOrdering() to a JavascriptTestBase test

Extra Developer (perspectives) TDD & UnitTest enhance the tests to test more potential usages & errors.

I think I agree with @droplet here. Drupal is a complex beast, you never know what happens.
Another point you should keep in mind, this issue is about converting, so the scope of the changes should be kept minimal, if possible.

Given that I think its worth to file an issue, discussing the potential duplicate coverage, but keep the conversions as parallel as possible, so they are sort of reviewable.

No question though, really detailed test coverage for ajax.js/tabledrag.js would be worth for itself.

Thanks for your replies and insights.

It sure is complex and on some parts specific tests are valuable even if the specifics don't have custom javascript. The combination of several core parts might cause problems of their own.

I agree completely on keeping the scope limited and having a lot of variants on testing various parts of core will make it easier later on to add proper coverage for the core parts.

Maybe later if we have detailed test coverage for the core javascript we can open issues to remove duplicate testing as this will improve performance.

For now I'll pitch in on converting all the tests. I hope to hear from @nod_ if there is an existing plan to add test coverage for core. If not I can create a meta and all the required issues.

We have #2702747: Add javascript unit testing #2815199: Add tools and scripts for writing and running javascript unit tests #2232271: [Meta] Use Behat for validation testing

Overall im with droplet on this.

Most of the random failures are now under control and we have a much better understanding on when and why they occur and how to fix them. Setting back to active.

Postponing for now until we figure out and decide on 2 things.

1. Do we convert the tests to either BrowserTestBase or JavascriptTestBase or both?

claudiu.cristea wrote earlier:

For many of the Ajax tests, I don't think they should be converted but rather extended. I think the actual tests are important because they prove how the functionality works with disabled Javascript. So, in such cases they should not removed but expanded with a sibling JTB test. Opinions?

Probably we need to keep them but convert them to pure non-JS testing.

I happen to agree but with an extra note. Right now neither the Javascript or non-javascript versions are really tested, only a sort of in the middle variant with drupalPostAjaxForm.

I think each test should be converted to BrowserTestBase and actually test the non-javascript variant. On top of that we should add test coverage for all of the Javascript.

2. Are we certain that we will continue to use PHPUnit+Mink for our Javascript tests or are we going to switch to a pure Javascript solution

Some experiments have been started with alternatives here #2869825: Leverage JS for JS testing (using nightwatch) and @droplet suggested some other options here:

https://www.drupal.org/node/2775653#comment-12065753

As another alternative I put forward Selenium.

Do we need to wait for a decision on this or should we continue adding JavascriptTestBase coverage? I'm in doubt because it's such a lot of work to be throwing away if we do switch later on..

For many of the Ajax tests, I don't think they should be converted but rather extended. I think the actual tests are important because they prove how the functionality works with disabled Javascript. So, in such cases they should not removed but expanded with a sibling JTB test. Opinions?

I really have a problem to understand that. drupalPostAjaxForm doesn't actually execute the PHP side of things, but rather it """emulates""" a browser.

I think each test should be converted to BrowserTestBase and actually test the non-javascript variant. On top of that we should add test coverage for all of the Javascript.

I agree on the longrun. For pure conversions though we need to keep in mind that we don't actually have any test coverage for the NON JS codepath at the moment.
In general I believe we should focus our efforts on the codepath which is more likely to be executed, aka. the JS enabled one :)

2. Are we certain that we will continue to use PHPUnit+Mink for our Javascript tests or are we going to switch to a pure Javascript solution

Selenium/webdriver are technically really the same thing, aka. selenium is using the webdriver API to control real browsers, while with chrome headless, you don't even need the selenium middleware. The actual question is then really "JUST" whether we want to use PHP or Javascript to test javascript. To be honest I believe that using PHP to test JS is really weird, and blocks us from a lot of best practise in the JS world.

On the other hand I don't believe we should wait for that issue, but rather continue adding tests to JavascriptTestBase. Expanding the test coverage there has a bigger net effect. Converting these tests over in the future is actually quite valueable, as we now they work already, so we don't have to figure out stuff like waiting for certain classes to appear.

So... Over on #2856747: Convert screenshot automation to JavascriptTestBase, I attempted to convert some tests I had written using WebTestBase (from Simpletest) to JavascriptTestBase. These tests are for the User Guide, and they basically replicate the steps in the User Guide go through to make a sample web site for a farmer's market. The purpose of the tests is both:

(a) Test that UI text in the User Guide is what the User Guide says it is (as we move to new versions of Drupal sometimes have new UIs and we need to update the User Guide when we detect that this has happened)

(b) Create automated screenshots, which are inserted into the User Guide. This is done for quite a few languages.

There was a somewhat complicated process for (b) involving dumping HTML output and making screenshots using a Greasemonkey script and ImageMagick, so I thought maybe JavascriptTestBase would make this simpler.

I worked on this for over a week, and it utterly failed. There were many bugs in JavascriptTestBase and/or PhantomJS that I had to work around (some of which I filed issues for), and in the end, I could not get around some PhantomJS or GastonJS bug that caused the French screenshot process for chapter 3 to fail consistently. I couldn't figure out how to work around it, so I abandoned the process today.

My conclusion was that JavascriptTestBase is not stable enough for me to use in production (i.e., production of screenshots for the User Guide).

So... I would respectfully ask that the Simpletest UI (which I use in screenshot production) and WebTestBase not be deprectated until there is a stable replacement. You can look at the first commit I made on that issue to see what I had to go through to even get the first few screenshots to pass... it wasn't pretty, and it got even worse to make the next section work for most (but not all) of the languages. If Simpletest UI and WebTestBase go away in Drupal 9, we will not be able to make updated screenshots for the User Guide any more, and that would be a shame.

Alternatively, if someone else wants to revive that issue and make a viable system that generates the screenshots, that would be fine, but I don't really think it is possible, with the current classes and PhantomJS having its own bugs.

I'm also wondering if others making JavascriptTestBase test classes are running into similar problems?

Here's the Drupal Core issue I filed, for reference... The other problems I ran into appeared to have issues already, the ones that appear to be internal PhantomJS issues that you cannot really debug or (as far as I can tell) get around.

This is no longer postponed. The direction we have chosen is just to keep converting the ajax parts of test to JavascriptTestBase and now WebDriverTestBase.

Yes some things might not work flawlessly yet, but it seems pretty stable now.

I'm going to mark this fixed (is that the right status for an implemented plan?), since we have moved forward with this, and no planning is required anymore.

Thanks everybody for getting some early perspectives on this.