On Mon, 10 Mar 2025 22:49:06 GMT, Anthony Scarpino <ascarp...@openjdk.org>
wrote:
>> Volodymyr Paprotski has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> more comment improvements
>
> test/jdk/com/sun/security/util/math/intpoly/MontgomeryPolynomialFuzzTest.java
> line 30:
>
>> 28: import sun.security.util.math.intpoly.*;
>> 29:
>> 30: /*
>
> It is strange that there are two copies of the `@test` block. Can you please
> remove one of them, unless you are seeing a difference that I do not
-XX:+/-UseIntPolyIntrinsics (test Java vs BigInt and intrinsic vs BigInt)
Though I think I did this before I knew much about junit.. I think I can just
have two @run commands (to make it clearer)? Will give that a try
> test/jdk/com/sun/security/util/math/intpoly/MontgomeryPolynomialFuzzTest.java
> line 123:
>
>> 121: }
>> 122:
>> 123: if (rnd.nextBoolean()) {
>
> Why is this done randomly? Wouldn't we want to check these situations every
> time?
I was mostly attempting to test 'random paths' through the code, and this was a
way to pseudo-randomly accomplish that. (i.e. a product of a difference, a
product of a product.. and so on..)
Since this is looping, we got 50% chance of getting both, without me having to
write/think-through all the many permutations of what input/outputs to each
operations can be.
(Extend the loop count to run for several hours during development.. and it
does wonders to testing corner cases. Have been following this 'template' in
most my PRs)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23719#discussion_r1988136095
PR Review Comment: https://git.openjdk.org/jdk/pull/23719#discussion_r1988134465
