Fix TestCase::expectErrorLog() with open_basedir php.ini

[mvorisek]

fix #6197

[staabm]

ignoring expectations sounds like a bad idea.. maybe we should turn such a test "risky" and/or can introduce a warning telling the author of the test, that not everything worked according to plan.

siltent ignore sounds like a bad idea.

I guess @sebastianbergmann has an opinion on that one

[mvorisek]

Good point.

"incomplete" should be the right status. If error_log file could not be created, test would still complete but will be marked as incomplete.

[mvorisek]

Should the "throw incomplete exception" be moved then into TestCase::runBare to always run https://github.com/sebastianbergmann/phpunit/blob/12.1.5/src/Framework/TestCase.php#L500-L502?

Another option is to revert/remove the newly introduced TestCase::expectErrorLog() until php-src allows to check it without temporary file. Currently, it suffers not only the issue here, but it is also not the fastest to having to create temporary file before each TestCase run.

[staabm]

but it is also not the fastest to having to create temporary file before each TestCase run.

please share a benchmark which proves your claim - noone else reported something like that since the change was released. how big is the impact?

[mvorisek]

<?php

$t1 = 0;
$t2 = 0;

for ($i = 0; $i < 1000; ++$i) {
    $ts = microtime(true);
    $capture = tmpfile();
    $ts2 = microtime(true);
    $t1 += $ts2 - $ts;
    $path = stream_get_meta_data($capture)['uri'];
    $t2 += microtime(true) - $ts2;
}

var_dump($t1 / $i);
var_dump($t2 / $i);

T2 does not call any FS, T1 is about 0.4 ms / per iteration (TestCase run).

[staabm]

I mean a real world impact, not microbenchmarks.
I am aware that not running stuff is faster then running stuff, but thats not helpful.

[mvorisek]

In another words about 1 second extra time per 2500 tests :)

This is the real world overhead of requiring FS.

Let @sebastianbergmann decide this thread incl. what to throw.

[sebastianbergmann]

I hope to find the time to look into this within the next week.

[mvorisek]

@sebastianbergmann it seems you deleted the target brach without retargetting this PR first. Please have a look on this PR, currently we cannot upgrade to PHPUnit 12 everywhere because of this issue.

[sebastianbergmann]

I am so sorry about that! I need to add branch retargeting to my list of things to consider when releasing a new minor version and will try my best to avoid a situation like this in the future.

That being said, I cannot reopen this pull request as the target branch no longer exists. Therefore, I cannot change the target of this pull request to 12.2.

Can you please send a new pull request against 12.2 (or advise me on how to reopen/retarget)? Thank you!

[mvorisek]

👍 I cannot tell if the target branch will be recreated, if the PR could be reopened then or not. I will create another one into 12.2.