On Fri, 1 Nov 2024 16:35:34 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> Weijun Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> acvp.test.alg system property
>
> test/jdk/sun/security/provider/acvp/Launcher.java line 50:
>
>> 48:
>> 49: public static void main(String[] args) throws Exception {
>> 50:
>
> Use of properties to configure the test is a bit puzzling since the test is a
> main(String[] args) program.
> It would more convenient to run it from the command line (where it would be
> run manually) to use normal arguments.
>
> It would be better to describe the properties, if retained, as "test
> properties", not system properties since they only apply to the test and not
> the implementation. A "test." prefix on the property names would also make it
> clearer.
>
> Including sample `* @run main/manual ...` lines might show more easily how
> they would be used.
We will put test data files into the `data` directory in future commits, and
then the test can run as an automatic test without any test properties. The
test can also be launched directly with the `jtreg` command while development
new algorithms, and the test properties would be useful. I didn't meant to make
it a manual test.
I'll rename the property names. Thanks.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1826157340
