Issue #3129074 by longwave, mondrake, Lal_, Suresh Prabhu Parkala, jo…

…hnwebdev, jungle, kapilkumar0324: Refactor assertions that use return values in conditionals (cherry picked from commit 15340c889974dd66dec31cbafc430a692653b949)
drupal · May 1, 2020 · cecd8ef · cecd8ef
1 parent b45b752
commit cecd8ef
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 52 deletions.
diff --git a/modules/content_translation/tests/src/Functional/ContentTranslationUITestBase.php b/modules/content_translation/tests/src/Functional/ContentTranslationUITestBase.php
@@ -369,11 +369,10 @@ protected function doTestTranslationDeletion() {
     $this->drupalPostForm(NULL, [], t('Delete @language translation', ['@language' => $language->getName()]));
     $storage->resetCache([$this->entityId]);
     $entity = $storage->load($this->entityId, TRUE);
-    if ($this->assertTrue(is_object($entity), 'Entity found')) {
-      $translations = $entity->getTranslationLanguages();
-      $this->assertCount(2, $translations, 'Translation successfully deleted.');
-      $this->assertEmpty($translations[$langcode], 'Translation successfully deleted.');
-    }
+    $this->assertIsObject($entity);
+    $translations = $entity->getTranslationLanguages();
+    $this->assertCount(2, $translations);
+    $this->assertArrayNotHasKey($langcode, $translations);
 
     // Check that the translator cannot delete the original translation.
     $args = [$this->entityTypeId => $entity->id(), 'language' => 'en'];

diff --git a/modules/file/tests/src/Functional/DownloadTest.php b/modules/file/tests/src/Functional/DownloadTest.php
@@ -174,9 +174,8 @@ private function checkUrl($scheme, $directory, $filename, $expected_url) {
     }
 
     $this->drupalGet($url);
-    if ($this->assertResponse(200) == 'pass') {
-      $this->assertRaw(file_get_contents($file->getFileUri()), 'Contents of the file are correct.');
-    }
+    $this->assertResponse(200);
+    $this->assertRaw(file_get_contents($file->getFileUri()));
 
     $file->delete();
   }

diff --git a/modules/image/tests/src/Functional/ImageStyleDeleteTest.php b/modules/image/tests/src/Functional/ImageStyleDeleteTest.php
@@ -45,15 +45,14 @@ public function testDelete() {
     /** @var \Drupal\Core\Entity\Display\EntityViewDisplayInterface $view_display */
     $view_display = EntityViewDisplay::load('node.page.default');
     // Checks that the formatter setting is replaced.
-    if ($this->assertNotNull($component = $view_display->getComponent('foo'))) {
-      $this->assertIdentical($component['settings']['image_style'], 'thumbnail');
-    }
+    $this->assertNotNull($component = $view_display->getComponent('foo'));
+    $this->assertSame('thumbnail', $component['settings']['image_style']);
+
     /** @var \Drupal\Core\Entity\Display\EntityFormDisplayInterface $form_display */
     $form_display = EntityFormDisplay::load('node.page.default');
     // Check that the widget setting is replaced.
-    if ($this->assertNotNull($component = $form_display->getComponent('foo'))) {
-      $this->assertIdentical($component['settings']['preview_image_style'], 'thumbnail');
-    }
+    $this->assertNotNull($component = $form_display->getComponent('foo'));
+    $this->assertSame('thumbnail', $component['settings']['preview_image_style']);
 
     $this->drupalGet('admin/config/media/image-styles/manage/thumbnail/delete');
     // Checks that the 'replacement' select element is displayed.

diff --git a/modules/migrate_drupal/src/Tests/StubTestTrait.php b/modules/migrate_drupal/src/Tests/StubTestTrait.php
@@ -19,14 +19,8 @@ trait StubTestTrait {
   protected function performStubTest($entity_type_id) {
     $entity_id = $this->createEntityStub($entity_type_id);
     $this->assertNotEmpty($entity_id, 'Stub successfully created');
-    if ($entity_id) {
-      $violations = $this->validateStub($entity_type_id, $entity_id);
-      if (!$this->assertIdentical(count($violations), 0, 'Stub is a valid entity')) {
-        foreach ($violations as $violation) {
-          $this->fail((string) $violation->getMessage());
-        }
-      }
-    }
+    // When validateStub fails, it will return an array with the violations.
+    $this->assertEmpty($this->validateStub($entity_type_id, $entity_id));
   }
 
   /**

diff --git a/tests/Drupal/KernelTests/Core/Asset/AttachedAssetsTest.php b/tests/Drupal/KernelTests/Core/Asset/AttachedAssetsTest.php
@@ -433,14 +433,12 @@ public function testDynamicLibrary() {
     $library_discovery->clearCachedDefinitions();
     $dynamic_library = $library_discovery->getLibraryByName('common_test', 'dynamic_library');
     $this->assertTrue(is_array($dynamic_library));
-    if ($this->assertTrue(isset($dynamic_library['version']))) {
-      $this->assertSame('1.0', $dynamic_library['version']);
-    }
+    $this->assertArrayHasKey('version', $dynamic_library);
+    $this->assertSame('1.0', $dynamic_library['version']);
     // Make sure the dynamic library definition could be altered.
     // @see common_test_library_info_alter()
-    if ($this->assertTrue(isset($dynamic_library['dependencies']))) {
-      $this->assertSame(['core/jquery'], $dynamic_library['dependencies']);
-    }
+    $this->assertArrayHasKey('dependencies', $dynamic_library);
+    $this->assertSame(['core/jquery'], $dynamic_library['dependencies']);
   }
 
   /**

diff --git a/tests/Drupal/KernelTests/Core/Database/FetchTest.php b/tests/Drupal/KernelTests/Core/Database/FetchTest.php
@@ -24,8 +24,8 @@ public function testQueryFetchDefault() {
     $this->assertInstanceOf(StatementInterface::class, $result);
     foreach ($result as $record) {
       $records[] = $record;
-      $this->assertTrue(is_object($record), 'Record is an object.');
-      $this->assertIdentical($record->name, 'John', '25 year old is John.');
+      $this->assertIsObject($record);
+      $this->assertSame('John', $record->name);
     }
 
     $this->assertIdentical(count($records), 1, 'There is only one record.');
@@ -39,8 +39,8 @@ public function testQueryFetchObject() {
     $result = $this->connection->query('SELECT name FROM {test} WHERE age = :age', [':age' => 25], ['fetch' => \PDO::FETCH_OBJ]);
     foreach ($result as $record) {
       $records[] = $record;
-      $this->assertTrue(is_object($record), 'Record is an object.');
-      $this->assertIdentical($record->name, 'John', '25 year old is John.');
+      $this->assertIsObject($record);
+      $this->assertSame('John', $record->name);
     }
 
     $this->assertIdentical(count($records), 1, 'There is only one record.');
@@ -54,9 +54,9 @@ public function testQueryFetchArray() {
     $result = $this->connection->query('SELECT name FROM {test} WHERE age = :age', [':age' => 25], ['fetch' => \PDO::FETCH_ASSOC]);
     foreach ($result as $record) {
       $records[] = $record;
-      if ($this->assertTrue(is_array($record), 'Record is an array.')) {
-        $this->assertIdentical($record['name'], 'John', 'Record can be accessed associatively.');
-      }
+      $this->assertIsArray($record);
+      $this->assertArrayHasKey('name', $record);
+      $this->assertSame('John', $record['name']);
     }
 
     $this->assertIdentical(count($records), 1, 'There is only one record.');
@@ -72,9 +72,8 @@ public function testQueryFetchClass() {
     $result = $this->connection->query('SELECT name FROM {test} WHERE age = :age', [':age' => 25], ['fetch' => FakeRecord::class]);
     foreach ($result as $record) {
       $records[] = $record;
-      if ($this->assertInstanceOf(FakeRecord::class, $record)) {
-        $this->assertIdentical($record->name, 'John', '25 year old is John.');
-      }
+      $this->assertInstanceOf(FakeRecord::class, $record);
+      $this->assertSame('John', $record->name);
     }
 
     $this->assertIdentical(count($records), 1, 'There is only one record.');
@@ -108,10 +107,9 @@ public function testQueryFetchClasstype() {
     $result = $this->connection->query('SELECT classname, name, job FROM {test_classtype} WHERE age = :age', [':age' => 26], ['fetch' => \PDO::FETCH_CLASS | \PDO::FETCH_CLASSTYPE]);
     foreach ($result as $record) {
       $records[] = $record;
-      if ($this->assertInstanceOf(FakeRecord::class, $record)) {
-        $this->assertSame('Kay', $record->name, 'Kay is found.');
-        $this->assertSame('Web Developer', $record->job, 'A 26 year old Web Developer.');
-      }
+      $this->assertInstanceOf(FakeRecord::class, $record);
+      $this->assertSame('Kay', $record->name);
+      $this->assertSame('Web Developer', $record->job);
       $this->assertFalse(isset($record->classname), 'Classname field not found, as intended.');
     }
 
@@ -126,9 +124,9 @@ public function testQueryFetchNum() {
     $result = $this->connection->query('SELECT name FROM {test} WHERE age = :age', [':age' => 25], ['fetch' => \PDO::FETCH_NUM]);
     foreach ($result as $record) {
       $records[] = $record;
-      if ($this->assertTrue(is_array($record), 'Record is an array.')) {
-        $this->assertIdentical($record[0], 'John', 'Record can be accessed numerically.');
-      }
+      $this->assertIsArray($record);
+      $this->assertArrayHasKey(0, $record);
+      $this->assertSame('John', $record[0]);
     }
 
     $this->assertIdentical(count($records), 1, 'There is only one record');
@@ -142,10 +140,11 @@ public function testQueryFetchBoth() {
     $result = $this->connection->query('SELECT name FROM {test} WHERE age = :age', [':age' => 25], ['fetch' => \PDO::FETCH_BOTH]);
     foreach ($result as $record) {
       $records[] = $record;
-      if ($this->assertTrue(is_array($record), 'Record is an array.')) {
-        $this->assertIdentical($record[0], 'John', 'Record can be accessed numerically.');
-        $this->assertIdentical($record['name'], 'John', 'Record can be accessed associatively.');
-      }
+      $this->assertIsArray($record);
+      $this->assertArrayHasKey(0, $record);
+      $this->assertSame('John', $record[0]);
+      $this->assertArrayHasKey('name', $record);
+      $this->assertSame('John', $record['name']);
     }
 
     $this->assertIdentical(count($records), 1, 'There is only one record.');

diff --git a/tests/Drupal/KernelTests/Core/Entity/EntityCrudHookTest.php b/tests/Drupal/KernelTests/Core/Entity/EntityCrudHookTest.php
@@ -69,9 +69,8 @@ protected function assertHookMessageOrder($messages) {
     foreach ($messages as $message) {
       // Verify that each message is found and record its position.
       $position = array_search($message, $GLOBALS['entity_crud_hook_test']);
-      if ($this->assertTrue($position !== FALSE, $message)) {
-        $positions[] = $position;
-      }
+      $this->assertNotFalse($position, $message);
+      $positions[] = $position;
     }
 
     // Sort the positions and ensure they remain in the same order.