From f573adddf1905f0a40e79e41293964e1338eff30 Mon Sep 17 00:00:00 2001 From: Max Semenik Date: Thu, 18 Mar 2021 14:53:14 +0300 Subject: [PATCH] run-tests: use the EXTENSIONS section for skipping Currently, most skip checks are just for making sure an extension is available. Even with recent addition of skip caching, this makes tests needlessly slow: * Checks for the same extension in its tests can have small differences impacting cacheability. * Even identical skip checks in two tests can still be executed twice if they're run by different workers. To remedy this, I'm repurposing the existing --EXTENSIONS-- section of .phpt files to specify wjich extensions are required for current test to run. Current behavior: 1) If the extension is already visible to PHP, all is good 2) If it isn't, assume it's present as a shared module and attempt to add it via a command line parameter 3) If that works, all is good 4) If it doesn't, PHP fails with a cryptic error message trying to execute the test itself After this commit: 1) and 2) are kept unchanged 3) Check if shared extension file from 2) is actually present 4) Skip the test if it isn't Other benefits include clear skip reasons (vs. sometimes none in many current skip checks) and moving test information from code to metadata, opening more opportunities for search and analysis. Since --EXTENSIONS-- is barely ever used, this change poses no risk of hidden failures. As a demonstration of the new approach, this commit migrates one extension to it. If merged, I will migrate other extensions in subsequent PRs. --- ext/sodium/tests/bug78114.phpt | 6 ++---- ext/sodium/tests/bug78516.phpt | 3 ++- ext/sodium/tests/crypto_aead.phpt | 3 ++- ext/sodium/tests/crypto_auth.phpt | 4 ++-- ext/sodium/tests/crypto_box.phpt | 4 ++-- ext/sodium/tests/crypto_generichash.phpt | 4 ++-- ext/sodium/tests/crypto_hex.phpt | 4 ++-- ext/sodium/tests/crypto_kdf.phpt | 4 ++-- ext/sodium/tests/crypto_kx.phpt | 4 ++-- ext/sodium/tests/crypto_scalarmult.phpt | 4 ++-- ext/sodium/tests/crypto_secretbox.phpt | 4 ++-- ext/sodium/tests/crypto_secretstream.phpt | 3 ++- ext/sodium/tests/crypto_shorthash.phpt | 4 ++-- ext/sodium/tests/crypto_sign.phpt | 4 ++-- ext/sodium/tests/crypto_stream.phpt | 4 ++-- .../tests/exception_trace_without_args.phpt | 4 ++-- ext/sodium/tests/inc_add.phpt | 4 ++-- ext/sodium/tests/installed.phpt | 4 ++-- ext/sodium/tests/pwhash_argon2i.phpt | 4 +++- ext/sodium/tests/pwhash_scrypt.phpt | 4 +++- ext/sodium/tests/sodium_error_001.phpt | 4 ++-- ext/sodium/tests/utils.phpt | 4 ++-- ext/sodium/tests/version.phpt | 4 ++-- run-tests.php | 15 +++++++++++++-- tests/run-test/bug75042-2.phpt | 10 ---------- tests/run-test/bug75042-3.phpt | 13 ------------- .../{bug75042.phpt => extensions-shared.phpt} | 7 ++++--- tests/run-test/extensions-static.phpt | 9 +++++++++ 28 files changed, 74 insertions(+), 71 deletions(-) delete mode 100644 tests/run-test/bug75042-2.phpt delete mode 100644 tests/run-test/bug75042-3.phpt rename tests/run-test/{bug75042.phpt => extensions-shared.phpt} (91%) create mode 100644 tests/run-test/extensions-static.phpt diff --git a/ext/sodium/tests/bug78114.phpt b/ext/sodium/tests/bug78114.phpt index 6d7df4348faa9..b988c09ec23f8 100644 --- a/ext/sodium/tests/bug78114.phpt +++ b/ext/sodium/tests/bug78114.phpt @@ -1,9 +1,7 @@ --TEST-- Bug #78114 (segfault when calling sodium_* functions from eval) ---SKIPIF-- - +--EXTENSIONS-- +sodium --FILE-- --FILE-- diff --git a/ext/sodium/tests/crypto_aead.phpt b/ext/sodium/tests/crypto_aead.phpt index a54a1c7dc6758..0da84194bbcdf 100644 --- a/ext/sodium/tests/crypto_aead.phpt +++ b/ext/sodium/tests/crypto_aead.phpt @@ -1,8 +1,9 @@ --TEST-- Check for libsodium AEAD +--EXTENSIONS-- +sodium --SKIPIF-- --FILE-- diff --git a/ext/sodium/tests/crypto_auth.phpt b/ext/sodium/tests/crypto_auth.phpt index 58d8350078b36..8f262414beb80 100644 --- a/ext/sodium/tests/crypto_auth.phpt +++ b/ext/sodium/tests/crypto_auth.phpt @@ -1,7 +1,7 @@ --TEST-- Check for libsodium auth ---SKIPIF-- - +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- --FILE-- diff --git a/ext/sodium/tests/crypto_shorthash.phpt b/ext/sodium/tests/crypto_shorthash.phpt index 68bffc823b00b..c5cc2334a68a2 100644 --- a/ext/sodium/tests/crypto_shorthash.phpt +++ b/ext/sodium/tests/crypto_shorthash.phpt @@ -1,7 +1,7 @@ --TEST-- Check for libsodium shorthash ---SKIPIF-- - +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- --FILE-- --FILE-- +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- +--EXTENSIONS-- +sodium --FILE-- = 5; diff --git a/run-tests.php b/run-tests.php index 825718407c2c1..ad1311bb8d12c 100755 --- a/run-tests.php +++ b/run-tests.php @@ -2057,15 +2057,26 @@ function run_test(string $php, $file, array $env): string $extensions = preg_split("/[\n\r]+/", trim($test->getSection('EXTENSIONS'))); [$ext_dir, $loaded] = $skipCache->getExtensions("$php $pass_options $extra_options $ext_params $no_file_cache"); $ext_prefix = IS_WINDOWS ? "php_" : ""; + $missing = []; foreach ($extensions as $req_ext) { if (!in_array($req_ext, $loaded)) { if ($req_ext == 'opcache') { - $ini_settings['zend_extension'][] = $ext_dir . DIRECTORY_SEPARATOR . $ext_prefix . $req_ext . '.' . PHP_SHLIB_SUFFIX; + $ext_file = $ext_dir . DIRECTORY_SEPARATOR . $ext_prefix . $req_ext . '.' . PHP_SHLIB_SUFFIX; + $ini_settings['zend_extension'][] = $ext_file; } else { - $ini_settings['extension'][] = $ext_dir . DIRECTORY_SEPARATOR . $ext_prefix . $req_ext . '.' . PHP_SHLIB_SUFFIX; + $ext_file = $ext_dir . DIRECTORY_SEPARATOR . $ext_prefix . $req_ext . '.' . PHP_SHLIB_SUFFIX; + $ini_settings['extension'][] = $ext_file; + } + if (!is_readable($ext_file)) { + $missing[] = $req_ext; } } } + if ($missing) { + $message = 'Required extension' . (count($missing) > 1 ? 's' : '') + . ' missing: ' . implode(', ', $missing); + return skip_test($tested, $tested_file, $shortname, $message); + } } // additional ini overwrites diff --git a/tests/run-test/bug75042-2.phpt b/tests/run-test/bug75042-2.phpt deleted file mode 100644 index 2c5718a9a0211..0000000000000 --- a/tests/run-test/bug75042-2.phpt +++ /dev/null @@ -1,10 +0,0 @@ ---TEST-- -phpt EXTENSIONS directive with static module ---EXTENSIONS-- -SPL ---FILE-- - ---EXPECT-- -bool(true) diff --git a/tests/run-test/bug75042-3.phpt b/tests/run-test/bug75042-3.phpt deleted file mode 100644 index 5a30143be9d3a..0000000000000 --- a/tests/run-test/bug75042-3.phpt +++ /dev/null @@ -1,13 +0,0 @@ ---TEST-- -phpt EXTENSIONS directive with nonexistent shared module ---INI-- -error_log= -display_startup_errors=1 -display_errors=1 ---EXTENSIONS-- -nonexistentsharedmodule ---FILE-- - ---EXPECTF-- -Warning: PHP Startup: Unable to load dynamic library '%snonexistentsharedmodule.%s' %A diff --git a/tests/run-test/bug75042.phpt b/tests/run-test/extensions-shared.phpt similarity index 91% rename from tests/run-test/bug75042.phpt rename to tests/run-test/extensions-shared.phpt index a7979d6b5ed6e..e8d9b52aa98a9 100644 --- a/tests/run-test/bug75042.phpt +++ b/tests/run-test/extensions-shared.phpt @@ -1,5 +1,7 @@ --TEST-- -phpt EXTENSIONS directive with shared module +phpt EXTENSIONS directive - shared module +--EXTENSIONS-- +openssl --SKIPIF--