Re: [PATCH] selftests/ftrace: Add color to the PASS / FAIL results

[Shuah Khan]

On 10/16/2018 09:39 AM, Steven Rostedt wrote:
> On Wed, 17 Oct 2018 00:26:32 +0900
> Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
> 
>> On Mon, 15 Oct 2018 23:42:31 +0900
>> Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
>>
>>> On Fri, 12 Oct 2018 22:21:06 -0400
>>> Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>>>   
>>>> From: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>
>>>>
>>>> Now that ftracetest has over 80 tests, it is difficult to simply scroll
>>>> up the console window to find the failed tests when it reports just two
>>>> tests have failed. In order to make this stand out better, have the
>>>> color of the word "PASS" be green, "FAIL" and "XFAIL" be red, and all
>>>> other results be blue. This helps tremendously in quickly spotting the
>>>> failed tests by just scrolling up the console window.
>>>>   
>>>
>>> Looks good to me.
>>>
>>> Acked-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>  
>>
>> Oops, I found some problems.
>>
>>>
>>> Thanks!
>>>   
>>>> Signed-off-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>
>>>> ---
>>>> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
>>>> index 5c71d58..4946b2e 100755
>>>> --- a/tools/testing/selftests/ftrace/ftracetest
>>>> +++ b/tools/testing/selftests/ftrace/ftracetest
>>>> @@ -153,7 +153,7 @@ else
>>>>  fi
>>>>  
>>>>  prlog() { # messages
>>>> -  [ -z "$LOG_FILE" ] && echo "$@" || echo "$@" | tee -a $LOG_FILE
>>>> +  [ -z "$LOG_FILE" ] && echo -e "$@" || echo -e "$@" | tee -a $LOG_FILE  
>>
>> I think we should strip escape-sequences from log files, like below;
>>
>> strip_esc() {
>>   sed -E "s/[[:cntrl:]]\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g"
>> }
>>
>>   [ -z "$LOG_FILE" ] && echo -e "$@" || echo -e "$@" | strip_esc | tee -a $LOG_FILE
>>
>>
>>>>  }
>>>>  catlog() { #file
>>>>    [ -z "$LOG_FILE" ] && cat $1 || cat $1 | tee -a $LOG_FILE
>>>> @@ -195,37 +195,37 @@ test_on_instance() { # testfile
>>>>  eval_result() { # sigval
>>>>    case $1 in
>>>>      $PASS)
>>>> -      prlog "	[PASS]"
>>>> +      prlog "	[\e[32mPASS\e[30m]"  
>>
>> The last one should be "\e[0m" (reset), "\e[30m" just set the color "black".
>> That doesn't work on my terminal, which is "white on black".
>>
>>
> 
> Ah thanks for the feedback. Want to write a patch on top, that Shuah
> can add?
> 
> -- Steve
> 

That will be great. I can apply the updated patch with Masami's Reviewed-by
perhaps.

thanks,
-- Shuah