Remove " " being considered as an invalid filename for is_file()

[patrickallaert]

Some builds fail because of it, last one to date is: https://travis-ci.com/github/php/php-src/jobs/517817543

[ramsey]

Looks like that was added to the test 13 years ago: f689708

[ramsey]

I've been trying to think through this and figure out whether a space character should still be tested, but if space can be a valid filename, then is_file() shouldn't always return false. So, removing it from the test is the right approach, since this test assumes space is an invalid value.

[nikic]

This is probably test parallelisation issue. Some other test must be creating that file, else it wouldn't exist.

[patrickallaert]

Looks like that was added to the test 13 years ago: f689708

Indeed, what I wonder is how many failed builds have been triggered by this one.

[ramsey]

This is probably test parallelisation issue. Some other test must be creating that file, else it wouldn't exist.

I also think this is what is happening, but if a space is a valid file name, then is_file() should return true for it, and this test assumes space is an invalid file name value, so it's trying to assert that it always returns false, which isn't the case.

Given that, it seems to me this is safe to remove.

Why anyone would want to use U+0020 as a file name, I have no idea. 🤷🏻‍♂️

[krakjoe]

Nowadays, we don't test ZPP, and will refuse to merge such tests. This test was clearly written to test the 'p' specifier for ZPP, and some false assumptions were made about how paths were handled at that time.

IMO the test should be removed, or rewritten so that it makes sense. If this test provides no additional coverage to php_stat, then drop it.

[patrickallaert]

Nowadays, we don't test ZPP, and will refuse to merge such tests. This test was clearly written to test the 'p' specifier for ZPP, and some false assumptions were made about how paths were handled at that time.

IMO the test should be removed, or rewritten so that it makes sense. If this test provides no additional coverage to php_stat, then drop it.

Did tests with code coverage with and without the file: no changes to the whole filestat.c (containing php_stat).
Should I adapt this PR to drop the whole file @krakjoe?

[krakjoe]

Yes please.