Skip to content

Commit 811476f

Browse files
memleakdmemleakd
committed
test: make random component execution safer
1 parent 661af8c commit 811476f

19 files changed

Lines changed: 319 additions & 204 deletions

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ AutoReview
1313
Autoloader
1414
# Cache
1515
CLI
16-
# Commands
16+
Commands
1717
Config
1818
Cookie
1919
# DataCaster

‎system/Cache/FactoriesCache/FileVarExportHandler.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,19 @@ public function save(string $key, mixed $val): void
2121
{
2222
$val = var_export($val, true);
2323

24+
if (! is_dir($this->path) && ! @mkdir($this->path, 0777, true) && ! is_dir($this->path)) {
25+
return;
26+
}
27+
2428
// Write to temp file first to ensure atomicity
2529
$tmp = $this->path . "/{$key}." . uniqid('', true) . '.tmp';
26-
file_put_contents($tmp, '<?php return ' . $val . ';', LOCK_EX);
30+
if (file_put_contents($tmp, '<?php return ' . $val . ';', LOCK_EX) === false) {
31+
return;
32+
}
2733

28-
rename($tmp, $this->path . "/{$key}");
34+
if (! @rename($tmp, $this->path . "/{$key}")) {
35+
@unlink($tmp);
36+
}
2937
}
3038

3139
public function delete(string $key): void

‎system/Commands/Database/ShowTableInfo.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ private function makeTbodyForShowAllTables(array $tables): array
243243
{
244244
$this->removeDBPrefix();
245245

246-
foreach ($tables as $id => $tableName) {
246+
foreach (array_values($tables) as $id => $tableName) {
247247
$table = $this->db->protectIdentifiers($tableName);
248248
$db = $this->db->query("SELECT * FROM {$table}");
249249

‎system/HotReloader/DirectoryHasher.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,12 @@ public function hashDirectory(string $path): string
7373

7474
foreach ($iterator as $file) {
7575
if ($file->isFile()) {
76-
$hashes[] = md5_file($file->getRealPath());
76+
$hashes[$file->getRealPath()] = md5_file($file->getRealPath());
7777
}
7878
}
7979

80+
ksort($hashes);
81+
8082
return md5(implode('', $hashes));
8183
}
8284
}

‎system/Log/Handlers/FileHandler.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ public function handle($level, $message): bool
121121
fclose($fp);
122122

123123
if ($newfile) {
124-
chmod($filepath, $this->filePermissions);
124+
@chmod($filepath, $this->filePermissions);
125125
}
126126

127127
return is_int($result);

‎tests/_support/Commands/Unsuffixable.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,4 +76,14 @@ public function run(array $params): void
7676
$this->setEnabledSuffixing(false);
7777
$this->generateClass($params);
7878
}
79+
80+
protected function prepare(string $class): string
81+
{
82+
return $this->parseTemplate(
83+
$class,
84+
['{group}', '{command}'],
85+
['Generators', 'make:foo'],
86+
['type' => 'basic'],
87+
);
88+
}
7989
}

‎tests/system/AutoReview/CreateNewChangelogTest.php‎

Lines changed: 62 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -48,56 +48,71 @@ protected function setUp(): void
4848
#[DataProvider('provideCreateNewChangelog')]
4949
public function testCreateNewChangelog(string $mode): void
5050
{
51-
$output = exec('git status --porcelain | wc -l');
52-
53-
if ($output !== '0') {
51+
$currentVersion = $this->currentVersion;
52+
$newVersion = $this->incrementVersion($currentVersion, $mode);
53+
$versionWithoutDots = str_replace('.', '', $newVersion);
54+
$changelogPath = "./user_guide_src/source/changelogs/v{$newVersion}.rst";
55+
$upgradePath = "./user_guide_src/source/installation/upgrade_{$versionWithoutDots}.rst";
56+
$trackedPathArgs = implode(' ', array_map(escapeshellarg(...), [
57+
'./system/CodeIgniter.php',
58+
'./user_guide_src/source/changelogs/index.rst',
59+
'./user_guide_src/source/installation/upgrading.rst',
60+
]));
61+
$generatedPathArgs = implode(' ', array_map(escapeshellarg(...), [
62+
$changelogPath,
63+
$upgradePath,
64+
]));
65+
$statusCount = trim((string) exec(
66+
"git status --porcelain -- {$trackedPathArgs} {$generatedPathArgs} | wc -l",
67+
));
68+
69+
if ($statusCount !== '0') {
5470
$this->markTestSkipped('You have uncommitted operations that will be erased by this test.');
5571
}
5672

57-
$currentVersion = $this->currentVersion;
58-
$newVersion = $this->incrementVersion($currentVersion, $mode);
59-
60-
exec(
61-
sprintf('php ./admin/create-new-changelog.php %s %s --dry-run', $currentVersion, $newVersion),
62-
$output,
63-
$exitCode,
64-
);
65-
66-
$this->assertSame(0, $exitCode, "Script exited with code {$exitCode}. Output: " . implode("\n", $output));
67-
68-
$this->assertStringContainsString(
69-
"public const CI_VERSION = '{$newVersion}-dev';",
70-
$this->getContents('./system/CodeIgniter.php'),
71-
);
72-
73-
$this->assertFileExists("./user_guide_src/source/changelogs/v{$newVersion}.rst");
74-
$this->assertStringContainsString(
75-
"Version {$newVersion}",
76-
$this->getContents("./user_guide_src/source/changelogs/v{$newVersion}.rst"),
77-
);
78-
$this->assertStringContainsString(
79-
"**{$newVersion} release of CodeIgniter4**",
80-
$this->getContents("./user_guide_src/source/changelogs/v{$newVersion}.rst"),
81-
);
82-
$this->assertStringContainsString(
83-
$newVersion,
84-
$this->getContents('./user_guide_src/source/changelogs/index.rst'),
85-
);
86-
87-
$versionWithoutDots = str_replace('.', '', $newVersion);
88-
$this->assertFileExists("./user_guide_src/source/installation/upgrade_{$versionWithoutDots}.rst");
89-
$this->assertStringContainsString(
90-
"Upgrading from {$currentVersion} to {$newVersion}",
91-
$this->getContents("./user_guide_src/source/installation/upgrade_{$versionWithoutDots}.rst"),
92-
);
93-
$this->assertStringContainsString(
94-
"upgrade_{$versionWithoutDots}",
95-
$this->getContents('./user_guide_src/source/installation/upgrading.rst'),
96-
);
97-
98-
// cleanup added and modified files
99-
exec('git restore .');
100-
exec('git clean -fd');
73+
try {
74+
$output = [];
75+
76+
exec(
77+
sprintf('php ./admin/create-new-changelog.php %s %s --dry-run', $currentVersion, $newVersion),
78+
$output,
79+
$exitCode,
80+
);
81+
82+
$this->assertSame(0, $exitCode, "Script exited with code {$exitCode}. Output: " . implode("\n", $output));
83+
84+
$this->assertStringContainsString(
85+
"public const CI_VERSION = '{$newVersion}-dev';",
86+
$this->getContents('./system/CodeIgniter.php'),
87+
);
88+
89+
$this->assertFileExists($changelogPath);
90+
$this->assertStringContainsString(
91+
"Version {$newVersion}",
92+
$this->getContents($changelogPath),
93+
);
94+
$this->assertStringContainsString(
95+
"**{$newVersion} release of CodeIgniter4**",
96+
$this->getContents($changelogPath),
97+
);
98+
$this->assertStringContainsString(
99+
$newVersion,
100+
$this->getContents('./user_guide_src/source/changelogs/index.rst'),
101+
);
102+
103+
$this->assertFileExists($upgradePath);
104+
$this->assertStringContainsString(
105+
"Upgrading from {$currentVersion} to {$newVersion}",
106+
$this->getContents($upgradePath),
107+
);
108+
$this->assertStringContainsString(
109+
"upgrade_{$versionWithoutDots}",
110+
$this->getContents('./user_guide_src/source/installation/upgrading.rst'),
111+
);
112+
} finally {
113+
exec("git restore -- {$trackedPathArgs}");
114+
exec("git clean -f -- {$generatedPathArgs}");
115+
}
101116
}
102117

103118
/**

‎tests/system/Commands/Cache/ClearCacheTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ public function testClearCacheFails(): void
8080
Services::injectMock('cache', $cache);
8181

8282
command('cache:clear');
83+
Services::resetSingle('cache');
8384

8485
$this->assertSame(
8586
"\nError while clearing the cache.\n",

‎tests/system/Commands/CreateDatabaseTest.php‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ final class CreateDatabaseTest extends CIUnitTestCase
3535

3636
protected function setUp(): void
3737
{
38-
$this->connection = Database::connect();
38+
$this->connection = Database::connect(null, false);
3939

4040
parent::setUp();
4141

@@ -54,12 +54,31 @@ private function dropDatabase(): void
5454
if ($this->connection instanceof SQLite3Connection) {
5555
$file = WRITEPATH . 'database.db';
5656

57+
$this->closeDatabaseConnections();
58+
5759
if (is_file($file)) {
5860
unlink($file);
5961
}
60-
} elseif (Database::utils('tests')->databaseExists('database')) {
62+
63+
return;
64+
}
65+
66+
if (Database::utils('tests')->databaseExists('database')) {
6167
Database::forge()->dropDatabase('database');
6268
}
69+
70+
$this->closeDatabaseConnections();
71+
}
72+
73+
private function closeDatabaseConnections(): void
74+
{
75+
$this->connection->close();
76+
77+
foreach (Database::getConnections() as $connection) {
78+
$connection->close();
79+
}
80+
81+
$this->setPrivateProperty(Database::class, 'instances', []);
6382
}
6483

6584
protected function getBuffer(): string

‎tests/system/Commands/Database/MigrateStatusTest.php‎

Lines changed: 22 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -29,33 +29,19 @@ final class MigrateStatusTest extends CIUnitTestCase
2929
use StreamFilterTrait;
3030
use DatabaseTestTrait;
3131

32-
private string $migrationFileFrom = SUPPORTPATH . 'MigrationTestMigrations/Database/Migrations/2018-01-24-102301_Some_migration.php';
33-
private string $migrationFileTo = APPPATH . 'Database/Migrations/2018-01-24-102301_Some_migration.php';
32+
private string $migrationNamespace = 'Tests\\Support\\MigrationTestMigrations';
33+
private string $migrationNamespacePath = SUPPORTPATH . 'MigrationTestMigrations/';
3434

3535
protected function setUp(): void
3636
{
37+
$this->resetServices();
38+
3739
parent::setUp();
3840

3941
Database::connect()->table('migrations')->emptyTable();
4042
Database::forge()->dropTable('foo', true);
4143

42-
if (! is_file($this->migrationFileFrom)) {
43-
$this->fail(clean_path($this->migrationFileFrom) . ' is not found.');
44-
}
45-
46-
if (is_file($this->migrationFileTo)) {
47-
@unlink($this->migrationFileTo);
48-
}
49-
50-
copy($this->migrationFileFrom, $this->migrationFileTo);
51-
52-
$contents = file_get_contents($this->migrationFileTo);
53-
$contents = str_replace(
54-
'namespace Tests\Support\MigrationTestMigrations\Database\Migrations;',
55-
'namespace App\Database\Migrations;',
56-
$contents,
57-
);
58-
file_put_contents($this->migrationFileTo, $contents);
44+
service('autoloader')->addNamespace($this->migrationNamespace, $this->migrationNamespacePath);
5945

6046
putenv('NO_COLOR=1');
6147
CLI::init();
@@ -66,13 +52,12 @@ protected function tearDown(): void
6652
parent::tearDown();
6753

6854
Database::connect()->table('migrations')->emptyTable();
69-
70-
if (is_file($this->migrationFileTo)) {
71-
@unlink($this->migrationFileTo);
72-
}
55+
Database::forge()->dropTable('foo', true);
7356

7457
putenv('NO_COLOR');
7558
CLI::init();
59+
60+
$this->resetServices();
7661
}
7762

7863
public function testMigrateAllWithWithTwoNamespaces(): void
@@ -82,41 +67,29 @@ public function testMigrateAllWithWithTwoNamespaces(): void
8267

8368
command('migrate:status');
8469

85-
$result = str_replace(PHP_EOL, "\n", $this->getStreamFilterBuffer());
86-
$result = preg_replace('/\d{4}-\d\d-\d\d \d\d:\d\d:\d\d/', 'YYYY-MM-DD HH:MM:SS', $result);
87-
$expected = <<<'EOL'
88-
+---------------+-------------------+--------------------+-------+---------------------+-------+
89-
| Namespace | Version | Filename | Group | Migrated On | Batch |
90-
+---------------+-------------------+--------------------+-------+---------------------+-------+
91-
| App | 2018-01-24-102301 | Some_migration | tests | YYYY-MM-DD HH:MM:SS | 1 |
92-
| Tests\Support | 20160428212500 | Create_test_tables | tests | YYYY-MM-DD HH:MM:SS | 1 |
93-
+---------------+-------------------+--------------------+-------+---------------------+-------+
94-
95-
96-
EOL;
97-
$this->assertSame($expected, $result);
70+
$this->assertMigrationStatusHasBothNamespaceMigrations();
9871
}
9972

10073
public function testMigrateWithWithTwoNamespaces(): void
10174
{
102-
command('migrate -n App');
75+
command('migrate -n Tests\\\\Support\\\\MigrationTestMigrations');
10376
command('migrate -n Tests\\\\Support');
10477
$this->resetStreamFilterBuffer();
10578

10679
command('migrate:status');
10780

108-
$result = str_replace(PHP_EOL, "\n", $this->getStreamFilterBuffer());
109-
$result = preg_replace('/\d{4}-\d\d-\d\d \d\d:\d\d:\d\d/', 'YYYY-MM-DD HH:MM:SS', $result);
110-
$expected = <<<'EOL'
111-
+---------------+-------------------+--------------------+-------+---------------------+-------+
112-
| Namespace | Version | Filename | Group | Migrated On | Batch |
113-
+---------------+-------------------+--------------------+-------+---------------------+-------+
114-
| App | 2018-01-24-102301 | Some_migration | tests | YYYY-MM-DD HH:MM:SS | 1 |
115-
| Tests\Support | 20160428212500 | Create_test_tables | tests | YYYY-MM-DD HH:MM:SS | 2 |
116-
+---------------+-------------------+--------------------+-------+---------------------+-------+
117-
81+
$this->assertMigrationStatusHasBothNamespaceMigrations();
82+
}
11883

119-
EOL;
120-
$this->assertSame($expected, $result);
84+
private function assertMigrationStatusHasBothNamespaceMigrations(): void
85+
{
86+
$result = str_replace(PHP_EOL, "\n", $this->getStreamFilterBuffer());
87+
88+
$this->assertStringContainsString($this->migrationNamespace, $result);
89+
$this->assertStringContainsString('2018-01-24-102301', $result);
90+
$this->assertStringContainsString('Some_migration', $result);
91+
$this->assertStringContainsString('Tests\Support', $result);
92+
$this->assertStringContainsString('20160428212500', $result);
93+
$this->assertStringContainsString('Create_test_tables', $result);
12194
}
12295
}

0 commit comments

Comments
 (0)