Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir created an issue. See original summary.

Berdir’s picture

Berdir’s picture

Moved the config tests out of the config module.

Berdir’s picture

And addressing #52.2 from the other issue.

The last submitted patch, 2: kernel-tests-a-i-2686207-2.patch, failed testing.

The last submitted patch, 3: kernel-tests-a-i-2686207-3.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 4: kernel-tests-a-i-2686207-4.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
91.29 KB
768 bytes

This should fix the test fail.

Berdir’s picture

Rerolled. Still applied with git apply -3.

dawehner’s picture

+++ b/core/modules/aggregator/tests/src/Kernel/AggregatorTitleTest.php
@@ -74,7 +76,7 @@ public function testStringFormatter() {
-    $this->assertTrue(strpos($result, 'testing title') === 0);
+    $this->assertTrue(strpos($result, 'testing title') !== FALSE);

@@ -86,7 +88,7 @@ public function testStringFormatter() {
-    $this->assertTrue(strpos($result, 'test title') === 0);
+    $this->assertTrue(strpos($result, 'test title') !== FALSE);

If we change these lines we could use assertContains directly

Berdir’s picture

Sure. Updated all in that test. Certainly results in much better fail output.

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

Looks pretty straightforward to me. Just a couple of comments over 80 chars that can be fixed on commit :)

  1. +++ b/core/modules/system/tests/modules/entity_test/entity_test.module
    @@ -250,7 +250,7 @@ function entity_test_entity_extra_field_info() {
    +      // just used in \Drupal\Tests\field_ui\Kernel\EntityDisplayTest to test the
    

    Here.

  2. +++ b/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php
    @@ -854,7 +854,7 @@ public function testDeleteRevision() {
    +    // Dependencies are tested in \Drupal\Tests\config\Kernel\ConfigDependencyTest.
    

    And here.

alexpott’s picture

  1. +++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigFileContentTest.php
    @@ -121,16 +121,16 @@ function testReadWriteConfig() {
    -    $this->assertEqual($config->get($false_key), '0', "Boolean FALSE value returned the string '0'.");
    +    $this->assertFalse($config->get($false_key), "Boolean FALSE value returned the FALSE.");
    ...
    -    $this->assertEqual($config->get($true_key), '1', "Boolean TRUE value returned the string '1'.");
    +    $this->assertTrue($config->get($true_key), "Boolean TRUE value returned the TRUE.");
    

    I think this exist because we used to case booleans to '0' and '1' - a long time ago - all config was cast to strings. I think we should file a follow up to fix this test to always use assertSame and fix the confusing documentation.

  2. +++ b/core/tests/Drupal/KernelTests/Core/Config/DefaultConfigTest.php
    @@ -23,15 +24,6 @@ class DefaultConfigTest extends KernelTestBase {
    -   * Enable the system module so that system_config_schema_info_alter() fires.
    

    We should file a followup to fix this test to not mention system_config_schema_info_alter() and update the documentation about themes because it is confusing (I probably wrote it).

  3. +++ b/core/tests/Drupal/KernelTests/Core/Config/DefaultConfigTest.php
    @@ -23,15 +24,6 @@ class DefaultConfigTest extends KernelTestBase {
    -  public static $modules = array('system', 'config_test');
    
    @@ -50,8 +42,8 @@ protected function setUp() {
           ->setClass('\Drupal\config_test\TestInstallStorage')
    

    This is neat - we can use classes from modules that are not installed :)

  4. +++ b/core/tests/Drupal/KernelTests/Core/Config/Storage/FileStorageTest.php
    @@ -75,12 +75,6 @@ public function testlistAll() {
    -    // Initialize FileStorage with absolute file path.
    -    $absolute_path = realpath($this->directory);
    -    $storage_absolute_path = new FileStorage($absolute_path);
    -    $config_files = $storage_absolute_path->listAll();
    -    $this->assertIdentical($config_files, $expected_files, 'Absolute path, two config files found.');
    

    How this is a removed - pointlessness?

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 76cf6e2 and pushed to 8.1.x and 8.2.x. Thanks!

Fixed on unused use on commit and stuff from #12

diff --git a/core/modules/filter/tests/src/Kernel/FilterUnitTest.php b/core/modules/filter/tests/src/Kernel/FilterUnitTest.php
index da29312..87988bd 100644
--- a/core/modules/filter/tests/src/Kernel/FilterUnitTest.php
+++ b/core/modules/filter/tests/src/Kernel/FilterUnitTest.php
@@ -13,7 +13,6 @@
 use Drupal\editor\EditorXssFilter\Standard;
 use Drupal\filter\Entity\FilterFormat;
 use Drupal\filter\FilterPluginCollection;
-use Drupal\filter\Tests\FilterInterface;
 use Drupal\KernelTests\KernelTestBase;
 
 /**
diff --git a/core/modules/system/tests/modules/entity_test/entity_test.module b/core/modules/system/tests/modules/entity_test/entity_test.module
index 6451258..9d9d63c 100644
--- a/core/modules/system/tests/modules/entity_test/entity_test.module
+++ b/core/modules/system/tests/modules/entity_test/entity_test.module
@@ -250,8 +250,8 @@ function entity_test_entity_extra_field_info() {
   $extra['entity_test']['bundle_with_extra_fields'] = array(
     'display' => array(
       // Note: those extra fields do not currently display anything, they are
-      // just used in \Drupal\Tests\field_ui\Kernel\EntityDisplayTest to test the
-      // behavior of entity display objects.
+      // just used in \Drupal\Tests\field_ui\Kernel\EntityDisplayTest to test
+      // the behavior of entity display objects.
       'display_extra_field' => array(
         'label' => t('Display extra field'),
         'description' => t('An extra field on the display side.'),
diff --git a/core/tests/Drupal/KernelTests/Core/Config/Storage/CachedStorageTest.php b/core/tests/Drupal/KernelTests/Core/Config/Storage/CachedStorageTest.php
index 7fe1ee0..546899a 100644
--- a/core/tests/Drupal/KernelTests/Core/Config/Storage/CachedStorageTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Config/Storage/CachedStorageTest.php
@@ -10,7 +10,6 @@
 use Drupal\Core\Config\FileStorage;
 use Drupal\Core\Config\CachedStorage;
 use Drupal\Core\DependencyInjection\ContainerBuilder;
-use Drupal\Core\Site\Settings;
 use Drupal\Core\StreamWrapper\PublicStream;
 use Symfony\Component\DependencyInjection\Reference;
 
diff --git a/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php b/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php
index 43903ec..cd589f6 100644
--- a/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php
+++ b/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php
@@ -854,7 +854,8 @@ public function testDeleteRevision() {
    * @covers ::doDelete
    */
   public function testDelete() {
-    // Dependencies are tested in \Drupal\Tests\config\Kernel\ConfigDependencyTest.
+    // Dependencies are tested in
+    // \Drupal\Tests\config\Kernel\ConfigDependencyTest.
     $this->configManager->expects($this->any())
       ->method('getConfigEntitiesToChangeOnDependencyRemoval')
       ->willReturn(['update' => [], 'delete' => [], 'unchanged' => []]);

  • alexpott committed 0db5b13 on 8.2.x
    Issue #2686207 by Berdir: Convert simpletest kernel tests in modules A-I...
alexpott’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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