Hi Igor, Looks good to me, I tried rewriting parts of an existing test to use gmock instead of a handcrafted mock, and things worked as expected!
Best regards, Robin > On 24 May 2019, at 23:33, Igor Ignatyev <igor.ignat...@oracle.com> wrote: > > @Erik, Thanks! > > @hotspot (looking at Robin), can I get another review? > > -- Igor > >> On May 24, 2019, at 8:28 AM, Erik Joelsson <erik.joels...@oracle.com> wrote: >> >> 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 >
