On Sat, 21 Jun 2025 23:32:04 GMT, Koushik Muthukrishnan Thirupattur
<[email protected]> wrote:
>> Mikhail Yankelevich has updated the pull request with a new target base due
>> to a merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into JDK-8357470
>> - JDK-8357470: src/java.base/share/classes/sun/security/util/Debug.java
>> implement the test for args.toLowerCase
>>
>> * added an automated mixed case option
>> * using multithreading now
>> * added logs for simpler debug
>> * added missing -Djava.security.auth.debug coverage
>
> test/jdk/sun/security/util/Debug/DebugOptions.java line 174:
>
>> 172:
>> 173: patternMatches.forEach(params -> {
>> 174: testsCallables.add(() -> {
>
> Can we consider adding negative tests with invalid/malformed parameters like
> null, empty strings, unknown keywords, etc?
This is not in the scope of this change, however this will be addressed in
[JDK-8360373](https://bugs.openjdk.org/browse/JDK-8360373)
> test/jdk/sun/security/util/Debug/DebugOptions.java line 207:
>
>> 205: return null;
>> 206: });
>> 207:
>
> Can we add a test to explicitly test something like this for
> `testParameter("-Djava.security.debug=", "all", EXPECTED_ALL_REGEX, "not
> expected")` and `testMixedCaseParameter("-Djava.security.debug=", "AlL",
> EXPECTED_ALL_REGEX, "not expected")` ?
As we discussed offline, if we only test a few options hardcoded like this we
risk missing a potential bug where one option may not support mixed and upper
case
> test/jdk/sun/security/util/Debug/DebugOptions.java line 214:
>
>> 212: final List<Future<Void>> res =
>> executorService.invokeAll(testsCallables);
>> 213: for (final Future<Void> future : res) {
>> 214: future.get();
>
> future.get() could throw an unchecked exception. Can we consider logging
> which test failed and why?
It is logged in the `test*Parameter` methods when the action is executed. It
would be saved to a separate log file by jtreg, one per thread
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25391#discussion_r2163565812
PR Review Comment: https://git.openjdk.org/jdk/pull/25391#discussion_r2163566241
PR Review Comment: https://git.openjdk.org/jdk/pull/25391#discussion_r2163566431
