Convert web tests to browser tests for taxonomy module Part -2

Converting:

• \Drupal\taxonomy\Tests\TermTranslationTest to \Drupal\Tests\taxonomy\Functional\TermTranslationTest
• \Drupal\taxonomy\Tests\RssTest to \Drupal\Tests\taxonomy\Functional\RssTest

[Composer\DependencyResolver\SolverProblemsException]                                                                         
Problem 1                                                                                                                     
       - The requested package phpunit/phpunit ^7.1 exists as phpunit/phpunit[6.5.8] but these are rejected by your constraint.  

Script Drupal\Core\Composer\Composer::upgradePHPUnit handling the drupal-phpunit-upgrade-check event terminated with an exception
  ERROR: PHPUnit testing framework version 6 or greater is required when running on PHP 7.0 or greater. Run the command 'composer run-script drupal-phpunit-upgrade' in order to fix this.

This was the reason the test failed earlier, I tried to retest to try make this pass, but it gave the same issue.

I'm not sure how the entry for phpunit was added to composer, I will remove it and resupply the patch.

Patch will need a reroll after #2969805: Fix "Drupal\taxonomy\Tests\TaxonomyTranslationTestTrait is deprecated in Drupal 8.4.0 and will be removed before Drupal 9.0.0. Instead, use \Drupal\Tests\taxonomy\Functional\TaxonomyTranslationTestTrait", or this issue might make that one a duplicate.

Golly, what a pain: #2767655-54: Allow WebAssert to inspect also hidden fields :-)

I converted TermTest::testTermReorder() before I realized it's supposed to be a JS test. However, I think I got it and we don't need the JS coverage, unless dragging the items around is important.

In RssTest::testTaxonomyRss() I was unable to get the css selector to work, so I verified that it's an RSS feed by checking the Content-Type header.

Bad namespace! No biscuit!

Awesome, this is looking great now.

Thanks. But there are still some tests in taxonomy/src/Tests that we need to convert.

This patch finishes the move. All that remains in taxonomy/src/Tests is a base class and two traits, all deprecated.

It also turns #2969805: Fix "Drupal\taxonomy\Tests\TaxonomyTranslationTestTrait is deprecated in Drupal 8.4.0 and will be removed before Drupal 9.0.0. Instead, use \Drupal\Tests\taxonomy\Functional\TaxonomyTranslationTestTrait" into a duplicate.

@Mile23++ looking great.

1. +++ b/core/modules/taxonomy/tests/src/Functional/RssTest.php
   @@ -106,7 +106,8 @@ public function testTaxonomyRss() {
   -    $this->assertTrue(!empty($this->cssSelect('rss[version="2.0"]')), "Feed page is RSS.");
   +    $assert = $this->assertSession();
   +    $assert->responseHeaderContains('Content-Type', 'application/rss+xml');
   

   Not quite a 1:1 conversion, can we check the version too? Did a quick search and I didn't see any other test checking this, so I feel we are loosing a bit of coverage here.

2. +++ b/core/modules/taxonomy/tests/src/Functional/TaxonomyTermIndentationTest.php
   @@ -42,15 +43,23 @@ public function testTermIndentation() {
   +    // Because we can't post hidden form elements, we have to change them in
   +    // code here, and then submit.
   +    foreach ($hidden_edit as $field => $value) {
   +      $node = $assert->hiddenFieldExists($field);
   +      $node->setValue($value);
   +    }
   

   nice!

3. +++ b/core/modules/taxonomy/tests/src/Functional/TermAutocompleteTest.php
   @@ -125,9 +134,27 @@ protected function setUp() {
   -    $this->drupalGet('node/add/article');
   +    $this->drupalGet(Url::fromRoute('node.add', ['node_type' => 'article']));
   

   Nice refactor, but I'm guessing its unrelated to getting the test to pass?

NW to get an answer on #1

Thanks.

Re: #25

1) Lots of hair-pulling about that. It turns out that $session->getContent()->find() only works for HTML, even using an xpath. Must use $session->getDriver()->find().

2) All praise to @berdir. #2767655-54: Allow WebAssert to inspect also hidden fields

3) Reverted.

Re #26.1: I think it is because of \Behat\Mink\Element\DocumentElement::getXpath that every xpath gets prefixed by //html

This looks good to me.

Committed and pushed 9547959081 to 8.7.x and 917f75bc59 to 8.6.x. Thanks!

This introduced test failures on 8.7.x:
https://www.drupal.org/pift-ci-job/1065875
https://www.drupal.org/pift-ci-job/1065877

Drupal\Tests\taxonomy\Functional\TaxonomyTermIndentationTest
fail: [Other] Line 0 of sites/default/files/simpletest/phpunit-1152.xml:
PHPunit Test failed to complete; Error: PHPUnit 6.5.13 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\taxonomy\Functional\TaxonomyTermIndentationTest
.                                                                   1 / 1 (100%)

Time: 6.11 seconds, Memory: 4.00MB

OK (1 test, 15 assertions)

...
Remaining deprecation notices (1)

  1x: assertNoPattern() is deprecated and scheduled for removal in Drupal 9.0.0. Use $this->assertSession()->responseNotMatches($pattern) instead. See https://www.drupal.org/node/2864262.
    1x in TaxonomyTermIndentationTest::testTermIndentation from Drupal\Tests\taxonomy\Functional

@tacituseu thanks! Reverted from both branches.

Took out the deprecated call.

Not sure how good of an idea it is to do things like #2970052: Fix "assertNoPattern() is deprecated and scheduled for removal in Drupal 9.0.0. Use $this->assertSession()->responseNotMatches($pattern) instead" already, since it will make doing minimal conversions harder and harder.

Changes in #35 addressed #31
Test looks good
+1 for RTBC

+++ b/core/modules/taxonomy/tests/src/Functional/TaxonomyTermIndentationTest.php
@@ -42,15 +43,23 @@ public function testTermIndentation() {
+    // Because we can't post hidden form elements, we have to change them in

No, I don't think we should do this. We are quitting the user perspective, which should be the core of functional tests. My point is that this test should be a JS test. We should do what the user is doing and not hack into the form hidden inputs.

I'm moving back to NR for an evaluation.

@claudiu.cristea the scope here is not to make the test better, it's to convert it to BrowserTestBase. The original WebTestBase did exactly the same thing except that Simpletest is way less concerned about what a user can and cannot do, so it didn't involve any hacky workarounds to submit hidden fields.

Would it be nice to also have a Javascript test for this? Sure, but that is not the scope of this issue.

Moving back to RTBC per #36

@Lendude, till now the rule was to make the conversion for such cases. But if we plan to keep this patch simple, at least we need a followup to convert TaxonomyTermIndentationTest into a JS test.

Filed the follow up for this #2999441: Convert or supplement TaxonomyTermIndentationTest with a javascript test

Committed and pushed dc4adc5723 to 8.7.x and 277a7055e8 to 8.6.x. Thanks!