RFR: 8240222: [TESTBUG] gtest/jfr/test_networkUtilization.cpp failed when the number of tests is greater than or equal to 2

[Markus Gronlund]

The interface_id assigns a unique primary key for each interface.

To support GTEST=REPEAT, I think the easiest way is to add a corresponding counter in the gtest itself that mirror the production code:

That is, as an example, instead of the "add_interface("eth0", 1), that hardcodes the traceid = 1, have it take a value from the corresponding counter, i.e. "add_interface("eth0", next_id++)";

Thanks
Markus



-----Original Message-----
From: Jia Huang <huangjia at loongson.cn> 
Sent: den 9 mars 2020 10:57
To: hotspot-dev at openjdk.java.net; hotspot-jfr-dev at openjdk.java.net
Subject: Re: RFR: 8240222: [TESTBUG] gtest/jfr/test_networkUtilization.cpp failed when the number of tests is greater than or equal to 2

In addition, the static local variable "id" is assigned in the function of "new_entry", that is in share/jfr/periodic/jfrNetworkUtilization.cpp.

---------------------------------------------------------------
share/jfr/periodic/jfrNetworkUtilization.cpp

static InterfaceEntry& new_entry(const NetworkInterface* iface,
GrowableArray<InterfaceEntry>* interfaces) {
   assert(iface != NULL, "invariant");
   assert(interfaces != NULL, "invariant");

   // single threaded premise
   static traceid interface_id = 0;                       // this is the static local variable

   const char* name = iface->get_name();
   assert(name != NULL, "invariant");

   InterfaceEntry entry;
   const size_t length = strlen(name);
   entry.name = NEW_C_HEAP_ARRAY(char, length + 1, mtInternal);
   strncpy(entry.name, name, length + 1);
   entry.id = ++interface_id;
   entry.bytes_in = iface->get_bytes_in();
   entry.bytes_out = iface->get_bytes_out();
   entry.written = false;
   return _interfaces->at(_interfaces->append(entry));
}
---------------------------------------------------------------

在 2020/3/9 15:29, Jia Huang 写道:
> Haha, I'm also trying to email hotspot-jfr-dev.
> Thank you, David.
>
> 在 2020/3/9 13:03, David Holmes 写道:
>> Hi Jia,
>>
>> I see this is getting no response so have cc'd hotspot-jfr-dev as 
>> they should have a better idea of how this gtest can and cannot be used.
>>
>> Just because the GTest framework supports "REPEAT" it doesn't mean 
>> all tests can, or should, be repeatable.
>>
>> Cheers,
>> David
>>
>> On 29/02/2020 1:53 pm, Jia Huang wrote:
>>> Hi David,
>>>
>>> 在 2020/2/28 21:16, David Holmes 写道:
>>>> Hi Jia,
>>>>
>>>> On 28/02/2020 11:08 pm, Jia Huang wrote:
>>>>> Hi all,
>>>>>
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8240222
>>>>> Webrev: http://cr.openjdk.java.net/~jiahuang/8240222/webrev.00/
>>>>>
>>>>> gtest/jfr/test_networkUtilization.cpp failed when the number of 
>>>>> tests is greater than or equal to 2.
>>>>> As shown in the following code, the test failed because "traceid 
>>>>> id" is a static local variable.
>>>>> When the test runs a second time, "id" continues to grow, 
>>>>> resulting in the return value of "i" being "_interfaces.end()".
>>>>
>>>> Interesting! I'm not at all familiar with the details of gtest and 
>>>> how it actually runs things. But I would expect that we have many 
>>>> tests that rely on starting from a cleanly initialized VM state, or 
>>>> test state, and so can't simply be repeated (it would be like 
>>>> editing an arbitrary test and putting the body of main into a 
>>>> for-loop!).
>>>
>>> I'm sorry that my last description may be a little incomplete.
>>> JfrTestNetworkUtilization is a group of tests,it includes four 
>>> individual tests:
>>>    --JfrTestNetworkUtilization.RequestFunctionBasic_test_vm
>>>    --JfrTestNetworkUtilization.RequestFunctionMultiple_test_vm
>>>    --JfrTestNetworkUtilization.InterfaceRemoved_test_vm
>>>    --JfrTestNetworkUtilization.InterfaceReset_test_vm
>>>
>>> If you only run a individual test once, such as the following 
>>> Testing, it still fails.
>>> The reason for the failure, as I said last time, is also because of 
>>> static local variables.
>>> In the "RequestFunctionMultiple_test_vm" test, the sequence number 
>>> of "eth0" is 2.
>>> However, the initial value of the static local variable "id" is 0.
>>>
>>> Testting:
>>>     --make run-test
>>> TEST=gtest:JfrTestNetworkUtilization.RequestFunctionMultiple_test_vm
>>> GTEST=REPEAT=1 CONF=release
>>>     --make run-test
>>> TEST=gtest:JfrTestNetworkUtilization.InterfaceRemoved_test_vm
>>> GTEST=REPEAT=1 CONF=release
>>>     --make run-test
>>> TEST=gtest:JfrTestNetworkUtilization.InterfaceReset_test_vm
>>> GTEST=REPEAT=1 CONF=release
>>>
>>>>
>>>> So I'm not sure what the right way to fix this is:
>>>> - don't use REPEAT?
>>>> - declare the test as not repeatable somehow?
>>>> - change the test to make it repeatable?
>>>
>>> In the doc/testing.md or doc/testing.html, Gtest keywords inclue 
>>> "REPEAT", so I think this means that all GTEST tests should be able 
>>> to use the "REPEAT parameter.
>>>
>>>
>>> Best regards,
>>> Jia
>>>
>>>
>>>>
>>>> Cheers,
>>>> David
>>>>
>>>>> ---------------------------------------------------------------------------- 
>>>>>
>>>>> test/hotspot/gtest/jfr/test_networkUtilization.cpp
>>>>>
>>>>> static const MockNetworkInterface& get_interface(traceid id) {
>>>>>    std::list<MockNetworkInterface>::const_iterator i = 
>>>>> _interfaces.begin();
>>>>>    for (; i != _interfaces.end(); ++i) {
>>>>>      if (i->id == id) {
>>>>>        break;
>>>>>      }
>>>>>    }
>>>>>    return *i;
>>>>> }
>>>>> ---------------------------------------------------------------------------- 
>>>>>
>>>>>
>>>>>
>>>>> Testing:
>>>>>      - make run-test TEST=gtest:JfrTestNetworkUtilization 
>>>>> GTEST=REPEAT=2 CONF=server-release
>>>>>      - make run-test TEST=gtest:JfrTestNetworkUtilization 
>>>>> GTEST=REPEAT=2 CONF=server-fastdebug
>>>>>
>>>>> Thanks a lot
>>>>>
>>>>> Best regards,
>>>>> Jia
>>>>>
>>>