tests: give each test its own spx.data_dir - #343
Conversation
|
Hi @andypost, could you retarget this against the v0.5 branch rather than master? |
|
@andypost also rebasing onto v0.5 may surface conflicts, and you might also need to patch additional new tests that exist there. |
|
doing it now, thank you! |
|
@NoiseByNorthwest FYI I'm using this patch to build for Alpinelinux but looks better to wait for new API numbers according to https://wiki.php.net/todo/php86 |
|
@NoiseByNorthwest btw used to ask claude to validate 0.5 branch and it reported only
|
|
Regarding PHP 8.6 support: I'll add it on v0.5 as soon as the image is available on the GitHub Actions side. From experience the cost is very low, so there's no need to anticipate it more than that. |
|
added 8.6 to matrix according to https://github.com/shivammathur/setup-php/blob/main/README.md#tada-php-support but looks pipelines need manual action |
The CI didn't trigger because the pull_request workflow only matched a master base, and fork PRs can't fall back to the push trigger. Fixed in #344. Once it's merged, a rebase here will pick up the CI run. |
Several tests share a report directory:
- spx_auto_start_005, spx_auto_start_007, spx_full_report_custom_metadata
and spx_full_report_data write "full" reports into the default data dir
/tmp/spx (src/php_spx.c: "spx.data_dir", "/tmp/spx"), and the last two
read them back;
- spx_ini_params_not_overridden and spx_ini_params_profiling_enabled
rm -rf that same /tmp/spx in their --CLEAN-- section;
- the five spx_ui_report_delete_* tests all point spx.data_dir at
{PWD}/tmp_data_dir, and each of them rm -rf's it both on entry and on
exit.
That was harmless while run-tests.php was serial. Since php-src 1d2ea5ce22c
("Run tests in parallel by default", GH-22939, first released in 8.6.0beta1)
it spawns min(nproc, 10) workers, so those tests race and a sibling wipes
the reports a test is still reading or writing.
Measured on PHP 8.6.0beta1, 8 cores, run-tests.php -j10, 20 runs of the
suite (83 tests, 16 skipped, 67 run):
before 18/20 runs had failures
spx_full_report_custom_metadata 18/20
spx_ui_report_delete_all 4/20
spx_ui_report_delete_one 2/20
spx_ui_report_delete_nonexistent 1/20
spx_full_report_data 1/20
after 0/20, plus 0/30 in a second batch
Give each of them its own data dir with --INI--, the way
spx_ui_report_access.phpt already does, and add the missing --CLEAN--
sections: four of them had none, so they were leaving reports behind after
every run. spx_ini_params_no_unnecessary_access_check enables the same HTTP
profiling as its two siblings and gets the same treatment, so the three stay
consistent.
Note --CLEAN-- is run with the original INI settings rather than the test's
--INI-- (run-tests.php passes $orig_ini_settings_args), so ini_get() yields
the default there and the cleanup paths have to be spelled out with __DIR__.
The read-only fixture directory tests/data_dir, shared by
spx_ui_report_access and spx_ui_report_metadata_access, is left as it is:
no test writes to it.
|
rebased and pushed but GH today is slow |
e47f0a5 to
3dee0a4
Compare
|
removed last commit as API version will change |
|
@andypost since the API-number check is gone but 8.6 stays in the matrix, the branch is red. So this PR is really the PHP 8.6 support PR now. Let's keep it that way and wait for the definitive API numbers to go green. Could you rename the PR to match that scope and update the README in the meantime? |
2 participants
Six tests write profiling reports into the shared default data dir /tmp/spx (src/php_spx.c: "spx.data_dir", "/tmp/spx"), and two of them wipe that directory in their --CLEAN-- section, while spx_custom_metadata reads its own reports back. Since php/php-src@1d2ea5ce22c ("Run tests in parallel by default", GH-22939, first released in 8.6.0beta1) run-tests.php spawns min(nproc, 10) workers, so those tests race and spx_custom_metadata fails on reports a sibling has just deleted.
Point each of them at its own data dir with --INI--, the way spx_ui_report_access.phpt already does, and add the missing --CLEAN-- sections. Three of the six had none, so they were leaving reports in /tmp/spx after every run.
Note --CLEAN-- runs without the test's --INI--, so the cleanup paths are spelled out with DIR.