Re: RFR: 8283279: [Testbug] Improve TestGetSwapSpaceSize [v2]
On Wed, 23 Mar 2022 09:27:07 GMT, Severin Gehwolf wrote: >> Please review this container test improvement. The test in question only >> makes sense iff the total swap space size as reported by the container aware >> OperatingSystemMXBean is `0`. If that's not the case, then something else >> might be amiss, e.g. OperatingSystemMXBean reporting the host swap limits. >> In such a case a passing test tells us nothing. Certainly not if the >> fix from [JDK-8242480](https://bugs.openjdk.java.net/browse/JDK-8242480) is >> present or not. >> >> Testing: >> - [x] Manual with and without the code fix of JDK-8242480. Still passes with >> the fix, and fails without. Tested the test on cgroups v1 and cgroups v2. > > Severin Gehwolf has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Incorporate review feedback > - Merge branch 'master' into test_getswapspace_improvement_8283279 > - 8283279: [Testbug] Improve TestGetSwapSpaceSize Thanks for the review! - PR: https://git.openjdk.java.net/jdk/pull/7854
Re: RFR: 8283279: [Testbug] Improve TestGetSwapSpaceSize [v2]
On Wed, 23 Mar 2022 09:27:07 GMT, Severin Gehwolf wrote: >> Please review this container test improvement. The test in question only >> makes sense iff the total swap space size as reported by the container aware >> OperatingSystemMXBean is `0`. If that's not the case, then something else >> might be amiss, e.g. OperatingSystemMXBean reporting the host swap limits. >> In such a case a passing test tells us nothing. Certainly not if the >> fix from [JDK-8242480](https://bugs.openjdk.java.net/browse/JDK-8242480) is >> present or not. >> >> Testing: >> - [x] Manual with and without the code fix of JDK-8242480. Still passes with >> the fix, and fails without. Tested the test on cgroups v1 and cgroups v2. > > Severin Gehwolf has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Incorporate review feedback > - Merge branch 'master' into test_getswapspace_improvement_8283279 > - 8283279: [Testbug] Improve TestGetSwapSpaceSize LGTM Thanks for the update. - Marked as reviewed by jiefu (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7854
Re: RFR: 8283279: [Testbug] Improve TestGetSwapSpaceSize [v2]
On Wed, 23 Mar 2022 03:06:00 GMT, Jie Fu wrote:
>> Severin Gehwolf has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains three additional
>> commits since the last revision:
>>
>> - Incorporate review feedback
>> - Merge branch 'master' into test_getswapspace_improvement_8283279
>> - 8283279: [Testbug] Improve TestGetSwapSpaceSize
>
> Please also update the copyright year.
> Thanks.
Thanks for the review, @DamonFool! Updated the patch.
> test/jdk/jdk/internal/platform/docker/GetFreeSwapSpaceSize.java line 28:
>
>> 26:
>> 27: // Usage:
>> 28: // GetFreeSwapSpaceSize
>>
>
> I would suggest
>
> // GetFreeSwapSpaceSize
>
Thanks, fixed.
> test/jdk/jdk/internal/platform/docker/GetFreeSwapSpaceSize.java line 32:
>
>> 30: public static void main(String[] args) {
>> 31: if (args.length != 4) {
>> 32: throw new RuntimeException("Unexpected arguments. Expected
>> 2, got " + args.length);
>
> Shouldn't be `Expected 4` ?
Yes. Fixed now.
-
PR: https://git.openjdk.java.net/jdk/pull/7854
Re: RFR: 8283279: [Testbug] Improve TestGetSwapSpaceSize [v2]
> Please review this container test improvement. The test in question only > makes sense iff the total swap space size as reported by the container aware > OperatingSystemMXBean is `0`. If that's not the case, then something else > might be amiss, e.g. OperatingSystemMXBean reporting the host swap limits. In > such a case a passing test tells us nothing. Certainly not if the > fix from [JDK-8242480](https://bugs.openjdk.java.net/browse/JDK-8242480) is > present or not. > > Testing: > - [x] Manual with and without the code fix of JDK-8242480. Still passes with > the fix, and fails without. Tested the test on cgroups v1 and cgroups v2. Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Incorporate review feedback - Merge branch 'master' into test_getswapspace_improvement_8283279 - 8283279: [Testbug] Improve TestGetSwapSpaceSize - Changes: - all: https://git.openjdk.java.net/jdk/pull/7854/files - new: https://git.openjdk.java.net/jdk/pull/7854/files/1bbb550a..891c2366 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7854=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7854=00-01 Stats: 2978 lines in 116 files changed: 1986 ins; 629 del; 363 mod Patch: https://git.openjdk.java.net/jdk/pull/7854.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7854/head:pull/7854 PR: https://git.openjdk.java.net/jdk/pull/7854
