Load extensions on demand for AppVeyor CI #6993
Conversation
PR php#6787 changed the behavior of the `--EXTENSIONS--` section, so that not yet loaded extensions are dynamically loaded if possible. However, when no tests are specified for the runner, only tests for already loaded extensions are run, what defeats the purpose of the improvement of the `--EXTENSIONS--` behavior. We cater to that by detecting loadable extensions, and also run their tests.
Several extension test suites have already been changed to rely on the new behavior of `--EXTENSIONS--` section, i.e. to dynamically load required extensions on demand. Thus, there is no need to load these extensions always, which should improve the performance of executing the tests.
| @@ -51,7 +51,7 @@ if %errorlevel% neq 0 exit /b 3 | |||
| if "%THREAD_SAFE%" equ "0" set ADD_CONF=%ADD_CONF% --disable-zts | |||
| if "%INTRINSICS%" neq "" set ADD_CONF=%ADD_CONF% --enable-native-intrinsics=%INTRINSICS% | |||
|
|
|||
| set EXT_EXCLUDE_FROM_TEST=snmp,oci8_12c,pdo_oci,pdo_firebird,ldap,imap,ftp | |||
| set EXT_EXCLUDE_FROM_TEST=bz2,exif,fileinfo,ffi,ftp,gmp,imap,ldap,oci8_12c,pdo_firebird,pdo_oci,snmp,soap,sodium,sqlite3,tidy | |||
There was a problem hiding this comment.
Would it be possible to convert this into a whitelist rather than a blacklist? I think that will make more sense going forward.
There was a problem hiding this comment.
This value is passed to the config option --with-test-ini-ext-exclude where blacklisting generally makes sense (it's only used if --enable-snapshot-build is also given, and usually you want to work with all available extensions).
For the test runner on AppVeyor, whitelisting would make more sense in the long run; would be trivial to change if it wasn't batch sctipting. I'll try to have a look.
There was a problem hiding this comment.
Whitelist would be nice going forward, but I think we can land this as-is. Maybe add a comment that (most of) these are actually tested, just loaded via --EXTENSIONS--.
| @@ -51,7 +51,7 @@ if %errorlevel% neq 0 exit /b 3 | |||
| if "%THREAD_SAFE%" equ "0" set ADD_CONF=%ADD_CONF% --disable-zts | |||
| if "%INTRINSICS%" neq "" set ADD_CONF=%ADD_CONF% --enable-native-intrinsics=%INTRINSICS% | |||
|
|
|||
| set EXT_EXCLUDE_FROM_TEST=snmp,oci8_12c,pdo_oci,pdo_firebird,ldap,imap,ftp | |||
| set EXT_EXCLUDE_FROM_TEST=bz2,exif,fileinfo,ffi,ftp,gmp,imap,ldap,oci8_12c,pdo_firebird,pdo_oci,snmp,soap,sodium,sqlite3,tidy | |||
There was a problem hiding this comment.
Whitelist would be nice going forward, but I think we can land this as-is. Maybe add a comment that (most of) these are actually tested, just loaded via --EXTENSIONS--.
|
@cmb69 poke for landing :) |
This is so far only possible for a few extensions.