RFR(S): 8167493: Test that JFR event can be retransformed by an agent

mikhailo.seledtsov at oracle.com mikhailo.seledtsov at oracle.com
Fri Feb 14 18:03:13 UTC 2020


Here is the updated webrev: 
http://cr.openjdk.java.net/~mseledtsov/8167493.03/

On 2/14/20 9:40 AM, Erik Gahlin wrote:
> Yes, that sums it up well.
>
> I think it would be good to use try-with-resources with the recording 
> object and make the transformer a separate inner class to make it more 
> explicit that the transform method belongs to the transformer.
>
> Erik
>
> On 2020-02-14 18:19, mikhailo.seledtsov at oracle.com wrote:
>> Hi Erik,
>>
>>   Thank you for the initial review. I looked thru your proposed 
>> logic, and it makes sense to me. If I understand correctly, changes 
>> you propose can be summarized as:
>>
>>    - simplify premain and move all test logic to main
>>
>>    - test case that tests construction of the event only, w/o commit
>>
>>    - test case with recording & commit, and assert that the event has 
>> been recorded (in addition to instrumentation callback)
>>
>>
>> I will update the test accordingly, retest and post updated webrev.
>>
>> Misha
>>
>> On 2/14/20 9:12 AM, Erik Gahlin wrote:
>>> Hi Misha,
>>>
>>> I think this tests things as they should but I'm not 100% sure. It 
>>> may make sense to change the flow a bit so it becomes more explicit. 
>>> if you only does this in premain.
>>>
>>> public static void premain(String args, Instrumentation inst) throws 
>>> Exception {
>>>    instrumentation = inst;
>>> }
>>>
>>> then in main:
>>>
>>> 1. TestEvent event = new TestEvent(); // loads class and run empty 
>>> constructor
>>> 2. instrumentation.addTransformer(new TestClassFileTransformer());
>>> 3. instrumentation.retransformClasses(TestEvent.class);
>>> 4. TestEvent event = new TestEvent(); // instrumented constructor
>>> 5. Asserts.assertTrue(InstrumentationEventCallback.wasCalled());
>>> 6. InstrumentationEventCallback.clear();
>>> 7. Recording r = new Recording(); r.enable(TestEvent.class); r.start();
>>> 8. TestEvent event = new TestEvent(); event.commit(); // JFR 
>>> instrumented commit
>>> 9. Asserts.assertTrue(InstrumentationEventCallback.wasCalled());
>>> 10. r.dump(file)
>>> 11. Asserts.assertTrue(!RecordingFile.readAllEvents().isEmpty())
>>>
>>> Thanks
>>> Erik
>>>
>>> On 2020-02-13 17:12, mikhailo.seledtsov at oracle.com wrote:
>>>
>>>> Please review this change that adds a new test to verify that a 
>>>> user-defined JFR event can be tramsformed by an agent.
>>>> This test uses a class transformer to add a callback into the 
>>>> <init> of the event class, generates/records the event,
>>>> and verifies that the added instrumentation callback was actually 
>>>> called.
>>>>
>>>> Also, the JavaAgentBuilder testlib utility was extended to accept 
>>>> additional manifest attributes.
>>>>
>>>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8167493
>>>>     Webrev: http://cr.openjdk.java.net/~mseledtsov/8167493.02/
>>>>     Testing:
>>>>         1. Ran the newly created test: PASS
>>>>         2. Ran all tests using JavaAgentBuilder: PASS
>>>>
>>>>
>>>> Thank you,
>>>> Misha
>>>>


More information about the hotspot-jfr-dev mailing list