From 775d8293c5e611ada321a4639c5227f42152fc30 Mon Sep 17 00:00:00 2001 From: Benjamin Cremer Date: Mon, 29 Jan 2024 07:40:02 +0100 Subject: [PATCH 1/3] Do not test sys_tmp_dir if cache dir is provied. Fix #65. Signed-off-by: Benjamin Cremer --- .laminas-ci.json | 10 ++++++++ src/FilesystemOptions.php | 6 +++-- ...FilesystemOptionsNonExistentTmpdirTest.php | 24 +++++++++++++++++++ test/unit/FilesystemOptionsTest.php | 6 +++++ 4 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 test/unit/FilesystemOptionsNonExistentTmpdirTest.php diff --git a/.laminas-ci.json b/.laminas-ci.json index 2c63c08..61106fc 100644 --- a/.laminas-ci.json +++ b/.laminas-ci.json @@ -1,2 +1,12 @@ { + "additional_checks": [ + { + "name": "Unit tests with non existent TMPDIR", + "job": { + "php": "*", + "dependencies": "locked", + "command": "TMPDIR=nonexistent vendor/bin/phpunit test/unit/FilesystemOptionsNonExistentTmpdirTest.php" + } + } + ] } diff --git a/src/FilesystemOptions.php b/src/FilesystemOptions.php index e849b8d..8b78304 100644 --- a/src/FilesystemOptions.php +++ b/src/FilesystemOptions.php @@ -112,9 +112,11 @@ public function __construct($options = null) $this->dirPermission = false; } - $this->setCacheDir(null); - parent::__construct($options); + + if ($this->cacheDir === null) { + $this->setCacheDir(null); + } } /** diff --git a/test/unit/FilesystemOptionsNonExistentTmpdirTest.php b/test/unit/FilesystemOptionsNonExistentTmpdirTest.php new file mode 100644 index 0000000..2e8db22 --- /dev/null +++ b/test/unit/FilesystemOptionsNonExistentTmpdirTest.php @@ -0,0 +1,24 @@ + '/./tmp']); + self::assertSame('/tmp', $option->getCacheDir()); + } +} diff --git a/test/unit/FilesystemOptionsTest.php b/test/unit/FilesystemOptionsTest.php index 980bad2..45f6f81 100644 --- a/test/unit/FilesystemOptionsTest.php +++ b/test/unit/FilesystemOptionsTest.php @@ -55,6 +55,12 @@ protected function createAdapterOptions(): AdapterOptions return new FilesystemOptions(); } + public function testSetCacheDirToSystemsTempDirWhenNoCacheDirIsProvided(): void + { + $options = new FilesystemOptions(); + self::assertEquals(realpath(sys_get_temp_dir()), $options->getCacheDir()); + } + public function testSetCacheDirToSystemsTempDirWithNull(): void { $this->options->setCacheDir(null); From 3fd0ccc609cb536e5426e6278303f486ccbbe363 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 10 Jul 2024 14:22:59 +0200 Subject: [PATCH 2/3] qa: add macos temp directory support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- .../FilesystemOptionsNonExistentTmpdirTest.php | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/test/unit/FilesystemOptionsNonExistentTmpdirTest.php b/test/unit/FilesystemOptionsNonExistentTmpdirTest.php index 2e8db22..41cc589 100644 --- a/test/unit/FilesystemOptionsNonExistentTmpdirTest.php +++ b/test/unit/FilesystemOptionsNonExistentTmpdirTest.php @@ -10,15 +10,30 @@ use function is_writable; use function sys_get_temp_dir; +use const PHP_OS_FAMILY; + final class FilesystemOptionsNonExistentTmpdirTest extends TestCase { + private const MACOS_FAMILY = 'Darwin'; + public function testWillNotWriteToSystemTempWhenCacheDirIsProvided(): void { if (is_writable(sys_get_temp_dir())) { self::markTestSkipped('Test has to be executed with a non existent TMPDIR'); } + /** + * Due to the usage of `realpath` {@see FilesystemOptions::normalizeCacheDirectory()}, the directory is changed + * depending on the host system. As there might be developers using MacOS brew PHP to execute these tests, we + * allow MacOS `/private/tmp` as well. + */ + $expectedTemporaryDirectory = match (PHP_OS_FAMILY) { + default => '/tmp', + self::MACOS_FAMILY => '/private/tmp', + }; + $option = new FilesystemOptions(['cacheDir' => '/./tmp']); - self::assertSame('/tmp', $option->getCacheDir()); + + self::assertSame($expectedTemporaryDirectory, $option->getCacheDir()); } } From eee255bf047e22f25ab625b91750cdc284eacfe8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 10 Jul 2024 14:46:28 +0200 Subject: [PATCH 3/3] qa: move test into dedicated script which is executed from unit test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reduces the number of Docker containers spun up during CI and results in fewer total checks. Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- .laminas-ci.json | 10 ----- ...iate_filesystem_options_with_cache_dir.php | 9 +++++ ...FilesystemOptionsNonExistentTmpdirTest.php | 39 ------------------- test/unit/FilesystemOptionsTest.php | 28 +++++++++++++ 4 files changed, 37 insertions(+), 49 deletions(-) create mode 100644 test/unit/Filesystem/TestAsset/instantiate_filesystem_options_with_cache_dir.php delete mode 100644 test/unit/FilesystemOptionsNonExistentTmpdirTest.php diff --git a/.laminas-ci.json b/.laminas-ci.json index 61106fc..2c63c08 100644 --- a/.laminas-ci.json +++ b/.laminas-ci.json @@ -1,12 +1,2 @@ { - "additional_checks": [ - { - "name": "Unit tests with non existent TMPDIR", - "job": { - "php": "*", - "dependencies": "locked", - "command": "TMPDIR=nonexistent vendor/bin/phpunit test/unit/FilesystemOptionsNonExistentTmpdirTest.php" - } - } - ] } diff --git a/test/unit/Filesystem/TestAsset/instantiate_filesystem_options_with_cache_dir.php b/test/unit/Filesystem/TestAsset/instantiate_filesystem_options_with_cache_dir.php new file mode 100644 index 0000000..0d14311 --- /dev/null +++ b/test/unit/Filesystem/TestAsset/instantiate_filesystem_options_with_cache_dir.php @@ -0,0 +1,9 @@ + '/./tmp']); +echo $option->getCacheDir(); diff --git a/test/unit/FilesystemOptionsNonExistentTmpdirTest.php b/test/unit/FilesystemOptionsNonExistentTmpdirTest.php deleted file mode 100644 index 41cc589..0000000 --- a/test/unit/FilesystemOptionsNonExistentTmpdirTest.php +++ /dev/null @@ -1,39 +0,0 @@ - '/tmp', - self::MACOS_FAMILY => '/private/tmp', - }; - - $option = new FilesystemOptions(['cacheDir' => '/./tmp']); - - self::assertSame($expectedTemporaryDirectory, $option->getCacheDir()); - } -} diff --git a/test/unit/FilesystemOptionsTest.php b/test/unit/FilesystemOptionsTest.php index 45f6f81..5dec375 100644 --- a/test/unit/FilesystemOptionsTest.php +++ b/test/unit/FilesystemOptionsTest.php @@ -29,12 +29,15 @@ use const DIRECTORY_SEPARATOR; use const PHP_EOL; use const PHP_OS; +use const PHP_OS_FAMILY; /** * @template-extends AbstractAdapterOptionsTest */ final class FilesystemOptionsTest extends AbstractAdapterOptionsTest { + private const MACOS_FAMILY = 'Darwin'; + /** @var string */ protected $keyPattern = FilesystemOptions::KEY_PATTERN; @@ -236,4 +239,29 @@ public function testTagSuffixIsMutable(): void $this->options->setTagSuffix('.cache'); self::assertSame('.cache', $this->options->getTagSuffix()); } + + /** + * The test asset script does provide a temporary directory and thus, the nonexistent system temp dir should + * not be used at all. + */ + public function testFilesystemOptionsInstantiationWithNonExistentSystemTemporaryDirectory(): void + { + /** + * Due to the usage of `realpath` {@see FilesystemOptions::normalizeCacheDirectory()}, the directory is changed + * depending on the host system. As there might be developers using MacOS brew PHP to execute these tests, we + * allow MacOS `/private/tmp` as well. + */ + $expectedTemporaryDirectory = match (PHP_OS_FAMILY) { + default => '/tmp', + self::MACOS_FAMILY => '/private/tmp', + }; + + $cacheDirectoryFromOptions = exec( + 'TMPDIR=nonexistent php test/unit/Filesystem/TestAsset/instantiate_filesystem_options_with_cache_dir.php', + $output, + $exitCode, + ); + self::assertSame(0, $exitCode); + self::assertSame($expectedTemporaryDirectory, $cacheDirectoryFromOptions); + } }