test: make random component execution safer

.github/scripts/random-tests-config.txt

@@ -13,7 +13,7 @@ AutoReview
 Autoloader
 # Cache
 CLI
-# Commands
+Commands
 Config
 Cookie
 # DataCaster

system/Cache/FactoriesCache/FileVarExportHandler.php

@@ -21,11 +21,28 @@ public function save(string $key, mixed $val): void
  {
  $val = var_export($val, true);
 
+ // Two processes may try to create the directory at the same time.
+ // is_dir() confirms it exists, so suppressing the warning is safe.
+ if (! is_dir($this->path) && ! @mkdir($this->path, 0777, true) && ! is_dir($this->path)) {
+ log_message('error', 'FactoriesCache: cannot create cache directory: ' . $this->path);
+
+ return;
+ }
+
  // Write to temp file first to ensure atomicity
  $tmp = $this->path . "/{$key}." . uniqid('', true) . '.tmp';
- file_put_contents($tmp, '<?php return ' . $val . ';', LOCK_EX);
+ if (file_put_contents($tmp, '<?php return ' . $val . ';', LOCK_EX) === false) {
+ log_message('warning', 'FactoriesCache: failed to write temp file for key: ' . $key);
+
+ return;
+ }
+
+ // Another process may have wiped the directory. Clean up on failure.
+ if (! @rename($tmp, $this->path . "/{$key}")) {
+ log_message('warning', 'FactoriesCache: failed to commit cache file for key: ' . $key);
 
- rename($tmp, $this->path . "/{$key}");
+ @unlink($tmp);
+ }
  }
 
  public function delete(string $key): void

system/Commands/Database/ShowTableInfo.php

@@ -241,9 +241,11 @@ private function showAllTables(array $tables)
  */
  private function makeTbodyForShowAllTables(array $tables): array
  {
+ $this->tbody = [];
+
  $this->removeDBPrefix();
 
- foreach ($tables as $id => $tableName) {
+ foreach ($tables as $id => $tableName) {
  $table = $this->db->protectIdentifiers($tableName);
  $db = $this->db->query("SELECT * FROM {$table}");
 

system/HotReloader/DirectoryHasher.php

@@ -73,10 +73,12 @@ public function hashDirectory(string $path): string
 
  foreach ($iterator as $file) {
  if ($file->isFile()) {
- $hashes[] = md5_file($file->getRealPath());
+ $hashes[$file->getRealPath()] = md5_file($file->getRealPath());
  }
  }
 
+ ksort($hashes);
+
  return md5(implode('', $hashes));
  }
 }

system/Log/Handlers/FileHandler.php

@@ -19,7 +19,7 @@
 /**
  * Log error messages to file system
  *
- * @see \CodeIgniter\Log\Handlers\FileHandlerTest
+ * @see FileHandlerTest
  */
 class FileHandler extends BaseHandler
 {
@@ -121,7 +121,8 @@ public function handle($level, $message): bool
  fclose($fp);
 
  if ($newfile) {
- chmod($filepath, $this->filePermissions);
+ // The log entry is already persisted - permission changes are best-effort.
+ @chmod($filepath, $this->filePermissions);
  }
 
  return is_int($result);

tests/_support/Commands/Unsuffixable.php

@@ -76,4 +76,14 @@ public function run(array $params): void
  $this->setEnabledSuffixing(false);
  $this->generateClass($params);
  }
+
+ protected function prepare(string $class): string
+ {
+ return $this->parseTemplate(
+ $class,
+ ['{group}', '{command}'],
+ ['Generators', 'make:foo'],
+ ['type' => 'basic'],
+ );
+ }
 }

tests/system/AutoReview/CreateNewChangelogTest.php

@@ -48,56 +48,71 @@ protected function setUp(): void
  #[DataProvider('provideCreateNewChangelog')]
  public function testCreateNewChangelog(string $mode): void
  {
- $output = exec('git status --porcelain | wc -l');
-
- if ($output !== '0') {
+ $currentVersion = $this->currentVersion;
+ $newVersion = $this->incrementVersion($currentVersion, $mode);
+ $versionWithoutDots = str_replace('.', '', $newVersion);
+ $changelogPath = "./user_guide_src/source/changelogs/v{$newVersion}.rst";
+ $upgradePath = "./user_guide_src/source/installation/upgrade_{$versionWithoutDots}.rst";
+ $trackedPathArgs = implode(' ', array_map(escapeshellarg(...), [
+ './system/CodeIgniter.php',
+ './user_guide_src/source/changelogs/index.rst',
+ './user_guide_src/source/installation/upgrading.rst',
+ ]));
+ $generatedPathArgs = implode(' ', array_map(escapeshellarg(...), [
+ $changelogPath,
+ $upgradePath,
+ ]));
+ $statusCount = trim((string) exec(
+ "git status --porcelain -- {$trackedPathArgs} {$generatedPathArgs} | wc -l",
+ ));
+
+ if ($statusCount !== '0') {
  $this->markTestSkipped('You have uncommitted operations that will be erased by this test.');
  }
 
- $currentVersion = $this->currentVersion;
- $newVersion = $this->incrementVersion($currentVersion, $mode);
-
- exec(
- sprintf('php ./admin/create-new-changelog.php %s %s --dry-run', $currentVersion, $newVersion),
- $output,
- $exitCode,
- );
-
- $this->assertSame(0, $exitCode, "Script exited with code {$exitCode}. Output: " . implode("\n", $output));
-
- $this->assertStringContainsString(
- "public const CI_VERSION = '{$newVersion}-dev';",
- $this->getContents('./system/CodeIgniter.php'),
- );
-
- $this->assertFileExists("./user_guide_src/source/changelogs/v{$newVersion}.rst");
- $this->assertStringContainsString(
- "Version {$newVersion}",
- $this->getContents("./user_guide_src/source/changelogs/v{$newVersion}.rst"),
- );
- $this->assertStringContainsString(
- "**{$newVersion} release of CodeIgniter4**",
- $this->getContents("./user_guide_src/source/changelogs/v{$newVersion}.rst"),
- );
- $this->assertStringContainsString(
- $newVersion,
- $this->getContents('./user_guide_src/source/changelogs/index.rst'),
- );
-
- $versionWithoutDots = str_replace('.', '', $newVersion);
- $this->assertFileExists("./user_guide_src/source/installation/upgrade_{$versionWithoutDots}.rst");
- $this->assertStringContainsString(
- "Upgrading from {$currentVersion} to {$newVersion}",
- $this->getContents("./user_guide_src/source/installation/upgrade_{$versionWithoutDots}.rst"),
- );
- $this->assertStringContainsString(
- "upgrade_{$versionWithoutDots}",
- $this->getContents('./user_guide_src/source/installation/upgrading.rst'),
- );
-
- // cleanup added and modified files
- exec('git restore .');
- exec('git clean -fd');
+ try {
+ $output = [];
+
+ exec(
+ sprintf('php ./admin/create-new-changelog.php %s %s --dry-run', $currentVersion, $newVersion),
+ $output,
+ $exitCode,
+ );
+
+ $this->assertSame(0, $exitCode, "Script exited with code {$exitCode}. Output: " . implode("\n", $output));
+
+ $this->assertStringContainsString(
+ "public const CI_VERSION = '{$newVersion}-dev';",
+ $this->getContents('./system/CodeIgniter.php'),
+ );
+
+ $this->assertFileExists($changelogPath);
+ $this->assertStringContainsString(
+ "Version {$newVersion}",
+ $this->getContents($changelogPath),
+ );
+ $this->assertStringContainsString(
+ "**{$newVersion} release of CodeIgniter4**",
+ $this->getContents($changelogPath),
+ );
+ $this->assertStringContainsString(
+ $newVersion,
+ $this->getContents('./user_guide_src/source/changelogs/index.rst'),
+ );
+
+ $this->assertFileExists($upgradePath);
+ $this->assertStringContainsString(
+ "Upgrading from {$currentVersion} to {$newVersion}",
+ $this->getContents($upgradePath),
+ );
+ $this->assertStringContainsString(
+ "upgrade_{$versionWithoutDots}",
+ $this->getContents('./user_guide_src/source/installation/upgrading.rst'),
+ );
+ } finally {
+ exec("git restore -- {$trackedPathArgs}");
+ exec("git clean -f -- {$generatedPathArgs}");
+ }
  }
 
  /**

tests/system/Commands/Cache/ClearCacheTest.php

@@ -80,6 +80,7 @@ public function testClearCacheFails(): void
  Services::injectMock('cache', $cache);
 
  command('cache:clear');
+ Services::resetSingle('cache');
 
  $this->assertSame(
  "\nError while clearing the cache.\n",

tests/system/Commands/CreateDatabaseTest.php

@@ -35,7 +35,7 @@ final class CreateDatabaseTest extends CIUnitTestCase
 
  protected function setUp(): void
  {
- $this->connection = Database::connect();
+ $this->connection = Database::connect(null, false);
 
  parent::setUp();
 
@@ -54,12 +54,31 @@ private function dropDatabase(): void
  if ($this->connection instanceof SQLite3Connection) {
  $file = WRITEPATH . 'database.db';
 
+ $this->closeDatabaseConnections();
+
  if (is_file($file)) {
  unlink($file);
  }
- } elseif (Database::utils('tests')->databaseExists('database')) {
+
+ return;
+ }
+
+ if (Database::utils('tests')->databaseExists('database')) {
  Database::forge()->dropDatabase('database');
  }
+
+ $this->closeDatabaseConnections();
+ }
+
+ private function closeDatabaseConnections(): void
+ {
+ $this->connection->close();
+
+ foreach (Database::getConnections() as $connection) {
+ $connection->close();
+ }
+
+ $this->setPrivateProperty(Database::class, 'instances', []);
  }
 
  protected function getBuffer(): string

tests/system/Commands/Database/MigrateStatusTest.php

@@ -29,33 +29,19 @@ final class MigrateStatusTest extends CIUnitTestCase
  use StreamFilterTrait;
  use DatabaseTestTrait;
 
- private string $migrationFileFrom = SUPPORTPATH . 'MigrationTestMigrations/Database/Migrations/2018-01-24-102301_Some_migration.php';
- private string $migrationFileTo = APPPATH . 'Database/Migrations/2018-01-24-102301_Some_migration.php';
+ private string $migrationNamespace = 'Tests\\Support\\MigrationTestMigrations';
+ private string $migrationNamespacePath = SUPPORTPATH . 'MigrationTestMigrations/';
 
  protected function setUp(): void
  {
+ $this->resetServices();
+
  parent::setUp();
 
  Database::connect()->table('migrations')->emptyTable();
  Database::forge()->dropTable('foo', true);
 
- if (! is_file($this->migrationFileFrom)) {
- $this->fail(clean_path($this->migrationFileFrom) . ' is not found.');
- }
-
- if (is_file($this->migrationFileTo)) {
- @unlink($this->migrationFileTo);
- }
-
- copy($this->migrationFileFrom, $this->migrationFileTo);
-
- $contents = file_get_contents($this->migrationFileTo);
- $contents = str_replace(
- 'namespace Tests\Support\MigrationTestMigrations\Database\Migrations;',
- 'namespace App\Database\Migrations;',
- $contents,
- );
- file_put_contents($this->migrationFileTo, $contents);
+ service('autoloader')->addNamespace($this->migrationNamespace, $this->migrationNamespacePath);
 
  putenv('NO_COLOR=1');
  CLI::init();
@@ -66,13 +52,12 @@ protected function tearDown(): void
  parent::tearDown();
 
  Database::connect()->table('migrations')->emptyTable();
-
- if (is_file($this->migrationFileTo)) {
- @unlink($this->migrationFileTo);
- }
+ Database::forge()->dropTable('foo', true);
 
  putenv('NO_COLOR');
  CLI::init();
+
+ $this->resetServices();
  }
 
  public function testMigrateAllWithWithTwoNamespaces(): void
@@ -82,41 +67,31 @@ public function testMigrateAllWithWithTwoNamespaces(): void
 
  command('migrate:status');
 
- $result = str_replace(PHP_EOL, "\n", $this->getStreamFilterBuffer());
- $result = preg_replace('/\d{4}-\d\d-\d\d \d\d:\d\d:\d\d/', 'YYYY-MM-DD HH:MM:SS', $result);
- $expected = <<<'EOL'
- +---------------+-------------------+--------------------+-------+---------------------+-------+
- | Namespace | Version | Filename | Group | Migrated On | Batch |
- +---------------+-------------------+--------------------+-------+---------------------+-------+
- | App | 2018-01-24-102301 | Some_migration | tests | YYYY-MM-DD HH:MM:SS | 1 |
- | Tests\Support | 20160428212500 | Create_test_tables | tests | YYYY-MM-DD HH:MM:SS | 1 |
- +---------------+-------------------+--------------------+-------+---------------------+-------+
-
-
- EOL;
- $this->assertSame($expected, $result);
+ $this->assertMigrationStatusHasBothNamespaceMigrations();
  }
 
  public function testMigrateWithWithTwoNamespaces(): void
  {
- command('migrate -n App');
+ command('migrate -n Tests\\\\Support\\\\MigrationTestMigrations');
  command('migrate -n Tests\\\\Support');
  $this->resetStreamFilterBuffer();
 
  command('migrate:status');
 
- $result = str_replace(PHP_EOL, "\n", $this->getStreamFilterBuffer());
- $result = preg_replace('/\d{4}-\d\d-\d\d \d\d:\d\d:\d\d/', 'YYYY-MM-DD HH:MM:SS', $result);
- $expected = <<<'EOL'
- +---------------+-------------------+--------------------+-------+---------------------+-------+
- | Namespace | Version | Filename | Group | Migrated On | Batch |
- +---------------+-------------------+--------------------+-------+---------------------+-------+
- | App | 2018-01-24-102301 | Some_migration | tests | YYYY-MM-DD HH:MM:SS | 1 |
- | Tests\Support | 20160428212500 | Create_test_tables | tests | YYYY-MM-DD HH:MM:SS | 2 |
- +---------------+-------------------+--------------------+-------+---------------------+-------+
-
+ $this->assertMigrationStatusHasBothNamespaceMigrations();
+ }
 
- EOL;
- $this->assertSame($expected, $result);
+ private function assertMigrationStatusHasBothNamespaceMigrations(): void
+ {
+ $result = str_replace(PHP_EOL, "\n", $this->getStreamFilterBuffer());
+ $theadPattern = '/^\|[[:space:]]+Namespace[[:space:]]+\|[[:space:]]+Version[[:space:]]+\|[[:space:]]+Filename[[:space:]]+\|[[:space:]]+Group[[:space:]]+\|[[:space:]]+Migrated On[[:space:]]+\|[[:space:]]+Batch[[:space:]]+\|$/m';
+
+ $this->assertMatchesRegularExpression($theadPattern, $result);
+ $this->assertStringContainsString($this->migrationNamespace, $result);
+ $this->assertStringContainsString('2018-01-24-102301', $result);
+ $this->assertStringContainsString('Some_migration', $result);
+ $this->assertStringContainsString('Tests\Support', $result);
+ $this->assertStringContainsString('20160428212500', $result);
+ $this->assertStringContainsString('Create_test_tables', $result);
  }
 }