[Ezbench-dev] [PATCH 23/25] tests/piglit/igt: Use owatch, reboot on monitored abort

[Martin Peres]

On 24/02/17 13:19, Petri Latvala wrote:
> Signed-off-by: Petri Latvala <petri.latvala at intel.com>
> ---
>   tests.d/piglit/igt.test | 31 ++++++++++++++++++++++++++-----
>   1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/tests.d/piglit/igt.test b/tests.d/piglit/igt.test
> index 9ae8d09..2228b3f 100644
> --- a/tests.d/piglit/igt.test
> +++ b/tests.d/piglit/igt.test
> @@ -1,5 +1,16 @@
>   test -e "$REPO_PIGLIT/piglit" || return 1
>   
> +OWATCHBIN="$ezBenchDir/utils/owatch/owatch"
> +OWATCH=
> +if [ -x "$OWATCHBIN" ]; then
> +    OWATCH="$OWATCHBIN 650"
> +fi
> +
> +PIGLITCONF=
> +if [ -f "$IGT_ROOT/piglit.conf" ]; then
> +    PIGLITCONF="-f $IGT_ROOT/piglit.conf"
> +fi

would be nice to split the two changes in two different commits 
(piglit.conf and owatch)

> +
>   function __igt_resume__ {
>       cd "$REPO_PIGLIT"
>   
> @@ -11,15 +22,15 @@ function __igt_resume__ {
>       # generate the report
>       if [ -f "$piglit_output/metadata.json" ]; then
>           # ignore the incomplete result to avoid being stuck in a loop
> -        local cmdline="sudo IGT_TEST_ROOT="$IGT_ROOT/tests" $REPO_PIGLIT/piglit resume -n ${run_log_file}_tmp"
> +        local cmdline="sudo IGT_TEST_ROOT="$IGT_ROOT/tests" $OWATCH $REPO_PIGLIT/piglit resume $PIGLITCONF -n ${run_log_file}_tmp"
>   
>           ENV_DUMP_RESTRICT_TO_BINARY="NO_ENVDUMP_PLEASE" \
>           $cmdline >> "$run_log_file.stdout" 2>> "$run_log_file.stderr"
>           local exit_code=$?
>   
> -        # piglit resume returns 1 if we try to resume a report that is already done
>           if [ $exit_code -ne 0 ]; then
> -            __igt_resume__
> +	    # reboot
> +            return 74

This change should be part of the piglit.conf patch, because this only 
works well if it sets --abort-on-errors.

How about putting piglit.conf in ezbench's tree, next to 
piglit-igt.conf? This way, we can rely on it being there, and
we can add abort conditions for amd or other drivers without conflicting 
with intel.

>           fi
>       fi
>   
> @@ -50,7 +61,11 @@ function __igt_run__ {
>       local piglit_output="${run_log_file}_tmp"
>   
>       # start piglit
> -    local cmdline="sudo IGT_TEST_ROOT="$IGT_ROOT/tests" $REPO_PIGLIT/piglit run $PIGLIT_RUN_PARAMS -s $parameters $testlist tests/igt.py ${piglit_output}"
> +    ABORTFLAG=
> +    if [ -n "$PIGLITCONF" ]; then
> +	ABORTFLAG="--abort-on-monitored-error"
> +    fi

I had no problem forcing the -s flag, I think this one could also be 
forced in here, no matter if we have piglit.conf available or not (or we 
mandate piglit.conf).

> +    local cmdline="sudo IGT_TEST_ROOT="$IGT_ROOT/tests" $OWATCH $REPO_PIGLIT/piglit run $PIGLITCONF $ABORTFLAG $PIGLIT_RUN_PARAMS -s $parameters $testlist tests/igt.py ${piglit_output}"
>   
>       ENV_DUMP_REQUIRE_ARGUMENT="$REPO_PIGLIT/piglit" \
>       ENV_DUMP_NO_METRICS=1 \
> @@ -58,7 +73,13 @@ function __igt_run__ {
>       local exit_code=$?
>   
>       if [ $exit_code -ne 0 ]; then
> -        __igt_resume__
> +	if [ -f "$piglit_output/metadata.json" ]; then
> +            # Testing started but didn't complete. Reboot.
> +	    return 74
> +	else
> +	    # Testing couldn't even begin
> +	    return 1
> +	fi

Very nice!

>       else
>           sudo chown -R "$(id -un):$(id -gn)" "$piglit_output"
>           __piglit_gen_report__