Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sidharthap created an issue. See original summary.

gaurav.kapoor’s picture

sidharthap’s picture

Status: Active » Needs review
FileSize
1.44 KB

Initial try.

gaurav.kapoor’s picture

gaurav.kapoor’s picture

JayKandari’s picture

Status: Needs review » Reviewed & tested by the community

#5 Looks good.
Only found 1 instance of "db_find_tables()" call in core/modules/simpletest/simpletest.module:666

The last submitted patch, 2: drupal-replacing-db_find_tables-in-core-2848808.patch, failed testing.

cilefen’s picture

Status: Reviewed & tested by the community » Needs work

I don't know why we would change a usage in the simpletest module but not in the system module. ModuleTestBase isn't specifically for testing the global function. AFAIK it would be nice to have a test somewhere for the global function. So, I don't know what we want here.

(edited)

xjm’s picture

Issue tags: +DrupalCampNJ2017

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

voleger’s picture

Reroll for 8.6.x
Also replaced 2 usages. See interdiff

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Mile23’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Patch no longer applies.

voleger’s picture

Title: Replace all calls to db_find_tables, which is deprecated. » [PP-2] Replace all calls to db_find_tables, which is deprecated.
Status: Needs work » Postponed
Related issues: +#2991337: Document the recommended ways to obtain the database connection object, +#2991542: Introduce a DatabaseLegacyTest class for deprecation testing
voleger’s picture

Title: [PP-2] Replace all calls to db_find_tables, which is deprecated. » [PP-1] Replace all calls to db_find_tables, which is deprecated.1
voleger’s picture

Title: [PP-1] Replace all calls to db_find_tables, which is deprecated.1 » [PP-1] Replace all calls to db_find_tables, which is deprecated.
voleger’s picture

Title: [PP-1] Replace all calls to db_find_tables, which is deprecated. » Replace all calls to db_find_tables, which is deprecated.
mondrake’s picture

Status: Needs review » Reviewed & tested by the community

Looks ready. RTBC

andypost’s picture

+++ b/core/tests/Drupal/KernelTests/Core/Database/DatabaseLegacyTest.php
@@ -66,6 +66,15 @@ public function testDbTableExists() {
+    $this->assertSame(['test' => 'test'] , db_find_tables('test'));

most of time this function used with "%" so please change test to use wildcard

mondrake’s picture

Status: Reviewed & tested by the community » Needs work

Fails on SQLite and Pgsql

andypost’s picture

The fail is unrelated, the fix for failure is #2996436: Follow-up to fix DatabaseLegacyTest::testDbRenameTable

Re-rolled patch to check wildcard ability

mondrake’s picture

Status: Needs review » Needs work
+++ b/core/tests/Drupal/KernelTests/Core/Database/DatabaseLegacyTest.php
@@ -67,6 +67,19 @@ public function testDbTableExists() {
+  public function testDbFindTables() {
+    $expected = [
+      'test_people' => 'test_people',
+      'test_people_copy' => 'test_people_copy',
+    ];
+    $this->assertSame($expected, db_find_tables('test_people%'));
+  }
+

We can just use assertEquals here that is doing a comparison without taking into account the sequence of the keys in the array (see #2925750: EntityDefinitionUpdateTest fails with contrib db driver). That should fix the Pgsql test failure.

Vidushi Mehta’s picture

Added a patch with above mentioned change by #25

Vidushi Mehta’s picture

FileSize
568 bytes

Interdiff

voleger’s picture

Status: Needs review » Reviewed & tested by the community

Looks good

voleger’s picture

Still applicable

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed b083070 and pushed to 8.7.x. Thanks!

diff --git a/core/tests/Drupal/KernelTests/Core/Database/DatabaseLegacyTest.php b/core/tests/Drupal/KernelTests/Core/Database/DatabaseLegacyTest.php
old mode 100755
new mode 100644

Fixed the file mode being changed by the patch.

  • alexpott committed b083070 on 8.7.x
    Issue #2848808 by voleger, gaurav.kapoor, andypost, Vidushi Mehta,...

Status: Fixed » Closed (fixed)

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