Fix TestCase::expectErrorLog() with open_basedir php.ini #6227
Conversation
| } | ||
| } elseif ($this->expectErrorLog) { | ||
| $this->markTestIncomplete('Could not create writable error_log file.'); |
There was a problem hiding this comment.
Unresolved feedback from the original PR - #6208 (comment)
The question is what to throw here, incomplete, risky, ...?
There was a problem hiding this comment.
@sebastianbergmann kindly ping as discussed in the previous PR.
There was a problem hiding this comment.
Treating the test as incomplete (or risky) is too weak, IMO. This should be an error.
|
I forgot about branch retargeting (again). However, I was able to regarget this pull request to |
|
The more I think about #6197 and the changes proposed here to address it, the more I seem to lean towards "won't fix". PHPUnit is a development tool that should only be used in a development environment. I cannot think of a situation where it makes sense to configure As it stands, I am not convinced that the added complexity of providing a more specific error message for this edge case is worthwhile. |
|
We run our whole development (and production) suite with The reason is we can protect our enviroment (project's unrelated files) better. This way we also assert no code accesses locations outside the projects. This single issue prevents us to upgrade to PHPUnit 12. I hope this can be fixed. I hope this PR is more like a bugfix, the tmp location can be readonly, not accessible at all, etc., so the file creation should be handled in any case. To finish this PR, I have reopened #6227 (comment) discussion. Please guide me how to emit an error you want or feel free to commit into this PR directly. |
|
This pull request has conflicts that must be resolved. |
|
PR rebased |
|
Thank you for your patience. I have cherry-picked your changes to |
|
Thank you! For documentation purposes, the change is in cc6414a. |
fix #6197
rebased version of #6208 (because of #6208 (comment))