Support from Acquia helps fund testing for Drupal Acquia logo

Comments

michielnugter created an issue. See original summary.

michielnugter’s picture

Status: Active » Postponed
dawehner’s picture

@michielnugter

Just as a general typ, it would be great to explain why things are postponed. This one here for example isn't obvious for me :)

michielnugter’s picture

Issue summary: View changes
Status: Postponed » Active

I postponed it on a test that required a base class from the system module. I updated the IS to exclude that test from the conversion and unpostponed the issue.

snetcher’s picture

Status: Active » Needs review
FileSize
4.43 KB

Status: Needs review » Needs work

The last submitted patch, 6: 2870440-6.patch, failed testing.

andypost’s picture

Status: Needs work » Needs review
FileSize
3.3 KB
6.55 KB

Few fixes

dawehner’s picture

+++ b/core/modules/config_translation/tests/src/Functional/ConfigTranslationOverviewTest.php
@@ -77,9 +77,9 @@ public function testMapperListPage() {
+      $this->assertIdentical(1, count($dropbutton->findAll('xpath', '/*')));

@@ -104,9 +104,9 @@ public function testMapperListPage() {
+        $this->assertIdentical(1, count($dropbutton->findAll('xpath', '/*')));

You could use assertCount here

Status: Needs review » Needs work

The last submitted patch, 8: 2870440-config_translation-8.patch, failed testing.

michielnugter’s picture

Component: phpunit » config_translation.module
Issue tags: +phpunit initiative
naveenvalecha’s picture

Status: Needs work » Needs review
FileSize
7.58 KB

Rewrote the whole conversion as the above patch was not getting applied.

//Naveen

Status: Needs review » Needs work

The last submitted patch, 12: 2870440-8.patch, failed testing. View results

naveenvalecha’s picture

Do we have any existing follow-up issue where are we adding the helper function drupalPostWithFormat?

1) Drupal\Tests\config_translation\Functional\ConfigTranslationUiTest::testViewsTranslationUI
PHPUnit_Framework_Exception: Fatal error: Call to undefined method Drupal\Tests\config_translation\Functional\ConfigTranslationUiTest::drupalPostWithFormat() in /var/www/html/core/modules/config_translation/tests/src/Functional/ConfigTranslationUiTest.php on line 1146

//Naveen

naveenvalecha’s picture

#14 This patch removed the conversion of the ConfigTranslationUiTest. Updated the issue summary accordingly.

//Naveen

Status: Needs review » Needs work

The last submitted patch, 15: 2870440-15.patch, failed testing. View results

naveenvalecha’s picture

Status: Needs work » Needs review
FileSize
4.98 KB
2.56 KB

Here we go with fixes

//Naveen

dawehner’s picture

+++ b/core/modules/config_translation/tests/src/Functional/ConfigTranslationOverviewTest.php
@@ -77,10 +77,8 @@ public function testMapperListPage() {
     foreach ($this->cssSelect('ul.dropbutton') as $i => $dropbutton) {
-      $this->assertIdentical(1, $dropbutton->count());
-      foreach ($dropbutton->li as $link) {
-        $this->assertTrue(((string) $link->a === 'Translate') || ((string) $link->a === 'List'));
-      }
+      $this->assertIdentical(1, substr_count($dropbutton->getHtml(), '</li>'));
+      $this->assertTrue(($dropbutton->getText() === 'Translate') || ($dropbutton->getText() === 'List'));
     }

@@ -104,10 +102,8 @@ public function testMapperListPage() {
-        $this->assertIdentical(1, $dropbutton->count());
-        foreach ($dropbutton->li as $link) {
-          $this->assertIdentical('Translate', (string) $link->a);
-        }
+        $this->assertIdentical(1, substr_count($dropbutton->getHtml(), '</li>'));
+        $this->assertIdentical('Translate', $dropbutton->getText());
       }
 

What about using $dropbutton->find('li') instead of testing the HTML directly?

naveenvalecha’s picture

yup we could use that. Accommodated #18

$ ../vendor/bin/phpunit modules/config_translation/tests/src/Functional/ConfigTranslationOverviewTest.php 
PHPUnit 4.8.35 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\config_translation\Functional\ConfigTranslationOverviewTest
...

Time: 11.53 minutes, Memory: 6.00MB

OK (3 tests, 72 assertions)

//Naveen

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

I like it, thank you @naveenvalecha!

  • Gábor Hojtsy committed 45d2411 on 8.3.x
    Issue #2870440 by naveenvalecha, andypost, snetcher, michielnugter,...

  • Gábor Hojtsy committed 4d45aab on 8.4.x
    Issue #2870440 by naveenvalecha, andypost, snetcher, michielnugter,...
Gábor Hojtsy’s picture

Version: 8.4.x-dev » 8.3.x-dev
Status: Reviewed & tested by the community » Fixed

Thanks all, agreed it looks good.

Status: Fixed » Closed (fixed)

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