Re: [PATCH v3 3/3] selftests/ftrace: add test case for GRP/ only input

[Linyu Yuan <quic_linyyuan@quicinc.com>]

hi Masami,

thanks for review, will update your comment in next V4.

thanks

On 6/1/2022 10:57 PM, Masami Hiramatsu (Google) wrote:
> Hi Linyu,
>
> On Tue, 31 May 2022 14:29:12 +0800
> Linyu Yuan <quic_linyyuan@quicinc.com> wrote:
>
>> add kprobe and eprobe event test for new GRP/ only format.
>>
>> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
>> ---
>> v3: first add in this version
>>
>>   .../testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc  | 7 ++++++-
>>   .../testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc  | 7 ++++++-
>>   2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
>> index 60c02b4..97856df 100644
>> --- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
>> +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
>> @@ -1,7 +1,7 @@
>>   #!/bin/sh
>>   # SPDX-License-Identifier: GPL-2.0
>>   # description: Generic dynamic event - add/remove eprobe events
>> -# requires: dynamic_events events/syscalls/sys_enter_openat "e[:[<group>/]<event>] <attached-group>.<attached-event> [<args>]":README
>> +# requires: dynamic_events events/syscalls/sys_enter_openat "e[:[<group>/][<event>]] <attached-group>.<attached-event> [<args>]":README
> Hmm, this is too restricted. With this change, this entire test only runs after
> patching this series.
> Can you change the requires so that skip checking the first half, like
>    "<attached-group>.<attached-event> [<args>]":README
> and check whether "e[:[<group>/][<event>]]" is in the README for the new
> test code?
>   
>>   echo 0 > events/enable
>>   
>> @@ -87,4 +87,9 @@ echo "-:eprobes/$EPROBE $SYSTEM/$EVENT $OPTIONS" >> dynamic_events
>>   ! grep -q "$EPROBE" dynamic_events
>>   ! test -d events/eprobes/$EPROBE
>>   
>> +echo "e:mygroup/ $SYSTEM/$EVENT $OPTIONS" >> dynamic_events
>> +test -d events/mygroup
>> +echo "-:mygroup/" >> dynamic_events
>> +! test -d events/mygroup
>> +
>>   clear_trace
>> diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
>> index b4da41d..6b102a7 100644
>> --- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
>> +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
>> @@ -1,7 +1,7 @@
>>   #!/bin/sh
>>   # SPDX-License-Identifier: GPL-2.0
>>   # description: Generic dynamic event - add/remove kprobe events
>> -# requires: dynamic_events "place: [<module>:]<symbol>":README "place (kretprobe): [<module>:]<symbol>":README
>> +# requires: dynamic_events "p[:[<group>/][<event>]] <place> [<args>]":README
> Ditto. In this case, don't touch the requires line (because it is not
> updated) and just add a README check for below new code.
>
> Thank you,
>
>
>>   
>>   echo 0 > events/enable
>>   echo > dynamic_events
>> @@ -23,4 +23,9 @@ grep -q myevent1 dynamic_events
>>   
>>   echo > dynamic_events
>>   
>> +echo "p:mygroup/ $PLACE" >> dynamic_events
>> +test -d events/mygroup
>> +echo "-:mygroup/" >> dynamic_events
>> +! test -d events/mygroup
>> +
>>   clear_trace
>> -- 
>> 2.7.4
>>
>