Thanks, looks good!

/Erik

On 2019-05-23 19:07, Igor Ignatyev wrote:
Hi Erik,

thanks for your review. I've updated CompileGtest.gmk as you advised[1], which 
gives [2].

-- Igor

[1]
diff -r 3443e083223d make/hotspot/lib/CompileGtest.gmk
--- a/make/hotspot/lib/CompileGtest.gmk Thu May 23 19:03:32 2019 -0700
+++ b/make/hotspot/lib/CompileGtest.gmk Thu May 23 19:04:09 2019 -0700
@@ -64,8 +64,9 @@
      EXCLUDES := $(JVM_EXCLUDES), \
      EXCLUDE_FILES := gtestLauncher.cpp, \
      EXCLUDE_PATTERNS := $(JVM_EXCLUDE_PATTERNS), \
-    EXTRA_FILES := $(GTEST_FRAMEWORK_SRC)/googletest/src/gtest-all.cc \
-                   $(GTEST_FRAMEWORK_SRC)/googlemock/src/gmock-all.cc, \
+    EXTRA_FILES := \
+        $(GTEST_FRAMEWORK_SRC)/googletest/src/gtest-all.cc \
+        $(GTEST_FRAMEWORK_SRC)/googlemock/src/gmock-all.cc, \
      EXTRA_OBJECT_FILES := $(filter-out %/operator_new$(OBJ_SUFFIX), \
          $(BUILD_LIBJVM_ALL_OBJS)), \
      CFLAGS := $(JVM_CFLAGS) \
[2] http://cr.openjdk.java.net/~iignatyev/8222414/webrev.1-2/

On May 23, 2019, at 6:57 AM, Erik Joelsson <erik.joels...@oracle.com> wrote:

Hello Igor,

Build changes look ok, with a (very) minor style suggestion [1] (point 18). We 
try to avoid padding continuation line breaks, so I would appreciate if 
CompileGtest.gmk:68 could be indented just 4 extra spaces compared to line 67. 
If you want the file names to line up, it's acceptable to break after the := on 
67.

[1] http://openjdk.java.net/groups/build/doc/code-conventions.html

/Erik

On 2019-05-22 22:20, Igor Ignatyev wrote:
http://cr.openjdk.java.net/~iignatyev//8222414/webrev.01
21638 lines changed: 21628 ins; 0 del; 10 mod;
Hi all,

could you please review this patch which makes mocking framework from google 
test available for hotspot tests?

JBS: https://bugs.openjdk.java.net/browse/JDK-8222414
testing: tier1
webrevs:
  - http://cr.openjdk.java.net/~iignatyev//8222414/webrev.00
21605 lines changed: 21605 ins; 0 del; 0 mod;
* moves gtest from test/fmw/gtest to test/fmw/gtest/googletest;
* adds only required (w/o build-aux, docs, make, msvc, scripts, tests 
directories and make related files) source of gmock to 
test/fmw/gtest/googlemock;

- http://cr.openjdk.java.net/~iignatyev//8222414/webrev.0-1
33 lines changed: 23 ins; 0 del; 10 mod;
* makes necessary changes in makefiles
* calls gmock framework init function
* works around the conflict b/w :testing::internal::Log function defined by 
gmock and Log macro defined by hotspot

- http://cr.openjdk.java.net/~iignatyev//8222414/webrev.01
21638 lines changed: 21628 ins; 0 del; 10 mod;
all the changes together


thanks,
-- Igor

Reply via email to