From f376cbf92a36c02496f073f6375fac12aac1bc25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 12 May 2025 10:49:53 +0200 Subject: [PATCH 1/3] Fix TestCase::expectErrorLog() with open_basedir php.ini --- src/Framework/TestCase.php | 47 ++++++++++++------- ...pect-error-log-fail-with-open_basedir.phpt | 36 ++++++++++++++ .../expect-error-log-with-open_basedir.phpt | 28 +++++++++++ 3 files changed, 95 insertions(+), 16 deletions(-) create mode 100644 tests/end-to-end/generic/expect-error-log-fail-with-open_basedir.phpt create mode 100644 tests/end-to-end/generic/expect-error-log-with-open_basedir.phpt diff --git a/src/Framework/TestCase.php b/src/Framework/TestCase.php index c48e99a8716..ae4c655be48 100644 --- a/src/Framework/TestCase.php +++ b/src/Framework/TestCase.php @@ -32,6 +32,7 @@ use function is_int; use function is_object; use function is_string; +use function is_writable; use function libxml_clear_errors; use function method_exists; use function ob_end_clean; @@ -1278,30 +1279,43 @@ private function runTest(): mixed { $testArguments = array_merge($this->data, array_values($this->dependencyInput)); - $capture = tmpfile(); - $errorLogPrevious = ini_set('error_log', stream_get_meta_data($capture)['uri']); + $capture = tmpfile(); + + if ($capture !== false) { + $capturePath = stream_get_meta_data($capture)['uri']; + + if (@is_writable($capturePath)) { + $errorLogPrevious = ini_set('error_log', $capturePath); + } else { + $capture = false; + } + } try { /** @phpstan-ignore method.dynamicName */ $testResult = $this->{$this->methodName}(...$testArguments); - $errorLogOutput = stream_get_contents($capture); + if ($capture !== false) { + $errorLogOutput = stream_get_contents($capture); - if ($this->expectErrorLog) { - $this->assertNotEmpty($errorLogOutput, 'Test did not call error_log().'); - } else { - if ($errorLogOutput !== false) { - // strip date from logged error, see https://github.com/php/php-src/blob/c696087e323263e941774ebbf902ac249774ec9f/main/main.c#L905 - print preg_replace('/\[.+\] /', '', $errorLogOutput); + if ($this->expectErrorLog) { + $this->assertNotEmpty($errorLogOutput, 'Test did not call error_log().'); + } else { + if ($errorLogOutput !== false) { + // strip date from logged error, see https://github.com/php/php-src/blob/c696087e323263e941774ebbf902ac249774ec9f/main/main.c#L905 + print preg_replace('/\[.+\] /', '', $errorLogOutput); + } } } } catch (Throwable $exception) { - if (!$this->expectErrorLog) { - $errorLogOutput = stream_get_contents($capture); + if ($capture !== false) { + if (!$this->expectErrorLog) { + $errorLogOutput = stream_get_contents($capture); - if ($errorLogOutput !== false) { - // strip date from logged error, see https://github.com/php/php-src/blob/c696087e323263e941774ebbf902ac249774ec9f/main/main.c#L905 - print preg_replace('/\[.+\] /', '', $errorLogOutput); + if ($errorLogOutput !== false) { + // strip date from logged error, see https://github.com/php/php-src/blob/c696087e323263e941774ebbf902ac249774ec9f/main/main.c#L905 + print preg_replace('/\[.+\] /', '', $errorLogOutput); + } } } @@ -1315,9 +1329,10 @@ private function runTest(): mixed } finally { if ($capture !== false) { fclose($capture); - } - ini_set('error_log', $errorLogPrevious); + /** @phpstan-ignore variable.undefined (https://github.com/phpstan/phpstan/issues/12992) */ + ini_set('error_log', $errorLogPrevious); + } } $this->expectedExceptionWasNotRaised(); diff --git a/tests/end-to-end/generic/expect-error-log-fail-with-open_basedir.phpt b/tests/end-to-end/generic/expect-error-log-fail-with-open_basedir.phpt new file mode 100644 index 00000000000..411494fb794 --- /dev/null +++ b/tests/end-to-end/generic/expect-error-log-fail-with-open_basedir.phpt @@ -0,0 +1,36 @@ +--TEST-- +https://github.com/sebastianbergmann/phpunit/issues/6197 +--FILE-- +run($_SERVER['argv']); +--EXPECTF-- +PHPUnit %s by Sebastian Bergmann and contributors. + +Runtime: %s + +. 1 / 1 (100%) + +Time: %s, Memory: %s + +Expect Error Log Fail (PHPUnit\TestFixture\ExpectNoErrorLog\ExpectErrorLogFail) + ✔ One + +OK (1 test, 1 assertion) diff --git a/tests/end-to-end/generic/expect-error-log-with-open_basedir.phpt b/tests/end-to-end/generic/expect-error-log-with-open_basedir.phpt new file mode 100644 index 00000000000..25eed2968fe --- /dev/null +++ b/tests/end-to-end/generic/expect-error-log-with-open_basedir.phpt @@ -0,0 +1,28 @@ +--TEST-- +https://github.com/sebastianbergmann/phpunit/issues/6197 +--FILE-- +run($_SERVER['argv']); +--EXPECTF-- +PHPUnit %s by Sebastian Bergmann and contributors. + +Runtime: %s + +logged a side effect +. 1 / 1 (100%) + +Time: %s, Memory: %s + +Expect Error Log (PHPUnit\TestFixture\ExpectErrorLog\ExpectErrorLog) + ✔ One + +OK (1 test, 1 assertion) From 686637dcc2a04f4ff5c993c49b0c75ad1f87844b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Sat, 10 May 2025 11:23:45 +0200 Subject: [PATCH 2/3] improve date stripping from error log --- src/Framework/TestCase.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Framework/TestCase.php b/src/Framework/TestCase.php index ae4c655be48..764ee6fce36 100644 --- a/src/Framework/TestCase.php +++ b/src/Framework/TestCase.php @@ -1302,8 +1302,7 @@ private function runTest(): mixed $this->assertNotEmpty($errorLogOutput, 'Test did not call error_log().'); } else { if ($errorLogOutput !== false) { - // strip date from logged error, see https://github.com/php/php-src/blob/c696087e323263e941774ebbf902ac249774ec9f/main/main.c#L905 - print preg_replace('/\[.+\] /', '', $errorLogOutput); + print $this->stripDateFromErrorLog($errorLogOutput); } } } @@ -1313,8 +1312,7 @@ private function runTest(): mixed $errorLogOutput = stream_get_contents($capture); if ($errorLogOutput !== false) { - // strip date from logged error, see https://github.com/php/php-src/blob/c696087e323263e941774ebbf902ac249774ec9f/main/main.c#L905 - print preg_replace('/\[.+\] /', '', $errorLogOutput); + print $this->stripDateFromErrorLog($errorLogOutput); } } } @@ -1340,6 +1338,12 @@ private function runTest(): mixed return $testResult; } + private function stripDateFromErrorLog(string $log): string + { + // https://github.com/php/php-src/blob/c696087e323263e941774ebbf902ac249774ec9f/main/main.c#L905 + return preg_replace('/\[\d+-\w+-\d+ \d+:\d+:\d+ [^\r\n[\]]+?\] /', '', $log); + } + /** * @throws ExpectationFailedException */ From cf803bd2fbbdcc215a31c7dd553a5ad0c62d2aea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 12 May 2025 10:55:24 +0200 Subject: [PATCH 3/3] mark test as incomplete if error_log file cannot be created --- src/Framework/TestCase.php | 2 ++ .../expect-error-log-fail-with-open_basedir.phpt | 14 ++++++++++---- .../expect-error-log-with-open_basedir.phpt | 11 ++++++++--- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/Framework/TestCase.php b/src/Framework/TestCase.php index 764ee6fce36..3ca91e0a373 100644 --- a/src/Framework/TestCase.php +++ b/src/Framework/TestCase.php @@ -1305,6 +1305,8 @@ private function runTest(): mixed print $this->stripDateFromErrorLog($errorLogOutput); } } + } elseif ($this->expectErrorLog) { + $this->markTestIncomplete('Could not create writable error_log file.'); } } catch (Throwable $exception) { if ($capture !== false) { diff --git a/tests/end-to-end/generic/expect-error-log-fail-with-open_basedir.phpt b/tests/end-to-end/generic/expect-error-log-fail-with-open_basedir.phpt index 411494fb794..a511ade932f 100644 --- a/tests/end-to-end/generic/expect-error-log-fail-with-open_basedir.phpt +++ b/tests/end-to-end/generic/expect-error-log-fail-with-open_basedir.phpt @@ -13,7 +13,8 @@ $_SERVER['argv'][] = __DIR__ . '/_files/ExpectErrorLogFailTest.php'; * - https://github.com/php/php-src/issues/17817 * - https://github.com/php/php-src/issues/18530 * - * Until then, ignore TestCase::expectErrorLog() if open_basedir php.ini is in effect. + * Until then, mark the test result as incomplete when TestCase::expectErrorLog() was called and an error_log file + * could not be created (because of open_basedir php.ini in effect, readonly filesystem...). */ ini_set('open_basedir', (ini_get('open_basedir') ? ini_get('open_basedir') . PATH_SEPARATOR : '') . dirname(__DIR__, 3)); @@ -26,11 +27,16 @@ PHPUnit %s by Sebastian Bergmann and contributors. Runtime: %s -. 1 / 1 (100%) +I 1 / 1 (100%) Time: %s, Memory: %s Expect Error Log Fail (PHPUnit\TestFixture\ExpectNoErrorLog\ExpectErrorLogFail) - ✔ One + ∅ One + │ + │ Could not create writable error_log file. -OK (1 test, 1 assertion) + │ + +OK, but there were issues! +Tests: 1, Assertions: 1, Incomplete: 1. diff --git a/tests/end-to-end/generic/expect-error-log-with-open_basedir.phpt b/tests/end-to-end/generic/expect-error-log-with-open_basedir.phpt index 25eed2968fe..d0218ba6558 100644 --- a/tests/end-to-end/generic/expect-error-log-with-open_basedir.phpt +++ b/tests/end-to-end/generic/expect-error-log-with-open_basedir.phpt @@ -18,11 +18,16 @@ PHPUnit %s by Sebastian Bergmann and contributors. Runtime: %s logged a side effect -. 1 / 1 (100%) +I 1 / 1 (100%) Time: %s, Memory: %s Expect Error Log (PHPUnit\TestFixture\ExpectErrorLog\ExpectErrorLog) - ✔ One + ∅ One + │ + │ Could not create writable error_log file. -OK (1 test, 1 assertion) + │ + +OK, but there were issues! +Tests: 1, Assertions: 1, Incomplete: 1.