mirror of
https://github.com/php/pftt2.git
synced 2026-03-24 09:12:17 +01:00
Some smoke tests are suppressed for PHP 7 #58
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @cmb69 on GitHub (Jan 2, 2020).
The smoke test for
phpinfo()output is suppessed for PHP 7. This is likely because these checks look brittle in the current form. We should have a look into this, and maybe just drop them altogether.@lavturo commented on GitHub (Jan 2, 2020):
PFTT has an array of strings to look for: NTS parts and TS parts.
It seems that the list for these parts were created manually based on an old PHP build, so the list is probably outdated. For example, one of the required features is:
Registered Stream Filters => convert.iconv.*, mcrypt.*, mdecrypt.*, string.rot13, string.toupper, string.tolower, string.strip_tags, convert.*, consumed, dechunk, zlib.*However, looking through a php-5.6.40-nts build we have:
Registered Stream Filters => convert.iconv.*, mcrypt.*, mdecrypt.*, string.rot13, string.toupper, string.tolower, string.strip_tags, convert.*, consumed, dechunk, zlib.*, bzip2.*It is slightly different, but fails the required test. I assume the reason that
build.is7(cm, host)is there because that list is based off of 5.x builds.Therefore, we could get rid of this altogether (as you said) or manually update the current list (but we would also have to create a separate phpinfo list for 7.x builds).
@cmb69 commented on GitHub (Jan 3, 2020):
It seems to me that this smoke test in its current form doesn't make much sense. There are many irrelevant checks, e.g. it is checked whether
Compiler =>is contained in PHP info, but not that a certain compiler has been used for the tested build. Other checks may be just too overly picky.Regarding the implementation, I wonder why this is a concatenated string literal, which is then split, instead of having an array literal in the first place. The latter would more easily allow to replace/add/remove certain entries, so we wouldn't need to have multiple large initializers which are very similar.
@lavturo commented on GitHub (Jan 3, 2020):
Yes. It seems to try and check if certain information is just in the phpinfo. I am assuming they made that list based on a phpinfo of an older php build.
We could have an entire overhaul of this. However, is it necessary to check the phpinfo to see if certain information is there? As you pointed out, there are many irrelevant checks.
@dalehhirt commented on GitHub (Jan 6, 2020):
Do we even need 90% of the smoke test? What is the overall purpose of the smoke test, and perhaps we can rewrite this to only look for the most absolutely necessary parts?
@cmb69 commented on GitHub (Jan 6, 2020):
The overall purpose is to be to bail out early, if anything with the PHP build/configuration is not what is expected by PFTT. Otherwise PFTT may run for quite some time, but produce many false positives or skip many tests. Thinking about it, we likely should get rid of this smoke test altogether; after all, we've been running PFTT for quite a while without this smoke test active, and apparently there have been no real issues.