Add expectErrorLog() for expecting error_log() output#6118
Add expectErrorLog() for expecting error_log() output#6118staabm wants to merge 15 commits intosebastianbergmann:mainfrom
expectErrorLog() for expecting error_log() output#6118Conversation
|
I tried debugging it a bit more to find out how phpunit knows what the unit-under-test passed to I can see the string also be mentioned in the event log stream of |
|
ohh I think I understand the problem now. |
1507fae to
fa8961d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6118 +/- ##
=========================================
Coverage 95.29% 95.29%
- Complexity 6755 6758 +3
=========================================
Files 726 726
Lines 21300 21311 +11
=========================================
+ Hits 20298 20309 +11
Misses 1002 1002 ☔ View full report in Codecov by Sentry. |
a078ae1 to
5b5c7ee
Compare
|
I think we are in better shape now (put a new summary into the PR description) |
| $this->outputExpectedString = $expectedString; | ||
| } | ||
|
|
||
| final protected function expectsErrorLog(): void |
There was a problem hiding this comment.
This should be expectErrorLog() to be consistent with expectOutputString, for example.
expectErrorLog() for expecting error_log() output
| @@ -0,0 +1,31 @@ | |||
| --TEST-- | |||
There was a problem hiding this comment.
expectErrorLog() is a new feature. Therefore, we should not put its end-to-end tests into the regression directory. The generic would be acceptable for now.
|
great catch, fixed it. thank you! |
|
Merged manually, thanks. |
| $testArguments = array_merge($this->data, array_values($this->dependencyInput)); | ||
|
|
||
| $capture = tmpfile(); | ||
| $errorLogPrevious = ini_set('error_log', stream_get_meta_data($capture)['uri']); |
There was a problem hiding this comment.
I saw your php-src feature request to support in memory stream. I think stream wrapper would work here - https://www.php.net/manual/en/stream.streamwrapper.example-1.php
There was a problem hiding this comment.
Also creating tmp. file for every testcase is far from ideal.
There was a problem hiding this comment.
feel free to improve the implementation with a followup PR
There was a problem hiding this comment.
I think stream wrapper would work here - https://www.php.net/manual/en/stream.streamwrapper.example-1.php
sadly even this is currently not possible - php/php-src#18530
summary:
error_log()without a call toexpectsErrorLog(), the output is directly rendered in-between the summary (as tested in Regression test for error_log() #6127)error_log()with a call toexpectsErrorLog(), no more output is rendered in-between the test summary printing. when noerror_log()is invoked the test is considered failed.backstory:
while looking into #2155 (comment) I realized PHPunit is already somehow capturing the
error_log()output.I think this means we don't need a workaround like #2155 (comment) in phpunit itself, but just a API which allows us to make assertions on the already captured output.
DX wise I think there is also some room for improvement since the
error_log'ged string just shows up in the test-debug output without any indication where this string is coming from (the test-author needs to look into the code beeing tested to realize that this string originates from aerror_log) or how to make assertions on it.With this PR I am just sending a test so we can see the status quo and discuss where to move from here
closes #2155