RFR: 8369232: testlibrary_tests/ir_framework/tests/TestScenariosCrossProduct.java timed out [v2]

[Emanuel Peter]

On Tue, 14 Oct 2025 08:54:55 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestScenariosCrossProduct.java line 251:
>> 
>>> 249:                         continue outer;
>>> 250:                     }
>>> 251:                 }
>> 
>> You could probably do this with an `stream().anyMatch()`:
>> Suggestion:
>> 
>>                 if (scenarios.stream().anyMatch(s -> s.equals(expectedScenarioFlags))) {
>>                     continue;
>>                 }
>> 
>> You may even be able to further simplify the lambda in there, to get rid of the `s`.
>
> Thanks for the suggestion. `s` in your code is a `Scenario` while `expectedScenarioFlags` is a `Set<String>`. But maybe you meant, to first `map()` the scenarios to `getFlags()` and then do the comparison. That would work but `getFlags()` returns a `List`. We first need to convert it to a `Set` in order to call `equals()` which ignores the order. It could look like this:
> 
> if (scenariosFromCrossProduct.stream()
>         .map(Scenario::getFlags)
>         .map(HashSet::new)
>         .anyMatch(flags -> flags.equals(expectedScenarioFlags))) {
>     continue;
> }
>  ```
> I'm fine with both.

Hmm yes, I suppose I overlooked some details. I'm fine with both as well. Now my solution looks not as simple as I had hoped it would.

Instead of making it a `HashSet`, I would use `Set::copyOf`. Actually, you should do that anyway. It gives you an unmodifyable set, and allows the JDK to pick what it prefers for that. But this is really a nit upon a nit 😆

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27672#discussion_r2428461910