| Message ID | 20190409181111.1034-1-daniel.diaz@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests] scripts: Colorize only when possible | expand |
On Tue, Apr 09, 2019 at 01:11:11PM -0500, Daniel Díaz wrote: > Instead of colorizing the output of PASS/SKIP/FAIL all the > time, do it only when the terminal supports colors and stdout > is opened on a terminal. I'm guessing the motivation for this is because you're redirecting the output of run_tests.sh or the standalone scripts and getting something like ^[[32mPASS^[[0m selftest-vectors-kernel (2 tests) in the output, which doesn't look nice nor lend itself well to parsing. Because otherwise you must be using a pretty old vt100 :) If I'm right about the motivation then we mostly care about the '[ -t 1]' part and we can drop the 'tput' stuff in order to avoid the ncurses dependency. Also that motivation should be added to the commit message. > > Signed-off-by: Daniel Díaz <daniel.diaz@linaro.org> > --- > scripts/runtime.bash | 20 +++++++++++++++++--- > 1 file changed, 17 insertions(+), 3 deletions(-) > > diff --git a/scripts/runtime.bash b/scripts/runtime.bash > index 200d5b6..0c38787 100644 > --- a/scripts/runtime.bash > +++ b/scripts/runtime.bash > @@ -2,9 +2,23 @@ > : ${MAX_SMP:=$(getconf _NPROCESSORS_CONF)} > : ${TIMEOUT:=90s} > > -PASS() { echo -ne "\e[32mPASS\e[0m"; } > -SKIP() { echo -ne "\e[33mSKIP\e[0m"; } > -FAIL() { echo -ne "\e[31mFAIL\e[0m"; } > +# Define text colors > +# Check available colors on the terminal, if any > +ncolors=`tput colors 2>/dev/null` > +color_reset= > +color_red= > +color_green= > +color_yellow= > +# If stdout exists and number of colors is eight or more, use them > +if [ -t 1 -a "$ncolors" -a "$ncolors" -ge 8 ]; then > + color_reset="\e[0m" > + color_red="\e[31m" > + color_green="\e[32m" > + color_yellow="\e[33m" > +fi Some bash style nits: - Please use $() instead of `` - Please use if [ cond1 ] && [ cond2 ] && ... instead of '-a' - This file uses four spaces for indentation. > +PASS() { echo -ne "${color_green}PASS${color_reset}"; } > +SKIP() { echo -ne "${color_yellow}SKIP${color_reset}"; } > +FAIL() { echo -ne "${color_red}FAIL${color_reset}"; } > > extract_summary() > { > -- > 2.17.1 > Thanks, drew
Hello! On Wed, 10 Apr 2019 at 04:34, Andrew Jones <drjones@redhat.com> wrote: > On Tue, Apr 09, 2019 at 01:11:11PM -0500, Daniel Díaz wrote: > > Instead of colorizing the output of PASS/SKIP/FAIL all the > > time, do it only when the terminal supports colors and stdout > > is opened on a terminal. > > I'm guessing the motivation for this is because you're redirecting the > output of run_tests.sh or the standalone scripts and getting something > like > > ^[[32mPASS^[[0m selftest-vectors-kernel (2 tests) > > in the output, which doesn't look nice nor lend itself well to parsing. That's the case, yes [1]. We [2] come from the context of parsing the logs and extracting the test results, and, while doable [3], color sequences generally get in the way of easily parsing the output. > Because otherwise you must be using a pretty old vt100 :) If I'm right > about the motivation then we mostly care about the '[ -t 1]' part and > we can drop the 'tput' stuff in order to avoid the ncurses dependency. > Also that motivation should be added to the commit message. [...] > Some bash style nits: > > - Please use $() instead of `` > - Please use if [ cond1 ] && [ cond2 ] && ... instead of '-a' > - This file uses four spaces for indentation. Great! Thanks for the review. I'll send V2 briefly. Greetings! Daniel Díaz daniel.diaz@linaro.org [1] https://lkft.validation.linaro.org/scheduler/job/673625#L1462 [2] https://lkft.linaro.org/ [3] https://lkft.validation.linaro.org/results/673625/1_kvm-unit-tests
diff --git a/scripts/runtime.bash b/scripts/runtime.bash index 200d5b6..0c38787 100644 --- a/scripts/runtime.bash +++ b/scripts/runtime.bash @@ -2,9 +2,23 @@ : ${MAX_SMP:=$(getconf _NPROCESSORS_CONF)} : ${TIMEOUT:=90s} -PASS() { echo -ne "\e[32mPASS\e[0m"; } -SKIP() { echo -ne "\e[33mSKIP\e[0m"; } -FAIL() { echo -ne "\e[31mFAIL\e[0m"; } +# Define text colors +# Check available colors on the terminal, if any +ncolors=`tput colors 2>/dev/null` +color_reset= +color_red= +color_green= +color_yellow= +# If stdout exists and number of colors is eight or more, use them +if [ -t 1 -a "$ncolors" -a "$ncolors" -ge 8 ]; then + color_reset="\e[0m" + color_red="\e[31m" + color_green="\e[32m" + color_yellow="\e[33m" +fi +PASS() { echo -ne "${color_green}PASS${color_reset}"; } +SKIP() { echo -ne "${color_yellow}SKIP${color_reset}"; } +FAIL() { echo -ne "${color_red}FAIL${color_reset}"; } extract_summary() {
Instead of colorizing the output of PASS/SKIP/FAIL all the time, do it only when the terminal supports colors and stdout is opened on a terminal. Signed-off-by: Daniel Díaz <daniel.diaz@linaro.org> --- scripts/runtime.bash | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)