On Mon, 1 Dec 2025 04:18:47 GMT, David Holmes <[email protected]> wrote:
>> Casper Norrbin has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> reworked check docker/resourcelimit functions
>
> test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java line 142:
>
>> 140: *
>> 141: * @return true if resource limits are supported in the current
>> configuration
>> 142: * @throws Exception
>
> I don't like this pattern of making a function seem like it is a query by
> declaring it returns a boolean when in reality it either returns true or else
> throws. These should just be void functions with names like
> `checkCanUseResourceLimits`, and we can elide the fake `if` conditions with
> unreachable `return` statements in the callers.
I agree that it could be made clearer.
I reworked both `canUseResourceLimits()` and `canTestDocker()` into `void`
functions that only throw. So
```c++
if (!DockerTestUtils.canTestDocker() ||
!DockerTestUtils.canUseResourceLimits()) {
return;
}
Instead becomes:
```c++
DockerTestUtils.checkCanTestDocker();
DockerTestUtils.checkCanUseResourceLimits();
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28557#discussion_r2576955639
