Skip to content

Commit a9edf31

Browse files
chubes4chubes4
authored
Fail closed on PHPUnit discovery errors (#1940)
* Capture malformed PHPUnit bootstrap payloads * Treat empty PHPUnit suites as successful * Fail closed on PHPUnit discovery errors
1 parent 4cdad6a commit a9edf31

5 files changed

Lines changed: 185 additions & 79 deletions

File tree

‎packages/runtime-core/src/command-registry.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -944,8 +944,9 @@ export const commandRegistry = [
944944
{ name: "project-autoload-file", description: "Project/plugin autoload path loaded after the project bootstrap has provided WordPress functions.", format: "sandbox path" },
945945
{ name: "tests-dir", description: "WP PHPUnit tests directory inside the sandbox.", format: "sandbox path" },
946946
{ name: "phpunit-xml", description: "phpunit.xml path inside the plugin.", format: "path" },
947+
{ name: "phpunit-xml-default", description: "Marks phpunit-xml as the standard default path, allowing an adjacent phpunit.xml fallback only when phpunit.xml.dist is absent.", format: "boolean" },
947948
{ name: "test-file", description: "Single test file to run.", format: "path" },
948-
{ name: "changed-tests-json", description: "Changed test files for diagnostics.", format: "JSON array" },
949+
{ name: "changed-tests-json", description: "Optional changed-scope selection. A non-empty JSON array of test file paths runs only matching files and may legitimately select zero tests.", format: "JSON array of non-empty paths" },
949950
{ name: "env-json", description: "PHPUnit environment values.", format: "JSON object" },
950951
{ name: "wp-config-defines-json", description: "wp-config.php constants for the run.", format: "JSON object" },
951952
{ name: "dependency-mounts", description: "Comma-separated mounted dependency paths loaded and activated after managed PHPUnit installation, before tests execute.", format: "comma-separated sandbox paths" },
@@ -1062,8 +1063,9 @@ export const commandRegistry = [
10621063
{ name: "core-root", description: "WordPress develop checkout root inside the sandbox. Must contain vendor/ with Composer dev dependencies (PHPUnit + yoast/phpunit-polyfills) installed before mounting.", format: "sandbox path" },
10631064
{ name: "tests-dir", description: "Core tests directory inside the sandbox (expects includes/bootstrap.php under it).", format: "sandbox path" },
10641065
{ name: "phpunit-xml", description: "phpunit.xml path.", format: "path" },
1066+
{ name: "phpunit-xml-default", description: "Marks phpunit-xml as the standard default path, allowing an adjacent phpunit.xml fallback only when phpunit.xml.dist is absent.", format: "boolean" },
10651067
{ name: "test-file", description: "Single test file to run.", format: "path" },
1066-
{ name: "changed-tests-json", description: "Changed test files for diagnostics.", format: "JSON array" },
1068+
{ name: "changed-tests-json", description: "Optional changed-scope selection. A non-empty JSON array of test file paths runs only matching files and may legitimately select zero tests.", format: "JSON array of non-empty paths" },
10671069
{ name: "autoload-file", description: "Autoload path inside the sandbox (typically <core-root>/vendor/autoload.php from a completed composer install).", format: "sandbox path" },
10681070
{ name: "wp-config-defines-json", description: "wp-config.php constants for the run.", format: "JSON object" },
10691071
{ name: "multisite", description: "Run as multisite.", format: "boolean" },

‎packages/runtime-core/src/recipe-builders.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ export function buildWordPressPhpunitRecipe(options: WordPressPhpunitRecipeOptio
110110
commandArg("tests-dir", options.testsDir ?? "/wp-codebox-vendor/wp-phpunit/wp-phpunit"),
111111
commandArg("test-root", options.testRoot ?? `${pluginTarget}/tests`),
112112
commandArg("phpunit-xml", options.phpunitXml ?? `${pluginTarget}/phpunit.xml.dist`),
113+
commandArg("phpunit-xml-default", options.phpunitXml === undefined ? "1" : ""),
113114
commandStringListArg("dependency-mounts", options.dependencyMounts ?? []),
114115
commandJsonArg("bootstrap-files-json", options.bootstrapFiles ?? []),
115116
commandJsonArg("preload-files-json", options.preloadFiles ?? []),

‎packages/runtime-playground/src/phpunit-command-handlers.ts‎

Lines changed: 26 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ export interface PhpunitRunCodeOptions {
99
testsDir: string
1010
testRoot?: string
1111
phpunitXml: string
12+
phpunitXmlIsDefault: boolean
1213
selectedTestFile: string
13-
changedTestFiles: unknown[]
14+
changedTestFiles: string[]
1415
phpunitArgs: string[]
1516
env: Record<string, unknown>
1617
wpConfigDefines: Record<string, unknown>
@@ -32,8 +33,9 @@ export interface CorePhpunitRunCodeOptions {
3233
coreRoot: string
3334
testsDir: string
3435
phpunitXml: string
36+
phpunitXmlIsDefault: boolean
3537
selectedTestFile: string
36-
changedTestFiles: unknown[]
38+
changedTestFiles: string[]
3739
autoloadFile: string
3840
wpConfigDefines: Record<string, unknown>
3941
multisite: boolean
@@ -51,9 +53,6 @@ export const PLUGIN_PHPUNIT_RESULT_FILE = "/tmp/wp-codebox-phpunit-result.txt"
5153
interface PhpunitConfigDiscoveryPhpOptions {
5254
functionName: string
5355
logFunction: string
54-
missingConfigMessage: string
55-
parseFailureMessage: string
56-
includeParseFailureDetail: boolean
5756
loadedConfigMessage: string
5857
fallbackXmlDist: boolean
5958
restrictDirectoriesToTests: boolean
@@ -68,7 +67,6 @@ interface PhpunitChangedTestFilterPhpOptions {
6867
logFunction: string
6968
rootParameterName: string
7069
testsPathFallback: boolean
71-
emptyWantedNotice: boolean
7270
}
7371

7472
function phpunitConfigDiscoveryPhp(options: PhpunitConfigDiscoveryPhpOptions): string {
@@ -90,9 +88,6 @@ function phpunitConfigDiscoveryPhp(options: PhpunitConfigDiscoveryPhpOptions): s
9088
const returnValues = options.uniqueReturnValues
9189
? "array(array_values(array_unique($directories)), array_values(array_unique($suffixes)), array_values(array_unique($prefixes)), $excludes, array_values(array_unique($files)))"
9290
: "array($directories, $suffixes, $prefixes, $excludes, $files)"
93-
const parseFailureLog = options.includeParseFailureDetail
94-
? `${options.logFunction}('${options.parseFailureMessage}' . $first . '); using defaults');`
95-
: `${options.logFunction}('${options.parseFailureMessage}');`
9691
const suffixAssignment = options.replaceDefaultMatchers ? "$suffixes = $config_suffixes;" : "$suffixes = array_merge($suffixes, $config_suffixes);"
9792
const prefixAssignment = options.replaceDefaultMatchers ? "$prefixes = $config_prefixes;" : "$prefixes = array_merge($prefixes, $config_prefixes);"
9893

@@ -106,8 +101,7 @@ function phpunitConfigDiscoveryPhp(options: PhpunitConfigDiscoveryPhpOptions): s
106101
return ${returnValues};
107102
};${fallbackXmlDist}
108103
if (!is_readable($xml_path)) {
109-
${options.logFunction}('${options.missingConfigMessage}' . $xml_path . '; using defaults');
110-
return $return_values();
104+
throw new RuntimeException('PHPUnit config is not readable: ' . $xml_path);
111105
}
112106
$prev = libxml_use_internal_errors(true);
113107
$xml = @simplexml_load_file($xml_path);
@@ -116,8 +110,7 @@ function phpunitConfigDiscoveryPhp(options: PhpunitConfigDiscoveryPhpOptions): s
116110
libxml_use_internal_errors($prev);
117111
if ($xml === false) {
118112
$first = $errors ? trim($errors[0]->message) : 'unknown';
119-
${parseFailureLog}
120-
return $return_values();
113+
throw new RuntimeException('PHPUnit config could not be parsed at ' . $xml_path . ': ' . $first);
121114
}
122115
$base = ${options.basePathExpression};
123116
$config_dirs = array();
@@ -170,11 +163,10 @@ function phpunitDiscoveryPhp(functionName: string, logFunction: string): string
170163
$found = array();
171164
foreach ($files as $file) {
172165
if (!is_string($file) || $file === '') {
173-
continue;
166+
throw new RuntimeException('configured PHPUnit test file is invalid');
174167
}
175-
if (!is_file($file) || pathinfo($file, PATHINFO_EXTENSION) !== 'php') {
176-
${logFunction}('NOTICE:test file does not exist: ' . $file);
177-
continue;
168+
if (!is_file($file) || !is_readable($file) || pathinfo($file, PATHINFO_EXTENSION) !== 'php') {
169+
throw new RuntimeException('configured PHPUnit test file is not a readable PHP file: ' . $file);
178170
}
179171
foreach ($excludes as $exclude) {
180172
if (strpos($file, $exclude) === 0) {
@@ -184,9 +176,8 @@ function phpunitDiscoveryPhp(functionName: string, logFunction: string): string
184176
$found[] = $file;
185177
}
186178
foreach ($directories as $dir) {
187-
if (!is_dir($dir)) {
188-
${logFunction}('NOTICE:test directory does not exist: ' . $dir);
189-
continue;
179+
if (!is_dir($dir) || !is_readable($dir)) {
180+
throw new RuntimeException('configured PHPUnit test directory is not a readable directory: ' . $dir);
190181
}
191182
$iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($dir, RecursiveDirectoryIterator::SKIP_DOTS), RecursiveIteratorIterator::LEAVES_ONLY);
192183
foreach ($iterator as $file) {
@@ -229,12 +220,6 @@ function phpunitChangedTestFilterPhp(options: PhpunitChangedTestFilterPhpOptions
229220
const testsPathFallback = options.testsPathFallback ? ` elseif (strpos($path, '/tests/') !== false) {
230221
$path = substr($path, strpos($path, '/tests/') + 1);
231222
}` : ""
232-
const emptyWantedNotice = options.emptyWantedNotice ? `
233-
if (empty($wanted)) {
234-
${options.logFunction}('NOTICE:changed tests did not contain usable test paths');
235-
return array();
236-
}` : ""
237-
238223
return `function ${options.filterFunctionName}(array $test_files, string $changed_files_json, string $${options.rootParameterName}): array {
239224
$decoded = json_decode($changed_files_json, true);
240225
if (!is_array($decoded) || empty($decoded)) {
@@ -249,7 +234,7 @@ function phpunitChangedTestFilterPhp(options: PhpunitChangedTestFilterPhpOptions
249234
if ($normalized !== '') {
250235
$wanted[$normalized] = true;
251236
}
252-
}${emptyWantedNotice}
237+
}
253238
$filtered = array();
254239
foreach ($test_files as $file) {
255240
if (isset($wanted[${options.relativeFunctionName}((string) $file, $${options.rootParameterName})])) {
@@ -360,6 +345,7 @@ $tests_dir = ${JSON.stringify(options.testsDir)};
360345
$test_root = ${JSON.stringify(options.testRoot || `/wordpress/wp-content/plugins/${options.pluginSlug}/tests`)};
361346
$selected_test_file = ${JSON.stringify(options.selectedTestFile)};
362347
$changed_test_files_raw = ${JSON.stringify(JSON.stringify(options.changedTestFiles))};
348+
$changed_test_scope = ${JSON.stringify(options.changedTestFiles.length > 0)};
363349
$phpunit_args_raw = json_decode(${JSON.stringify(JSON.stringify(options.phpunitArgs))}, true);
364350
$bench_env = json_decode(${JSON.stringify(JSON.stringify(options.env))}, true);
365351
$wp_config_defines = json_decode(${JSON.stringify(JSON.stringify(options.wpConfigDefines))}, true);
@@ -765,8 +751,8 @@ function pg_plugin_real_path(string $relative_path, string $kind): ?string {
765751
return $real;
766752
}
767753
768-
function pg_project_bootstrap_from_config(string $xml_path): string {
769-
if (!is_readable($xml_path) && basename($xml_path) === 'phpunit.xml.dist') {
754+
function pg_project_bootstrap_from_config(string &$xml_path, bool $xml_is_default): string {
755+
if ($xml_is_default && !is_readable($xml_path) && basename($xml_path) === 'phpunit.xml.dist') {
770756
$alternate = dirname($xml_path) . '/phpunit.xml';
771757
if (is_readable($alternate)) {
772758
$xml_path = $alternate;
@@ -838,9 +824,10 @@ function pg_run_project_bootstrap_stage(array $cfg): void {
838824
try {
839825
$bootstrap = trim((string) ($cfg['project_bootstrap'] ?? ''));
840826
$phpunit_xml = (string) ($cfg['phpunit_xml'] ?? '');
827+
$phpunit_xml_is_default = !empty($cfg['phpunit_xml_is_default']);
841828
$from_config = false;
842829
if ($bootstrap === '') {
843-
$bootstrap = pg_project_bootstrap_from_config($phpunit_xml);
830+
$bootstrap = pg_project_bootstrap_from_config($phpunit_xml, $phpunit_xml_is_default);
844831
$from_config = $bootstrap !== '';
845832
}
846833
$bootstrap_real = pg_project_bootstrap_real_path($bootstrap, $phpunit_xml, $from_config);
@@ -1038,7 +1025,6 @@ ${phpunitChangedTestFilterPhp({
10381025
logFunction: "pg_log",
10391026
rootParameterName: "plugin_path",
10401027
testsPathFallback: true,
1041-
emptyWantedNotice: true,
10421028
})}
10431029
10441030
pg_install_diagnostics_handlers();
@@ -1089,7 +1075,7 @@ $config_path = pg_run_boot_stage(array('extra_defines' => $wp_config_defines, 't
10891075
if ($bootstrap_mode === 'project') {
10901076
pg_prepare_project_bootstrap_environment($config_path);
10911077
pg_skip_project_bootstrap_shell_install();
1092-
pg_run_project_bootstrap_stage(array('project_bootstrap' => $project_bootstrap, 'phpunit_xml' => ${JSON.stringify(options.phpunitXml)}));
1078+
pg_run_project_bootstrap_stage(array('project_bootstrap' => $project_bootstrap, 'phpunit_xml' => ${JSON.stringify(options.phpunitXml)}, 'phpunit_xml_is_default' => ${JSON.stringify(options.phpunitXmlIsDefault)}));
10931079
pg_run_project_autoload_stage($project_autoload_file !== '' ? $project_autoload_file : $legacy_project_autoload_file);
10941080
} else {
10951081
if ($bootstrap_mode !== 'managed') {
@@ -1216,11 +1202,8 @@ try {
12161202
${phpunitConfigDiscoveryPhp({
12171203
functionName: "wp_codebox_phpunit_parse_config",
12181204
logFunction: "pg_log",
1219-
missingConfigMessage: "NOTICE:phpunit.xml.dist not readable at ",
1220-
parseFailureMessage: "NOTICE:phpunit.xml.dist parse failed (",
1221-
includeParseFailureDetail: true,
12221205
loadedConfigMessage: "NOTICE:phpunit.xml.dist loaded from ",
1223-
fallbackXmlDist: true,
1206+
fallbackXmlDist: options.phpunitXmlIsDefault,
12241207
restrictDirectoriesToTests: !options.testRoot,
12251208
basePathExpression: "dirname($xml_path)",
12261209
uniqueReturnValues: false,
@@ -1233,10 +1216,7 @@ pg_stage_begin('discover_tests');
12331216
try {
12341217
$test_dir = $test_root;
12351218
if (!is_dir($test_dir)) {
1236-
pg_log('NO_TEST_FILES');
1237-
pg_log('NOTICE:tests directory not found at ' . $test_dir);
1238-
pg_stage_ok('discover_tests');
1239-
exit(0);
1219+
throw new RuntimeException('configured PHPUnit test root is not a readable directory: ' . $test_dir);
12401220
}
12411221
list($directories, $suffixes, $prefixes, $excludes, $configured_files) = wp_codebox_phpunit_parse_config(${JSON.stringify(options.phpunitXml)}, $test_dir);
12421222
$test_files = wp_codebox_phpunit_discover($directories, $suffixes, $prefixes, $excludes, $configured_files);
@@ -1258,10 +1238,9 @@ try {
12581238
$test_files = array($selected_abs);
12591239
}
12601240
pg_log('DISCOVERY: dirs=' . implode(',', $directories) . ' files=' . count($configured_files) . ' suffixes=' . implode(',', $suffixes) . ' prefixes=' . implode(',', $prefixes) . ' excludes=' . count($excludes) . ' found=' . count($test_files));
1261-
if (empty($test_files)) {
1241+
if (empty($test_files) && !$changed_test_scope) {
12621242
pg_log('NO_TEST_FILES');
1263-
pg_stage_ok('discover_tests');
1264-
exit(0);
1243+
throw new RuntimeException('PHPUnit discovery found no test files');
12651244
}
12661245
pg_stage_ok('discover_tests');
12671246
} catch (Throwable $e) {
@@ -1347,6 +1326,7 @@ $tests_dir = rtrim(${JSON.stringify(options.testsDir)}, '/');
13471326
$phpunit_xml = ${JSON.stringify(options.phpunitXml)};
13481327
$selected_test_file = ${JSON.stringify(options.selectedTestFile)};
13491328
$changed_test_files_raw = ${JSON.stringify(JSON.stringify(options.changedTestFiles))};
1329+
$changed_test_scope = ${JSON.stringify(options.changedTestFiles.length > 0)};
13501330
$autoload_file = ${JSON.stringify(options.autoloadFile)};
13511331
$wp_config_defines = json_decode(${JSON.stringify(JSON.stringify(options.wpConfigDefines))}, true);
13521332
$multisite = ${JSON.stringify(options.multisite)};
@@ -1482,11 +1462,8 @@ function core_pg_write_tests_config(string $core_root, array $extra_defines): st
14821462
${phpunitConfigDiscoveryPhp({
14831463
functionName: "core_pg_parse_phpunit_config",
14841464
logFunction: "core_pg_log",
1485-
missingConfigMessage: "NOTICE:phpunit config not readable at ",
1486-
parseFailureMessage: "NOTICE:phpunit config parse failed; using defaults",
1487-
includeParseFailureDetail: false,
14881465
loadedConfigMessage: "NOTICE:phpunit config loaded from ",
1489-
fallbackXmlDist: false,
1466+
fallbackXmlDist: options.phpunitXmlIsDefault,
14901467
restrictDirectoriesToTests: false,
14911468
basePathExpression: "dirname($xml_path)",
14921469
uniqueReturnValues: true,
@@ -1501,7 +1478,6 @@ ${phpunitChangedTestFilterPhp({
15011478
logFunction: "core_pg_log",
15021479
rootParameterName: "core_root",
15031480
testsPathFallback: false,
1504-
emptyWantedNotice: false,
15051481
})}
15061482
15071483
${phpunitArgsPhp("core_pg_phpunit_args", "core_pg_log")}
@@ -1584,10 +1560,9 @@ try {
15841560
$test_files = array($selected_abs);
15851561
}
15861562
core_pg_log('DISCOVERY: dirs=' . implode(',', $directories) . ' files=' . count($configured_files) . ' suffixes=' . implode(',', $suffixes) . ' prefixes=' . implode(',', $prefixes) . ' excludes=' . count($excludes) . ' found=' . count($test_files));
1587-
if (empty($test_files)) {
1563+
if (empty($test_files) && !$changed_test_scope) {
15881564
core_pg_log('NO_TEST_FILES');
1589-
core_pg_stage_ok('discover_tests');
1590-
exit(0);
1565+
throw new RuntimeException('PHPUnit discovery found no test files');
15911566
}
15921567
core_pg_stage_ok('discover_tests');
15931568
} catch (Throwable $e) {

‎packages/runtime-playground/src/wordpress-command-runners.ts‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,7 @@ export async function runPhpunitCommand({
907907
spec: ExecutionSpec
908908
}): Promise<string> {
909909
const args = spec.args ?? []
910+
const phpunitXmlArg = argValue(args, "phpunit-xml")
910911
const explicitCode = argValue(args, "code") || argValue(args, "code-file")
911912
const pluginSlug = argValue(args, "plugin-slug")?.trim() || ""
912913
const bootstrapMode = argValue(args, "bootstrap-mode")?.trim() || "managed"
@@ -923,9 +924,10 @@ export async function runPhpunitCommand({
923924
projectAutoloadFile: argValue(args, "project-autoload-file")?.trim() || "",
924925
testsDir: argValue(args, "tests-dir")?.trim() || "/wp-codebox-vendor/wp-phpunit/wp-phpunit",
925926
testRoot: argValue(args, "test-root")?.trim() || `/wordpress/wp-content/plugins/${pluginSlug}/tests`,
926-
phpunitXml: argValue(args, "phpunit-xml")?.trim() || `/wordpress/wp-content/plugins/${pluginSlug}/phpunit.xml.dist`,
927+
phpunitXml: phpunitXmlArg?.trim() || `/wordpress/wp-content/plugins/${pluginSlug}/phpunit.xml.dist`,
928+
phpunitXmlIsDefault: phpunitXmlArg === undefined || booleanArg(args, "phpunit-xml-default"),
927929
selectedTestFile: argValue(args, "test-file")?.trim() || "",
928-
changedTestFiles: jsonArrayArg(args, "changed-tests-json"),
930+
changedTestFiles: changedTestFilesArg(args),
929931
phpunitArgs: jsonArrayArg(args, "phpunit-args-json").filter((value): value is string => typeof value === "string"),
930932
env: jsonObjectArg(args, "env-json"),
931933
wpConfigDefines: jsonObjectArg(args, "wp-config-defines-json"),
@@ -974,6 +976,15 @@ function boundedProcessIdentity(value: string | undefined): string {
974976
return value
975977
}
976978

979+
function changedTestFilesArg(args: string[]): string[] {
980+
return jsonArrayArg(args, "changed-tests-json").map((value) => {
981+
if (typeof value !== "string" || value.trim() === "") {
982+
throw new Error("changed-tests-json must be a JSON array of non-empty test file paths")
983+
}
984+
return value.trim()
985+
})
986+
}
987+
977988
export async function runCorePhpunitCommand({
978989
artifactRoot,
979990
runPlaygroundCommand,
@@ -986,6 +997,7 @@ export async function runCorePhpunitCommand({
986997
spec: ExecutionSpec
987998
}): Promise<string> {
988999
const args = spec.args ?? []
1000+
const phpunitXmlArg = argValue(args, "phpunit-xml")
9891001
const explicitCode = argValue(args, "code") || argValue(args, "code-file")
9901002
// Write structured diagnostics to a sandbox-internal /tmp path rather than inside
9911003
// the (often read-only) core mount, so the result survives read-only mounts and a
@@ -994,9 +1006,10 @@ export async function runCorePhpunitCommand({
9941006
const code = explicitCode ? await phpCodeFromArgs(args, "wordpress.core-phpunit") : normalizePhpCode(corePhpunitRunCode({
9951007
coreRoot: argValue(args, "core-root")?.trim() || "/wordpress",
9961008
testsDir: argValue(args, "tests-dir")?.trim() || "/wordpress/tests/phpunit",
997-
phpunitXml: argValue(args, "phpunit-xml")?.trim() || "/wordpress/tests/phpunit/phpunit.xml.dist",
1009+
phpunitXml: phpunitXmlArg?.trim() || "/wordpress/tests/phpunit/phpunit.xml.dist",
1010+
phpunitXmlIsDefault: phpunitXmlArg === undefined || booleanArg(args, "phpunit-xml-default"),
9981011
selectedTestFile: argValue(args, "test-file")?.trim() || "",
999-
changedTestFiles: jsonArrayArg(args, "changed-tests-json"),
1012+
changedTestFiles: changedTestFilesArg(args),
10001013
autoloadFile: argValue(args, "autoload-file")?.trim() || "/wordpress/vendor/autoload.php",
10011014
wpConfigDefines: jsonObjectArg(args, "wp-config-defines-json"),
10021015
multisite: booleanArg(args, "multisite"),

0 commit comments

Comments
 (0)