Improve testing for MetadataTest

system/Database/BaseConnection.php

@@ -1334,20 +1334,21 @@ protected function getDriverFunctionPrefix(): string
     /**
      * Returns an array of table names
      *
-     * @return array|bool
+     * @return array|false
      *
      * @throws DatabaseException
      */
     public function listTables(bool $constrainByPrefix = false)
     {
-        // Is there a cached result?
         if (isset($this->dataCache['table_names']) && $this->dataCache['table_names']) {
-            return $constrainByPrefix ?
-                preg_grep("/^{$this->DBPrefix}/", $this->dataCache['table_names'])
+            return $constrainByPrefix
+                ? preg_grep("/^{$this->DBPrefix}/", $this->dataCache['table_names'])
                 : $this->dataCache['table_names'];
         }
 
-        if (false === ($sql = $this->_listTables($constrainByPrefix))) {
+        $sql = $this->_listTables($constrainByPrefix);
+
+        if ($sql === false) {
             if ($this->DBDebug) {
                 throw new DatabaseException('This feature is not available for the database you are using.');
             }
@@ -1360,24 +1361,9 @@ public function listTables(bool $constrainByPrefix = false)
         $query = $this->query($sql);
 
         foreach ($query->getResultArray() as $row) {
-            // Do we know from which column to get the table name?
-            if (! isset($key)) {
-                if (isset($row['table_name'])) {
-                    $key = 'table_name';
-                } elseif (isset($row['TABLE_NAME'])) {
-                    $key = 'TABLE_NAME';
-                } else {
-                    /* We have no other choice but to just get the first element's key.
-                     * Due to array_shift() accepting its argument by reference, if
-                     * E_STRICT is on, this would trigger a warning. So we'll have to
-                     * assign it first.
-                     */
-                    $key = array_keys($row);
-                    $key = array_shift($key);
-                }
-            }
+            $table = $row['table_name'] ?? $row['TABLE_NAME'] ?? $row[array_key_first($row)];
 
-            $this->dataCache['table_names'][] = $row[$key];
+            $this->dataCache['table_names'][] = $table;
         }
 
         return $this->dataCache['table_names'];

tests/system/Database/Live/MetadataTest.php

@@ -11,7 +11,6 @@
 
 namespace CodeIgniter\Database\Live;
 
-use CodeIgniter\Database\Forge;
 use CodeIgniter\Test\CIUnitTestCase;
 use CodeIgniter\Test\DatabaseTestTrait;
 use Config\Database;
@@ -26,22 +25,22 @@ final class MetadataTest extends CIUnitTestCase
 {
     use DatabaseTestTrait;
 
-    private ?Forge $forge = null;
-    protected $refresh    = true;
-    protected $seed       = CITestSeeder::class;
-
     /**
-     * Array of expected tables.
+     * The seed file used for all tests within this test case.
+     *
+     * @var string
      */
-    private array $expectedTables;
+    protected $seed = CITestSeeder::class;
+
+    private array $expectedTables = [];
 
     protected function setUp(): void
     {
         parent::setUp();
 
-        // Prepare the array of expected tables once
-        $prefix               = $this->db->getPrefix();
-        $this->expectedTables = [
+        $prefix = $this->db->getPrefix();
+
+        $tables = [
             $prefix . 'migrations',
             $prefix . 'user',
             $prefix . 'job',
@@ -55,55 +54,90 @@ protected function setUp(): void
         ];
 
         if (in_array($this->db->DBDriver, ['MySQLi', 'Postgre'], true)) {
-            $this->expectedTables[] = $prefix . 'ci_sessions';
+            $tables[] = $prefix . 'ci_sessions';
         }
+
+        sort($tables);
+        $this->expectedTables = $tables;
     }
 
-    public function testListTables()
+    private function createExtraneousTable(): void
     {
-        $result = $this->db->listTables(true);
+        $oldPrefix = $this->db->getPrefix();
+        $this->db->setPrefix('tmp_');
+
+        Database::forge($this->DBGroup)
+            ->addField([
+                'name'       => ['type' => 'varchar', 'constraint' => 31],
+                'created_at' => ['type' => 'datetime', 'null' => true],
+            ])
+            ->createTable('widgets');
 
-        $this->assertSame($this->expectedTables, array_values($result));
+        $this->db->setPrefix($oldPrefix);
     }
 
-    public function testListTablesConstrainPrefix()
+    private function dropExtraneousTable(): void
     {
-        $result = $this->db->listTables(true);
+        $oldPrefix = $this->db->getPrefix();
+        $this->db->setPrefix('tmp_');
+
+        Database::forge($this->DBGroup)->dropTable('widgets');
 
-        $this->assertSame($this->expectedTables, array_values($result));
+        $this->db->setPrefix($oldPrefix);
     }
 
-    public function testConstrainPrefixIgnoresOtherTables()
+    public function testListTablesUnconstrainedByPrefixReturnsAllTables()
     {
-        $this->forge = Database::forge($this->DBGroup);
+        try {
+            $this->createExtraneousTable();
 
-        // Stash the prefix and change it
-        $DBPrefix = $this->db->getPrefix();
-        $this->db->setPrefix('tmp_');
+            $tables = $this->db->listTables();
+            $this->assertIsArray($tables);
+            $this->assertNotSame([], $tables);
 
-        // Create a table with the new prefix
-        $fields = [
-            'name' => [
-                'type'       => 'varchar',
-                'constraint' => 31,
-            ],
-            'created_at' => [
-                'type' => 'datetime',
-                'null' => true,
-            ],
-        ];
-        $this->forge->addField($fields);
-        $this->forge->createTable('widgets');
+            $expectedTables   = $this->expectedTables;
+            $expectedTables[] = 'tmp_widgets';
 
-        // Restore the prefix and get the tables with the original prefix
-        $this->db->setPrefix($DBPrefix);
-        $result = $this->db->listTables(true);
+            sort($tables);
+            $this->assertSame($expectedTables, array_values($tables));
+        } finally {
+            $this->dropExtraneousTable();
+        }
+    }
 
-        $this->assertSame($this->expectedTables, array_values($result));
+    public function testListTablesConstrainedByPrefixReturnsOnlyTablesWithMatchingPrefix()
+    {
+        try {
+            $this->createExtraneousTable();
 
-        // Clean up temporary table
-        $this->db->setPrefix('tmp_');
-        $this->forge->dropTable('widgets');
-        $this->db->setPrefix($DBPrefix);
+            $tables = $this->db->listTables(true);
+            $this->assertIsArray($tables);
+            $this->assertNotSame([], $tables);
+
+            sort($tables);
+            $this->assertSame($this->expectedTables, array_values($tables));
+        } finally {
+            $this->dropExtraneousTable();
+        }
+    }
+
+    public function testListTablesConstrainedByExtraneousPrefixReturnsOnlyTheExtraneousTable()
+    {
+        try {
+            $this->createExtraneousTable();
+
+            $oldPrefix = $this->db->getPrefix();
+            $this->db->setPrefix('tmp_');
+
+            $tables = $this->db->listTables(true);
+            $this->assertIsArray($tables);
+            $this->assertNotSame([], $tables);
+
+            sort($tables);
+            $this->assertSame(['tmp_widgets'], array_values($tables));
+        } finally {
+            $this->db->setPrefix($oldPrefix);
+            $this->dropExtraneousTable();
+        }
     }
 }