Build/Test Tools: Improve performance of bcrypt related unit tests in the 6.8 branch - #13018
Build/Test Tools: Improve performance of bcrypt related unit tests in the 6.8 branch#13018lucatume wants to merge 2 commits into
Conversation
…crypt hashing. Merges [60253] to the 6.8 branch. User creation is more expensive on 6.8 since the switch to bcrypt hashing, and users created in wpSetUpBeforeClass fixtures hash at PHP's full default cost. Moving per-test user creation into shared fixtures cuts how many of those hashes the suite performs. Props peterwilsoncc, johnbillion. See #63026.
…peed up tests. Merges [60298] to the 6.8 branch. 6.8 hashes test user passwords at PHP's default bcrypt cost, which PHP 8.4 raised from 10 to 12. PHPUnit jobs on PHP 8.4 run to a median of 1118 seconds against 820 for PHP 7.2, 7.4 and 8.0, and hit the workflow timeout often enough to trigger an automatic rerun. The lowest allowable cost in PHP is 4, so 5 is used, which leaves test_wp_check_password_supports_hash_with_reduced_bcrypt_cost() room to reduce the cost by one. Props johnbillion. See #63026.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
1 participant
6.8 branched on 2025-03-25, before [60253] and [60298] landed on trunk under #63026, so it's the only branch that ships bcrypt password hashing without the accompanying test suite cost reductions. 6.7 and earlier predate bcrypt; 6.9 and later inherited both changesets.
Every PHP version pays for it. Median job duration, run 31596990187 on 6.8 against run 31600298705 on 6.9, an hour apart on matrices of the same size:
PHP 8.4 is worse again, because it raised the default bcrypt cost from 10 to 12 while [60298] pins the test cost to 5. On
PHP 8.4 / MySQL 8.4 example.org:8889, the one job name common to both branches, theRun PHPUnit testsstep takes 999s on 6.8 and 701s on 6.9. That job is left out of the table above because 6.9 runs PHP 8.4 only on the twoexample.org:8889variants, so a median would compare 26 jobs against 2.That tail is what breaks the run. Three jobs exceeded
timeout-minutesand were cancelled, which dispatchedfailed-workflow.ymland reran the run, for 51m18s of wall clock against 18m32s for 6.9. p90 was 1134s against 829s.This merges both trunk changes so 6.8's suite matches 6.9's:
wpSetUpBeforeClasshash at PHP's full default cost, since the cost filter is registered inset_up().Both applied without conflict.
Two things for review:
reduce_hash_cost()now produces cost 4, PHP's minimum, which is why the pinned cost is 5 and not 4.[60253] touches 28 test files that have had 16 months to drift on this branch. The merge was clean, but that only means the text lined up. The CI run on this PR is the real check, and breakage would most likely show as order dependence in the customize and meta suites.
Trac ticket: https://core.trac.wordpress.org/ticket/65861
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.