Fixes Bug #66960 #3013
Closed
Fixes Bug #66960 #3013
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If given a non-existent path that exceeds the build defined maximum path length, we should just not find the file rather than dumping core.
|
Putting it in the current directory is fine, but there should be a As a general note, phpt tests should use var_dump rather than assert. Apart from assert not being guaranteed to execute (you'd need to specify additional inis), it is not debuggable. It's borderline in this case because you can infer from the assertion failure that the return value is true rather than false, but apart from the case of booleans specifically, assert does not tell you what the actual value was, which is especially important for CI failures which cannot be directly debugged. |
|
@nikic I appreciate the feedback, thank you and addressed. |
|
Merged as cb73872, thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If given a non-existent path that exceeds the build-defined maximum path length, we should not find the file -- rather than dumping core as described in the bug report. This PR adds a test asserting the buggy behavior reported in 2014 doesn't re-appear.
https://bugs.php.net/bug.php?id=66960
Note that the use of
__DIR__in this test is as a temporary directory, as I'm not sure tests can rely uponsys_get_temp_dir()to reliably avoid test false failures.