Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[PHPUnit Bridge]Implement poor man's error handler#21795
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| $group ='other'; | ||
| $isWeak = DeprecationErrorHandler::MODE_WEAK ===$mode || (DeprecationErrorHandler::MODE_WEAK_VENDORS ===$mode &&$isVendor =$inVendors($file)); | ||
| $isWeak = DeprecationErrorHandler::MODE_WEAK ===$mode || (self::MODE_WEAK_VENDORS ===$mode &&$isVendor =$inVendors($file)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this change is a no-go. The PHPUnit Bridge is still compatible with PHP 5.3 even in master (our own testsuite uses the newest bridge even in LTS branches)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@stof: this is to demonstrate the error handling, and will be removed from the PR before merging. Please read the commit messages :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Any way to showcase it in a test case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think that would imply isolating it in a public method. Or maybe… in a private method, and then do a bit of black magic to make it public with reflection. I'll see what I can do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hmm… that won't be so easy since we're inside a closure, which is itself inside another closure. But the condition to run this piece of code is just to change the error handler. Let's try that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why focus on this specific way to trigger a failure? Aren't there "userland" side to showcase the new behavior in e.g. a phpt test case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
No there aren't AFAIK. I think this only happens when there is a programming error in the deprecation error handler. Should I think otherwise? I only saw it in action when (failing at) writing my other PR on this error handler, so maybe I'm missing other possible ways to trigger this piece of code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Really? Then we don't care sorry...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Argh. Please reconsider… these few lines can save literal hours to anyone try to contribute to this error handler. I know the audience is small, but it exists.
This line of code is hit when the error handler has failed. In thatcase, we want to get errors in the most reliable possible way, with verysimple code. I think var_export and error_get_last are good candidatesfor that.
7e2c5e8 to7832b32Comparegreg0ire commentedMar 1, 2017
You probably know better, let's just close this. |
Uh oh!
There was an error while loading.Please reload this page.
This line of code is hit when the error handler has failed. In that
case, we want to get errors in the most reliable possible way, with very
simple code. I think var_export and error_get_last are good candidates
for that.
Here is what it looks like. I will remove the purposefully wrong commit once everyone is ok with the output. Please not that in that case, it's a
phpt, so it's a bit special.Also, to elaborate a bit on the why, if you feel like people developing on the deprecation error handler could easily add these lines and remove them after debugging their problem, bear in mind that some problems, just like the one I introduced to test this, occur on some builds only.
The problem I introduced was a mistake I made in#21539 .
To debug it I had to find the right Travis container from quay.io, set uptravis-build on it, which is quite an ordeal, and then run the generated
ci.shscript.