Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Allow running PHPUnit with "xdebug.scream" ON#29645
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
nicolas-grekas left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Works for me, but note that running withxdebug.scream is unsupported in the generic case: in some situations (eg some race conditions), there is no way around triggering a silenced notice.
| } | ||
| @unlink(\dirname($dir.$file).'.legacy'); | ||
| $legacyFile =\dirname($dir.$file).'.legacy'; | ||
| if (\file_exists($legacyFile)) { |
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.
please remove the\ here (but keep it for dirname)
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.
absolutely, I've found a mix of both in the codebase so didn't know which was the preferred one. all done now
Sincesymfony#25733 the Kernel attempts to unlink the legacy container while being built.This throws an error if the file did not exist, for example on a clean install, on the build, which is then silenced.That's fine on production systems, but on our build we have enabled "xdebug.scream" in order to visualise every errors, which basically un-silences the errors. I believe there should not be a need to silence anything on a usual, clean usage of the system.Making this `unlink` conditional fixes it.Could you please approve and merge this PR?Thanks
nicolas-grekas commentedDec 19, 2018
Thank you@nico-incubiq. |
…biq)This PR was submitted for the master branch but it was merged into the 3.4 branch instead (closes#29645).Discussion----------Allow running PHPUnit with "xdebug.scream" ON| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no <!-- don't forget to update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets | <!-- #-prefixed issue number(s), if any -->| License | MIT| Doc PR | <!-- required for new features -->Since#25733 the Kernel attempts to unlink the legacy container while being built.This throws an error if the file did not exist, for example on a clean install, on the build, which is then silenced.That's fine on production systems, but on our build we have enabled `xdebug.scream` in order to visualise every errors, which basically un-silences the errors. I believe there should not be a need to silence anything on a usual, clean usage of the system.Making this `unlink` conditional fixes it.Could you please approve and merge this PR?ThanksCommits-------7a7165e Allow running PHPUnit with "xdebug.scream" ON
Uh oh!
There was an error while loading.Please reload this page.
Since#25733 the Kernel attempts to unlink the legacy container while being built.
This throws an error if the file did not exist, for example on a clean install, on the build, which is then silenced.
That's fine on production systems, but on our build we have enabled
xdebug.screamin order to visualise every errors, which basically un-silences the errors. I believe there should not be a need to silence anything on a usual, clean usage of the system.Making this
unlinkconditional fixes it.Could you please approve and merge this PR?
Thanks