Unify structure for ext/random's randomizer tests #9410
Conversation
68703a3 to
95561c7
Compare
This fixes an incompatibility when wrapping native 32-bit engines with a userland engine. The latter always used the 64-bit range function which then used two 32-bit numbers from the underlying engine to fill the 64-bit range, whereas the native implementation used only one. Now the selection of the range variant only depends on the requested range. A 32-bit range uses the 32-bit variant (even for 64-bit engines), whereas a larger range uses the 64-bit variant. This was found in php#9410 (comment)
This fixes an incompatibility when wrapping native 32-bit engines with a userland engine. The latter always used the 64-bit range function which then used two 32-bit numbers from the underlying engine to fill the 64-bit range, whereas the native implementation used only one. Now the selection of the range variant only depends on the requested range. A 32-bit range uses the 32-bit variant (even for 64-bit engines), whereas a larger range uses the 64-bit variant. This was found in php#9410 (comment)
aca8f15 to
8304060
Compare
) This fixes an incompatibility when wrapping native 32-bit engines with a userland engine. The latter always used the 64-bit range function which then used two 32-bit numbers from the underlying engine to fill the 64-bit range, whereas the native implementation used only one. Now the selection of the range variant only depends on the requested range. A 32-bit range uses the 32-bit variant (even for 64-bit engines), whereas a larger range uses the 64-bit variant. This was found in #9410 (comment)
8304060 to
f0765a0
Compare
This makes adjustments that were missed in 2d6a883.
f0765a0 to
78dbe19
Compare
|
@zeriyoshi This is now ready. I did a full sweeping change across all the randomizer tests to unify them and make them easier to understand and maintain (especially if new methods are added in the future). I recommend checking out the branch locally and not attempting to use the GitHub web interface to review. |
|
@zeriyoshi Friendly ping. Are you able to look into this before the RC 1 tomorrow? If there's something you're unhappy with, then that's fine, just comment and I'll fix. |
|
@saundefined @ramsey @adoy This PR cleans up and reorganizes the tests for the new-in-PHP-8.2 ext/random to make them easier to maintain in the long run and to make them more reliable. It does not introduce any functional changes whatsoever. I would expect that this is fine to merge into PHP-8.2 as-is, because merging into master only will create a chaos for 8.2 → master merges, but I'd like to have your confirmation to be sure. |
|
No problem for 8.2 you can go ahead with it. Thanks |
|
@adoy Thanks for the confirmation, I'll handle the merge tonight. |
* PHP-8.2: Unify structure for ext/random's randomizer tests (#9410)
This is a work-in-progress follow-up for #9321:
This PR makes a sweeping change across all tests in
ext/random/tests/03_randomizerto achieve a uniform appearance:useto reduce the visual noise within the test for repeated references to specific classes.echo $e->getMessage(), PHP_EOL;with a comma to dump error messages.var_dump()to output randomizer values.->getInt(0, $i)for generic verification purposes, unless a specific method is tested.basic.phpt).$i), instead of testing multiple times with the same parameters to ensure the methods work correctly with different boundaries.