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

[Erik Gahlin]

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
>>>