Implement support for tests loading all extensions #16251
Conversation
run-tests.php
Outdated
| @@ -2027,6 +2028,9 @@ function run_test(string $php, $file, array $env): string | |||
| $extensions = []; | |||
| if ($test->hasSection('EXTENSIONS')) { | |||
| $extensions = preg_split("/[\n\r]+/", trim($test->getSection('EXTENSIONS'))); | |||
| if (in_array("*", $extensions, true)) { | |||
| $extensions = array_diff($exts_to_test, ["php8apache2_4", "php8phpdbg", "php8ts_debug"]); | |||
There was a problem hiding this comment.
The list of exclusions doesn't belong here; this needs to be handled otherwise. Maybe somebody has an idea (except for using dl()).
There was a problem hiding this comment.
Could probably require the php_ prefix:
Line 869 in b445641
Or is that an issue on non Windows platforms?
There was a problem hiding this comment.
Instead of * manual list with ? prefix - loaded only if present?
There was a problem hiding this comment.
The whole point is to load all available extensions (even for external extensions).
There was a problem hiding this comment.
I mean * can be dangerous as it can load unwanted non-php-src extensions.
There was a problem hiding this comment.
What do you mean by "dangerous"? Security related, or just that even external extensions would be tested as well. If the latter, in my opinion, this is a good thing, since all extensions should conform to the contracts established by these tests. If some extensions don't, it will be noticed, and the extensions can be fixed.
There was a problem hiding this comment.
I mean the latter, if it is wanted to load/run unknown extension at all.
When running the tests, extensions are already loaded, or are requested via the `--EXTENSIONS--` section explicitly. Some tests, though, would benefit from loading and testing all available extension. We make that possible by adding support for the `*` wildcard.
cmb69
force-pushed
the
cmb/extensions-wildcard
branch
from
646b851 to
becf31f
Compare
|
It works – Windows segfaults as expected. Now on to PR #10278. |
run-tests.php
Outdated
| @@ -866,7 +866,7 @@ function write_information(array $user_tests, $phpdbg): void | |||
| $exts = get_loaded_extensions(); | |||
| $ext_dir = ini_get('extension_dir'); | |||
| foreach (scandir($ext_dir) as $file) { | |||
| if (preg_match('/^(?:php_)?([_a-zA-Z0-9]+)\.(?:so|dll)$/', $file, $matches)) { | |||
| if (preg_match('/^(?:php_)([_a-zA-Z0-9]+)\.(?:so|dll)$/', $file, $matches)) { | |||
There was a problem hiding this comment.
Note that this change is not directly related to this PR (although it is required to make this patch work on Windows, at least). If desired, I can provide a separate PR for this.
There was a problem hiding this comment.
Unfortunately I believe that the php_ prefix is specific to windows (at least there is no prefix on linux).
Maybe using a block list would be fine. Otherwise I'm not sure how we can detect that a particular .so/.dll file is an extension. We could check if it defines a ${name}_module_entry symbol but this will not be portable.
There was a problem hiding this comment.
Unfortunately I believe that the
php_prefix is specific to windows (at least there is no prefix on linux).
Ah, thank you! Maybe we should just used slightly different regexps on Windows and other platforms? Another option would be for Windows builds to put the extension DLLs in the ext/ folder; I'll have a look at this.
There was a problem hiding this comment.
Okay, I've pushed a commit separating the regexps (Windows vs. non-Windows). An early draft of putting the extension DLLs in a subfolder is PR #16282.
|
Do we need this for Zend/tests/new_oom.phpt? |
Would probably make sense. |
|
Okay, apparently all extensions are loaded now on all platforms; however, I'm not sure what would happen if a zend_extension would be loaded (probably a bad idea). Need to think about that; converting to draft for the time being. |
|
Just going to cross-link #16317 as another case where a test for all extensions would be helpful |
When running the tests, extensions are already loaded, or are requested via the
--EXTENSIONS--section explicitly. Some tests, though, would benefit from loading and testing all available extension. We make that possible by adding support for the*wildcard.