RFR: CODETOOLS-7903748 - jcstress: Test list should honor concurrency settings [v2]

[PM]

On Tue, 2 Jul 2024 07:38:57 GMT, Jiří Vaněk <jvanek at openjdk.org> wrote:

>> This is extracting  List<TestConfig> configs =prepareRunProgram(classes, tests);
>> with all he HW/switches setup to separated method and reusing it in `-l` mode
>> 
>> I'm aware of triplicated removal of -agentlib, and will clean it up as
>> CODETOOLS-7903756 will progress.
>> 
>> -l now honours also verbose mode, in which it prints not just matching
>> tests but all really run tests, and thus enabling much more easy
>> determining of all tests
>> 
>> help adjusted.
>> 
>> Maybe I'm missing plain quick initial all tests metod now, but with 
>> artificial -c MAX it seems doing exactly that
>
> Jiří Vaněk has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
> 
>   CODETOOLS-7903748 - jcstress: Test list should honor concurrency settings
>   
>   This is extracting  List<TestConfig> configs =prepareRunProgram(classes, tests);
>   with all he HW/switches setup to separated method and reusing it in `-l` mode
>   
>   -l now honours also verbose mode, in which it prints not just matching
>   tests but all really run tests, and thus enabling much more easy
>   determining of all tests
>   
>   help adjusted.
>   
>   Maybe I'm missing plain quick initial all tests metod now, but with
>   artificial -c MAX it seems doing exactly that

Changes requested by matcdac at github.com (no known OpenJDK username).

jcstress-core/src/main/java/org/openjdk/jcstress/Main.java line 74:

> 72:             for (String test : testsToPrint) {
> 73:                 System.out.println(test);
> 74:             }

maybe move this code to a separate method, for better readability and maintainability

jcstress-core/src/main/java/org/openjdk/jcstress/Options.java line 85:

> 83: 
> 84:         OptionSpec<Boolean> list = parser.accepts("l", "List the available tests matching the requested settings, " +
> 85:                         "after all filters (like CPU count) are applied. In verbose mode it prints all real combinations which will run.")

"List the available tests matching the requested settings, " +
                        "after all filters (like CPU count) are applied. In verbose mode it prints all real combinations which will run."

this might have been done for better readability, but remove the string concatenation, define it as a part of single string, possibly at class level by making it static final

jcstress-core/src/main/java/org/openjdk/jcstress/infra/runners/TestConfig.java line 39:

> 37: import java.io.Serializable;
> 38: import java.util.List;
> 39: import java.util.stream.Collectors;

import is not required

jcstress-core/src/main/java/org/openjdk/jcstress/infra/runners/TestConfig.java line 265:

> 263:                 ", strideCount=" + strideCount +
> 264:                 ", cpuMap=" + cpuMap +
> 265:                 ", " + jvmArgs + "}";

kindly modify it to return the json based structure, as followed in rest of the class, equals symbol does not appear in json

-------------

PR Review: https://git.openjdk.org/jcstress/pull/149#pullrequestreview-2154420348
PR Review Comment: https://git.openjdk.org/jcstress/pull/149#discussion_r1662918818
PR Review Comment: https://git.openjdk.org/jcstress/pull/149#discussion_r1662897106
PR Review Comment: https://git.openjdk.org/jcstress/pull/149#discussion_r1662892838
PR Review Comment: https://git.openjdk.org/jcstress/pull/149#discussion_r1662891388