LGTM — Igor
> On Jun 23, 2020, at 11:13 AM, Erik Joelsson <erik.joels...@oracle.com> wrote: > > That looks much better! :) > > /Erik > >> On 2020-06-23 09:53, Magnus Ihse Bursie wrote: >>> On 2020-06-23 17:05, Erik Joelsson wrote: >>> Looks good, but that was the worst way of posting a patch I've seen to date. >> The mail you quoted looked awful, yes! :-( I tried a new way of formatting >> the mail so the patch and log should be fixed space. Apparently it failed >> horribly. >> >> Also, I realized that since the fix is technically in hotspot code, I should >> have a hotspot review on this as well. cc:ing them. >> >> Here is the original mail again, sans the formatting: >> ------- >> >> If you run make test TEST=jtreg:gtest/GTestWrapper.java but had not built >> using the gtest framework, you would just get a result like this: >> >> STDERR: >> java.lang.Error: TESTBUG: the library has not been found in >> /home/shade/trunks/jdk-jdk/build/linux-x86_64-server-fastdebug/images/test/hotspot/jtreg/native >> at GTestWrapper.main(GTestWrapper.java:60) >> at >> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native >> Method) >> at >> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64) >> at >> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) >> at java.base/java.lang.reflect.Method.invoke(Method.java:564) >> at >> com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298) >> at java.base/java.lang.Thread.run(Thread.java:832) >> >> JavaTest Message: Test threw exception: java.lang.Error >> JavaTest Message: shutting down test >> >> This is not very helpful in analyzing the problem. I have added a helpful >> suggestion to the exception. >> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8247573 >> Patch inline: >> >> diff --git a/test/hotspot/jtreg/gtest/GTestWrapper.java >> b/test/hotspot/jtreg/gtest/GTestWrapper.java >> --- a/test/hotspot/jtreg/gtest/GTestWrapper.java >> +++ b/test/hotspot/jtreg/gtest/GTestWrapper.java >> @@ -1,5 +1,5 @@ >> /* >> - * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights >> reserved. >> + * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights >> reserved. >> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. >> * >> * This code is free software; you can redistribute it and/or modify it >> @@ -57,7 +57,7 @@ >> .resolve(jvmVariantDir); >> } >> if (!path.toFile().exists()) { >> - throw new Error("TESTBUG: the library has not been found in " + >> nativePath); >> + throw new Error("TESTBUG: the library has not been found in " + >> nativePath + ". Did you forget to use --with-gtest to configure?"); >> } >> >> Path execPath = path.resolve("gtestLauncher" + >> (Platform.isWindows() ? ".exe" : "")); >> >> /Magnus >>> >>> /Erik >>> >>> On 2020-06-23 07:37, Magnus Ihse Bursie wrote: >>>> If you run |make test TEST=jtreg:gtest/GTestWrapper.java| but had not >>>> built using the gtest framework, you would just get a result like this: >>>> >>>> |STDERR: java.lang.Error: TESTBUG: the library has not been found in >>>> /home/shade/trunks/jdk-jdk/build/linux-x86_64-server-fastdebug/images/test/hotspot/jtreg/native >>>> at GTestWrapper.main(GTestWrapper.java:60) at >>>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native >>>> Method) at >>>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64) >>>> at >>>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) >>>> at java.base/java.lang.reflect.Method.invoke(Method.java:564) at >>>> com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298) >>>> at java.base/java.lang.Thread.run(Thread.java:832) JavaTest Message: Test >>>> threw exception: java.lang.Error JavaTest Message: shutting down test | >>>> >>>> This is not very helpful in analyzing the problem. I have added a helpful >>>> suggestion to the exception. >>>> >>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8247573 >>>> Patch inline: >>>> >>>> |diff --git a/test/hotspot/jtreg/gtest/GTestWrapper.java >>>> b/test/hotspot/jtreg/gtest/GTestWrapper.java --- >>>> a/test/hotspot/jtreg/gtest/GTestWrapper.java +++ >>>> b/test/hotspot/jtreg/gtest/GTestWrapper.java @@ -1,5 +1,5 @@ /* - * >>>> Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights >>>> reserved. + * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All >>>> rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE >>>> HEADER. * * This code is free software; you can redistribute it and/or >>>> modify it @@ -57,7 +57,7 @@ .resolve(jvmVariantDir); } if >>>> (!path.toFile().exists()) { - throw new Error("TESTBUG: the library has >>>> not been found in " + nativePath); + throw new Error("TESTBUG: the library >>>> has not been found in " + nativePath + ". Did you forget to use >>>> --with-gtest to configure?"); } Path execPath = >>>> path.resolve("gtestLauncher" + (Platform.isWindows() ? ".exe" : "")); | >>>> >>>> /Magnus >>>> >>>> >>
