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

[Emanuel Peter]

On Tue, 7 Oct 2025 11:03:13 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> The test ` testlibrary_tests/ir_framework/tests/TestScenariosCrossProduct.java` intermittently timed out in our CI. On my local machine, I measured ~80s which is quite long given that we actually only want to test that Cartesian product for scenarios work.
> 
> #### Reduce Execution Time by not Executing the Scenarios
> I had a closer look at the test to try to cut the execution time down. Currently, we are executing many IR framework runs with different Cartesian products for scenarios. Afterwards, we compare the output of IR matching to check if the computation of the Cartesian products were correct. However, since we are only interested in verifying that the Cartesian product computation for scenarios works (i.e. `addCrossProductScenarios()`), we could skip the actual execution of the scenarios itself - we can trust that the IR framework is already tested well enough.
> 
> To achieve that, we can use reflection to get the added scenarios to the IR framework (I don't want to add a public accessor because a user should not need access to them) and then fetch the corresponding scenario flags and compare against our expectation. That's what I propose with this change.
> 
> #### Changes
> - Verification without actually running scenarios.
> - Added a test passing 3 sets to `addCrossProductScenarios()` which was missing before.
> - Improved `addScenarios()` where we added a scenario to the list even though it already existed. That normally does not matter because we are throwing a `TestFormatException` anyway afterwards. But it messes with the test: We are adding the duplicated scenario and then read it again in the verification part of the test.
> - Refactored the test a little more.
> - Refactored some small things in `addCrossProductScenarios()` while looking at it.
> - Added a sentence about passing a single set to `addCrossProductScenarios()` which was not evidently clear what is happening when looking at the method comment.
> 
> #### Execution Time Comparison
> Measured on my local machine:
> - Mainline: ~80s
> - With patch: ~2-3s
> 
> Thanks,
> Christian

Generally looks ok. But I'm still a little sad that we don't even get to test a single case end-to-end now. Can we not do at least a 2x2 case?

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`.

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

PR Review: https://git.openjdk.org/jdk/pull/27672#pullrequestreview-3331828102
PR Review Comment: https://git.openjdk.org/jdk/pull/27672#discussion_r2426490918