On Tue, 22 Sep 2020 21:13:53 GMT, Igor Ignatyev <iignat...@openjdk.org> wrote:
> the patch
> - removes `PropertyResolvingWrapper` from `vmTestbase/nsk/aod` tests
> - updates `AODRunnerArgParser` to remove surrounding `"` symbols from
> `javaOpts` option values
> - updates vmTestbase/nsk/aod test descriptions to have `"` in `javaOpts`
> values (as opposed of around `-javaOpts=$value`
> as it's now)
> - mechanically reformats/cleans up the tests (whitespaces, imports, etc)
>
> testing:
> * [x] vmTestbase/nsk/aod on {macosx,windows,linux}-x64
> * [x] vmTestbase/nsk/aod on macosx-x64 w/ `VM_OPTIONS`, `JAVA_OPTIONS` being
> empty and having value
Looks good, modulo minor comments below.
test/hotspot/jtreg/vmTestbase/nsk/aod/VirtualMachine/VirtualMachine03/VirtualMachine03.java
line 78:
> 76: } finally {
> 77: vm2.detach();
> 78: }
Is nesting needed here because `vm1`/`vm2` can be `null` at `finally`? Then it
is cleaner IMO to just do:
} finally {
if (vm1 != null) vm1.detach();
if (vm2 != null) vm2.detach();
}
test/hotspot/jtreg/vmTestbase/nsk/aod/VirtualMachineDescriptor/VirtualMachineDescriptor01/VirtualMachineDescriptor01.java
line 112:
> 110:
> 111: TestUtils.assertEquals(targetVMDesc.hashCode(),
> targetVMDesc2.hashCode(),
> 112: "VirtualMachineDescriptor.hashCode() returns
> different values for '" + targetVMDesc + "' and
> '" + targetVMDesc2 + "'");
Does `TestUtils.assertEquals` print out actual/expected value as well? If not,
dropping `hashCode` printout here loses
debugging data.
-------------
Marked as reviewed by shade (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/311
