Problem/Motivation

Currently we use Simpletest to do stuff like drupalPostAjaxForm() which fakes AJAX requests. That is not ideal because we should test the real Javascript interaction and now we have JavascriptTestBase to do that.

Therefore we should convert our tests covering Javascript and AJAX functionality to use JavascriptTestBase.

Proposed resolution

Open sub-issues to conversions for each module where the tests use drupalPostAjaxForm().

Remaining tasks

Create sub-issues, review them, commit them.

User interface changes

none.

API changes

none.

Data model changes

none.

Comments

klausi created an issue. See original summary.

dawehner’s picture

Assigned: Unassigned » dawehner

I'll work on creating the necessary subissues.

jhodgdon’s picture

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?

dawehner’s picture

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.

dawehner’s picture

Parent issue: » #2807237: PHPUnit initiative
jhodgdon’s picture

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?

klausi’s picture

Of course you can always assert text on a page with ->assertSession()->pageTextContains() in JavascriptTestBase, so that should not be a problem.

The usual test will trigger some javascript interaction by clicking somewhere, then call a method to wait until something is visible or has changed, then assert text or whatever. There is also assertWaitOnAjaxRequest() to explicitly wait until an AJAX request is completed. I would encourage you to look at all the existing tests extending JavascriptTestBase in core to get a feeling what you can do and what a test usually looks like.

jhodgdon’s picture

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.

dawehner’s picture

dawehner’s picture

Thank you @claudiu.cristea!

claudiu.cristea’s picture

#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.

claudiu.cristea’s picture

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?

dawehner’s picture

Assigned: dawehner » Unassigned

@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.

claudiu.cristea’s picture

@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.

claudiu.cristea’s picture

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

michielnugter’s picture

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.

dawehner’s picture

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.

michielnugter’s picture

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?

dawehner’s picture

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.

michielnugter’s picture

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 AJAX part of \Drupal\field\Tests\NestedFormTest::testNestedFieldForm to JavascriptTestBase.

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 AJAX part of \Drupal\comment\Tests\CommentPagerTest to JavascriptTestBase 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.

droplet’s picture

#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.

dawehner’s picture

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.

michielnugter’s picture

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.

nod_’s picture

klausi’s picture

JavascriptTestBase has at least 3 random test fails right now, so we need to stop converting tests to JavascriptTestBase until #2830485: \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest fails randomly is resolved. Sorry!

klausi’s picture

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.

michielnugter’s picture

Status: Postponed » Active

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.

michielnugter’s picture

Issue tags: +phpunit initiative