Convert ViewKernelTestBase to use KernelTestBaseTNG

I doubt this will work with the changes we made to the test location ... It would be though great to see whether the views tests are passing.

Did you tried running them?

@martin107, Check #2553661-6: KernelTestBase fails to set up FileCache. I'm getting too the Required prefix configuration is missing apparently for no reason. Or I'm missing something from the picture. I only found the docs from \Drupal\KernelTests\KernelTestBase.

I think this can be unpostponed now.

There is a new base class now, now we have to move all tests that extend the old one.

Most are passing, not sure yet about some remaining fails:

Time: 6.2 minutes, Memory: 116.00Mb

There were 5 errors:

1) Drupal\Tests\language\Kernel\Views\RenderedEntityTest::testRenderedEntityWithoutField
Error: Call to a member function preview() on null

/home/berdir/Projekte/d8/core/modules/language/tests/src/Kernel/Views/RenderedEntityTest.php:133

2) Drupal\Tests\language\Kernel\Views\RenderedEntityTest::testRenderedEntityWithField
Error: Call to a member function preview() on null

5) Drupal\Tests\views\Kernel\ViewExecutableTest::testDisplays
RuntimeException: TestBase::$assertions property no longer exists

/home/berdir/Projekte/d8/core/tests/Drupal/KernelTests/KernelTestBase.php:1128
/home/berdir/Projekte/d8/core/modules/views/tests/src/Kernel/ViewExecutableTest.php:239

--

There were 5 failures:

1) Drupal\Tests\comment\Kernel\Views\CommentLinksTest::testLinkApprove
Found a comment approve link for an unapproved comment.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<a href="/comment/1/approve?destination=/%253Cnone%253E&amp;token=95a2a82a8d6d8e8a35b9c44bb294e5c50e4c745f">Approve</a>'
+'<a href="/comment/1/approve?destination=/&amp;token=95a2a82a8d6d8e8a35b9c44bb294e5c50e4c745f">Approve</a>'

/home/berdir/Projekte/d8/core/tests/Drupal/KernelTests/KernelTestBase.php:1156
/home/berdir/Projekte/d8/core/tests/Drupal/KernelTests/AssertLegacyTrait.php:62
/home/berdir/Projekte/d8/core/modules/comment/tests/src/Kernel/Views/CommentLinksTest.php:70

3) Drupal\Tests\views\Kernel\Handler\FieldEntityLinkTest::testEntityLink
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<a href="/entity_test/manage/1/edit?destination=/" hreflang="en">Edit entity test</a>'
+'<a href="/entity_test/manage/1/edit?destination=/%253Cnone%253E" hreflang="en">Edit entity test</a>'

/home/berdir/Projekte/d8/core/tests/Drupal/KernelTests/KernelTestBase.php:1156
/home/berdir/Projekte/d8/core/tests/Drupal/KernelTests/AssertLegacyTrait.php:62
/home/berdir/Projekte/d8/core/modules/views/tests/src/Kernel/Handler/FieldEntityLinkTest.php:125
/home/berdir/Projekte/d8/core/modules/views/tests/src/Kernel/Handler/FieldEntityLinkTest.php:80

4) Drupal\Tests\views\Kernel\ModuleTest::testLoadFunctions
Views::getAllViews works as expected.
Failed asserting that Array &0 (
    0 => 'archive'
    1 => 'test_argument'
    2 => 'test_view'
    3 => 'test_view_status'
    4 => 'user_admin_people'
    5 => 'who_s_new'
    6 => 'who_s_online'
    7 => 'content'
    8 => 'content_recent'
    9 => 'frontpage'
    10 => 'glossary'
) is identical to Array &0 (
    0 => 'archive'
    1 => 'content'
    2 => 'content_recent'
    3 => 'frontpage'
    4 => 'glossary'
    5 => 'test_argument'
    6 => 'test_view'
    7 => 'test_view_status'
    8 => 'user_admin_people'
    9 => 'who_s_new'
    10 => 'who_s_online'
).

/home/berdir/Projekte/d8/core/tests/Drupal/KernelTests/AssertLegacyTrait.php:82
/home/berdir/Projekte/d8/core/modules/views/tests/src/Kernel/ModuleTest.php:161

5) Drupal\Tests\views\Kernel\Plugin\RowRenderCacheTest::testAdvancedCaching
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<a href="/node/1/edit?destination=/" hreflang="en">edit</a>'
+'<a href="/node/1/edit?destination=/%253Cnone%253E" hreflang="en">edit</a>'

/home/berdir/Projekte/d8/core/tests/Drupal/KernelTests/KernelTestBase.php:1156
/home/berdir/Projekte/d8/core/tests/Drupal/KernelTests/AssertLegacyTrait.php:62
/home/berdir/Projekte/d8/core/modules/views/tests/src/Kernel/Plugin/RowRenderCacheTest.php:154
/home/berdir/Projekte/d8/core/modules/views/tests/src/Kernel/Plugin/RowRenderCacheTest.php:94

FAILURES!
Tests: 2483, Assertions: 9102, Errors: 5, Failures: 5.

Some weird thing with none in some URL's, order of views doesn't match, one test trying to mess with $this->assertions, and RenderedEntityTest fatals. Note that some errors/fails are missing that weren't related to this.

<3 <3 <3 <3 <3 <3
<3 <3 <3 <3 <3 <3
<3 <3 <3 <3 <3 <3
<3 <3 <3 <3 <3 <3

Note: the fixes in #2679096: Convert Kernel Tests in Drupal\system\Tests\Entity to phpunit should fix some of the current test fails.

Here are some fix for the other test failures. Review is coming as well.

1. +++ b/core/modules/language/tests/src/Kernel/Views/LanguageTestBase.php
   index 136a8e2..137de83 100644
   --- a/core/modules/views/src/Tests/Handler/FieldRenderedEntityTest.php
   
   --- a/core/modules/views/src/Tests/Handler/FieldRenderedEntityTest.php
   +++ b/core/modules/language/tests/src/Kernel/Views/RenderedEntityTest.php
   
   +++ b/core/modules/language/tests/src/Kernel/Views/RenderedEntityTest.php
   +++ b/core/modules/language/tests/src/Kernel/Views/RenderedEntityTest.php
   @@ -2,42 +2,42 @@
   
   @@ -2,42 +2,42 @@
    
    /**
     * @file
   - * Contains \Drupal\views\Tests\Handler\FieldRenderedEntityTest.
   + * Contains \Drupal\Tests\language\Kernel\Views\RenderedEntityTest.
     */
    
   -namespace Drupal\views\Tests\Handler;
   +namespace Drupal\Tests\language\Kernel\Views;
   ...
    use Drupal\Core\Entity\Entity\EntityViewMode;
    
    /**
   - * Tests the core Drupal\views\Plugin\views\field\RenderedEntity handler.
   + * Tests the Drupal\entity\Plugin\views\field\RenderedEntity handler.
     *
   - * @group views
   + * @group entity
     */
   -class FieldRenderedEntityTest extends ViewKernelTestBase {
   +class RenderedEntityTest extends ViewsKernelTestBase {
    
   

   I'm confused by this one, maybe git is just tricking us, but note, this one also fails

2. +++ b/core/modules/views/src/Plugin/views/field/Field.php
   index d27a05f..0000000
   --- a/core/modules/views/src/Tests/Plugin/PluginKernelTestBase.php
   
   --- a/core/modules/views/src/Tests/Plugin/PluginKernelTestBase.php
   +++ /dev/null
   

   Should we keep that around for a bit and deprecate it for 8.2.x?

3. +++ b/core/modules/views/tests/src/Kernel/AreaTextTest.php
   similarity index 87%
   rename from core/modules/views/src/Tests/Handler/AreaTitleTest.php
   
   rename from core/modules/views/src/Tests/Handler/AreaTitleTest.php
   rename to core/modules/views/tests/src/Kernel/AreaTitleTest.php
   

   Note: This applies to multiple examples: It is sad that we loose the Handler namespace in those tests.

4. +++ b/core/modules/views/tests/src/Kernel/EventSubscriber/ViewsEntitySchemaSubscriberIntegrationTest.php
   @@ -87,8 +87,6 @@ protected function setUp() {
   -
   -    $this->installSchema('system', 'key_value_expire');
   

   Why is that no longer needed?

5. +++ b/core/modules/views/tests/src/Kernel/FieldDateTest.php
   @@ -126,7 +126,7 @@ public function testFieldDate() {
          'raw time span' => $time_since,
   -      'inverse time span' => -$time_since,
   +      'inverse time span' => "-$time_since",
          'time span' => t('%time ago', array('%time' => $time_since)),
        );
        $this->assertRenderedDatesEqual($view, $intervals);
   @@ -135,7 +135,7 @@ public function testFieldDate() {
   
   @@ -135,7 +135,7 @@ public function testFieldDate() {
        $time = gmmktime(0, 0, 0, 1, 1, 2050);
        $formatted = $this->container->get('date.formatter')->formatTimeDiffUntil($time);
        $intervals = array(
   -      'raw time span' => -$formatted,
   +      'raw time span' => "-$formatted",
   

   That is pretty phenomenal, indeed

1. Yes, I'm confused too, will check again. I think something is indeed wrong there.
2. Fine with me.
3. That was an accident, I'll check and fix them.
4. Because the new base class adds that table already.

1. +++ b/core/modules/aggregator/tests/src/Kernel/Views/IntegrationTest.php
   @@ -120,7 +120,7 @@ public function testAggregatorItemView() {
          });
   -      $this->assertEqual($output, $expected_link, 'Ensure the right link is generated');
   +      $this->assertEqual($output, $expected_link->getGeneratedLink(), 'Ensure the right link is generated');
    
   

   fairly certain this wouldn't be needed with the safe casting, it didn't fail for me locally, but I'm also fine with making it more explicit.

2. +++ b/core/modules/comment/tests/src/Kernel/Views/CommentLinksTest.php
   @@ -31,6 +31,15 @@ class CommentLinksTest extends CommentViewsKernelTestBase {
   +   */
   +  protected function setUp($import_test_views = TRUE) {
   +    parent::setUp($import_test_views);
   +
   +    \Drupal::service('router.builder')->rebuild();
   +  }
   

   Interesting, why was this not necessary before? should the base class do it maybe?

3. +++ b/core/modules/field/tests/src/Kernel/Views/EntityReferenceRelationshipTest.php
   @@ -2,10 +2,10 @@
    
   -namespace Drupal\Tests\entity_reference\Kernel\Views;
   +namespace Drupal\Tests\field\Kernel\EntityReference\Views;
   

   oops.

4. +++ b/core/modules/field/tests/src/Kernel/Views/EntityReferenceRelationshipTest.php
   @@ -17,9 +17,9 @@
   + * @group entity_reference
     */
   

   I guess that's why I messed that up, should we really keep entity_reference as a group?

5. +++ b/core/modules/views/tests/src/Kernel/ModuleTest.php
   @@ -158,19 +158,20 @@ public function testLoadFunctions() {
        // Test Views::getAllViews().
   -    $this->assertIdentical(array_keys($all_views), array_keys(Views::getAllViews()), 'Views::getAllViews works as expected.');
   +    ksort($all_views);
   +    $this->assertEquals(array_keys($all_views), array_keys(Views::getAllViews()), 'Views::getAllViews works as expected.');
   

   I was wondering about adding that, but I'm not sure why it is different. Maybe some unstable sort thing? But I think the order doesn't matter here for us.

6. +++ b/core/modules/views/tests/src/Kernel/ViewExecutableTest.php
   @@ -236,10 +236,9 @@ public function testDisplays() {
   +    // Error is triggered while calling the wrong display.
   +    $this->setExpectedException(\PHPUnit_Framework_Error::class);
        $view->setDisplay('invalid');
   -    $count_after = count($this->assertions);
   -    $this->assertTrue($count_after - $count_before, 'Error is triggered while calling the wrong display.');
    
        $this->assertEqual($view->current_display, 'default', 'If setDisplay is called with an invalid display id the default display should be used.');
   

   will this then even call what's below?

(setting to review for testbot)

fairly certain this wouldn't be needed with the safe casting, it didn't fail for me locally, but I'm also fine with making it more explicit.

Yeah this is the reason I went with that.

I guess that's why I messed that up, should we really keep entity_reference as a group?

I don't care, well it should be in views IMHO, the entity reference module is super deprecated already.

Interesting, why was this not necessary before? should the base class do it maybe?

I was experimenting a bit, at the end indeed just the destination->set() is actually needed.

I was wondering about adding that, but I'm not sure why it is different. Maybe some unstable sort thing? But I think the order doesn't matter here for us.

Yeah, I beleive the order in which we install modules changes from the different kernel tests.

will this then even call what's below?

Good question! Ideally we should split this up into its own test method anyway.

There we go, some work on that.

I think I messed up a few things while creating the initial patch. No idea where RenderedEntityTest comes from, there's no such thing anyway. Removed. Also cleaned up a few wrong changes.

Seems ready to me. Just minor observation can be fixed on commit.

+++ b/core/tests/Drupal/KernelTests/KernelTestBase.php
@@ -17,6 +17,7 @@
+use Drupal\Core\DependencyInjection\DependencySerializationTrait;

Where are we using it?

Needs a reroll

+++ b/core/lib/Drupal/Core/DependencyInjection/DependencySerializationTrait.php
@@ -49,6 +49,10 @@ public function __sleep() {
+    // Tests in isolation potentially unserialize in the parent process.
+    if (isset($GLOBALS['__PHPUNIT_BOOTSTRAP']) && !\Drupal::hasContainer()) {
+      return;
+    }

Given #29 Is this necessary then? Or does this change just make life easier?

@alexpott: It's not necessary when the tests are passing. But without it, phpunit dies *very* badly if it's trying to display the error, which makes writting/debugging tests extremely painful.

We could open a separate issue but now that we've converted many tests to phpunit and more are in the pipeline, it's pretty important to have this fixed.

If you want to try it for yourself, try one of the earlier patches that had fails and didn't have this part.

@Berdir yep I think I've seen the error - I'm happy for this to happen here... you are right it is painful.

Given #29 Is this necessary then? Or does this change just make life easier?

Well, it breaks when you store an entity for example on the test class. Once there, it will be serialized and then unserialized when the error is rendered, which is in the phpunit parent process, which doesn't have a container. Yes, it makes our life easier, but by easier this is actually the difference between hunting and living in a modern society.

Rerolled

Back to RTBC.

Committed aebd0a8 and pushed to 8.1.x and 8.2.x. Thanks!

diff --git a/core/modules/views/tests/src/Kernel/Handler/FilterStringTest.php b/core/modules/views/tests/src/Kernel/Handler/FilterStringTest.php
index d92fde3..27ca0db 100644
--- a/core/modules/views/tests/src/Kernel/Handler/FilterStringTest.php
+++ b/core/modules/views/tests/src/Kernel/Handler/FilterStringTest.php
@@ -7,7 +7,6 @@
 
 namespace Drupal\Tests\views\Kernel\Handler;
 
-use Drupal\views\Tests\Handler\view;
 use Drupal\Tests\views\Kernel\ViewsKernelTestBase;
 use Drupal\views\Views;
 
diff --git a/core/tests/Drupal/KernelTests/KernelTestBase.php b/core/tests/Drupal/KernelTests/KernelTestBase.php
index 43d6036..160c84d 100644
--- a/core/tests/Drupal/KernelTests/KernelTestBase.php
+++ b/core/tests/Drupal/KernelTests/KernelTestBase.php
@@ -17,7 +17,6 @@
 use Drupal\Core\Config\StorageInterface;
 use Drupal\Core\Database\Database;
 use Drupal\Core\DependencyInjection\ContainerBuilder;
-use Drupal\Core\DependencyInjection\DependencySerializationTrait;
 use Drupal\Core\DependencyInjection\ServiceProviderInterface;
 use Drupal\Core\DrupalKernel;
 use Drupal\Core\Entity\Sql\SqlEntityStorageInterface;

Fixing some unused uses on commit.

Had to roll this back because contrib tests are broken :( see https://dispatcher.drupalci.org/job/default/98960/console err no