[Intel-gfx] [PATCH i-g-t v2] tests: Clean up shell scripts
Jani Nikula
jani.nikula at intel.com
Mon Feb 6 13:25:27 UTC 2017
On Mon, 06 Feb 2017, Joonas Lahtinen <joonas.lahtinen at linux.intel.com> wrote:
> Convert all scripts to use /bin/sh shebang and fix all shellcheck
> reported problems.
Pro-tip, this is the place reserved in commit messages for describing
*why* you think the change is needed or for the better. ;)
BR,
Jani.
>
> v2: Include tests/Makefile.sources (Petri)
>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Petri Latvala <petri.latvala at intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Jani Nikula <jani.nikula at intel.com>
> Reviewed-by: Petri Latvala <petri.latvala at intel.com>
> ---
> tests/Makefile.sources | 1 -
> tests/check_drm_clients | 15 +++++++++----
> tests/ddx_intel_after_fbdev | 42 ++++++++++++++++++-------------------
> tests/debugfs_emon_crash | 16 +++++++++-----
> tests/debugfs_wedged | 18 ++++++++++------
> tests/drm_getopt.sh | 20 ++++++++++--------
> tests/drm_lib.sh | 51 +++++++++++++++++++++++----------------------
> tests/drv_debugfs_reader | 17 ++++++++++-----
> tests/igt_command_line.sh | 26 +++++++++++------------
> tests/sysfs_l3_parity | 30 ++++++++++++++++----------
> tests/test_rte_check | 6 ------
> tests/tools_test | 17 +++++++++------
> 12 files changed, 147 insertions(+), 112 deletions(-)
> delete mode 100755 tests/test_rte_check
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 6e07d93..dcc80c5 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -242,7 +242,6 @@ TESTS_scripts = \
> debugfs_emon_crash \
> drv_debugfs_reader \
> sysfs_l3_parity \
> - test_rte_check \
> tools_test \
> $(NULL)
>
> diff --git a/tests/check_drm_clients b/tests/check_drm_clients
> index 2a891b8..25c03b1 100755
> --- a/tests/check_drm_clients
> +++ b/tests/check_drm_clients
> @@ -1,6 +1,13 @@
> -#!/bin/bash
> +#!/bin/sh
>
> -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> -. $SOURCE_DIR/drm_lib.sh
> +if [ -n "$BASH_SOURCE" ]; then
> + THIS_SCRIPT=$BASH_SOURCE
> +else
> + THIS_SCRIPT=$0
> +fi
> +
> +SOURCE_DIR=$(dirname "$THIS_SCRIPT")
> +. "$SOURCE_DIR/drm_lib.sh"
> +
> +exit "$IGT_EXIT_SUCCESS"
>
> -exit $IGT_EXIT_SUCCESS
> diff --git a/tests/ddx_intel_after_fbdev b/tests/ddx_intel_after_fbdev
> index f068209..2ca5ce0 100755
> --- a/tests/ddx_intel_after_fbdev
> +++ b/tests/ddx_intel_after_fbdev
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!/bin/sh
> #
> # Testcase: Load Intel DDX after fbdev was loaded
> #
> @@ -9,7 +9,7 @@ whoami | grep -q root || {
> }
>
> # no other X session should be running
> -find /tmp/ -name .X*lock 2>/dev/null | grep -q X && {
> +find /tmp/ -name ".X*lock" 2>/dev/null | grep -q X && {
> echo "ERROR: X session already running"
> exit 1
> }
> @@ -19,14 +19,14 @@ TMPDIR=$(mktemp -d /tmp/igt.XXXX) || {
> exit 1
> }
>
> -cat > $TMPDIR/xorg.conf.fbdev << EOF
> +cat > "$TMPDIR/xorg.conf.fbdev" << EOF
> Section "Device"
> Driver "fbdev"
> Identifier "Device[fbdev]"
> EndSection
> EOF
>
> -cat > $TMPDIR/xorg.conf.intel << EOF
> +cat > "$TMPDIR/xorg.conf.intel" << EOF
> Section "Device"
> Driver "intel"
> Identifier "Device[intel]"
> @@ -34,40 +34,40 @@ EndSection
> EOF
>
> # log before fbdev
> -dmesg -c > $TMPDIR/dmesg.1.before.fbdev
> -cp /var/log/Xorg.0.log $TMPDIR/Xorg.0.log.1.before.fbdev
> +dmesg -c > "$TMPDIR/dmesg.1.before.fbdev"
> +cp /var/log/Xorg.0.log "$TMPDIR/Xorg.0.log.1.before.fbdev"
>
> # run fbdev
> -xinit -- /usr/bin/X -config $TMPDIR/xorg.conf.fbdev &
> +xinit -- /usr/bin/X -config "$TMPDIR/xorg.conf.fbdev" &
> sleep 5
> -if [ -f `which intel_reg` ]; then
> -`which intel_reg` dump > $TMPDIR/intel_reg_dump.1.fbdev
> +if [ -f "$(which intel_reg)" ]; then
> + intel_reg dump > "$TMPDIR/intel_reg_dump.1.fbdev"
> fi
> killall X
>
> # log after fbdev & before intel
> -dmesg -c > $TMPDIR/dmesg.2.after.fbdev.before.intel
> -cp /var/log/Xorg.0.log $TMPDIR/Xorg.0.log.2.after.fbdev.before.intel
> +dmesg -c > "$TMPDIR/dmesg.2.after.fbdev.before.intel"
> +cp /var/log/Xorg.0.log "$TMPDIR/Xorg.0.log.2.after.fbdev.before.intel"
>
> sleep 5
>
> # run intel
> -xinit -- /usr/bin/X -config $TMPDIR/xorg.conf.intel &
> -sleep 5
> -if [ -f `which intel_reg` ]; then
> -`which intel_reg` dump > $TMPDIR/intel_reg_dump.2.intel
> +xinit -- /usr/bin/X -config "$TMPDIR/xorg.conf.intel" &
> +sleep 5
> +if [ -f "$(which intel_reg)" ]; then
> + intel_reg dump > "$TMPDIR/intel_reg_dump.2.intel"
> fi
> killall X
>
> # log after intel
> -dmesg -c > $TMPDIR/dmesg.3.after.intel
> -cp /var/log/Xorg.0.log $TMPDIR/Xorg.0.log.3.after.intel
> +dmesg -c > "$TMPDIR/dmesg.3.after.intel"
> +cp /var/log/Xorg.0.log "$TMPDIR/Xorg.0.log.3.after.intel"
>
> -cp $0 $TMPDIR/
> +cp "$0" "$TMPDIR/"
>
> -tar czf $TMPDIR.tar.gz $TMPDIR/*
> -if [ -f $TMPDIR.tar.gz ]; then
> - echo $TMPDIR.tar.gz contains this script, all configs and logs generated on this tests
> +tar czf "$TMPDIR.tar.gz" "$TMPDIR/"
> +if [ -f "$TMPDIR.tar.gz" ]; then
> + echo "'$TMPDIR.tar.gz' contains this script, all configs and logs generated on this tests"
> fi
>
> exit 0
> diff --git a/tests/debugfs_emon_crash b/tests/debugfs_emon_crash
> index 1dbfcb2..11b1c44 100755
> --- a/tests/debugfs_emon_crash
> +++ b/tests/debugfs_emon_crash
> @@ -1,16 +1,22 @@
> -#!/bin/bash
> +#!/bin/sh
> #
> # This check if we can crash the kernel with segmentation-fault
> # by reading /sys/kernel/debug/dri/0/i915_emon_status too quickly
> #
>
> -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> -. $SOURCE_DIR/drm_lib.sh
> +if [ -n "$BASH_SOURCE" ]; then
> + THIS_SCRIPT=$BASH_SOURCE
> +else
> + THIS_SCRIPT=$0
> +fi
> +
> +SOURCE_DIR=$(dirname "$THIS_SCRIPT")
> +. "$SOURCE_DIR/drm_lib.sh"
>
> for z in $(seq 1 1000); do
> - cat $i915_dfs_path/i915_emon_status > /dev/null 2&>1
> + cat "$i915_dfs_path/i915_emon_status" > /dev/null 2>&1
> done
>
> # If we got here, we haven't crashed
> +exit "$IGT_EXIT_SUCCESS"
>
> -exit $IGT_EXIT_SUCCESS
> diff --git a/tests/debugfs_wedged b/tests/debugfs_wedged
> index f15ac46..26b40de 100755
> --- a/tests/debugfs_wedged
> +++ b/tests/debugfs_wedged
> @@ -1,10 +1,16 @@
> -#!/bin/bash
> +#!/bin/sh
>
> -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> -. $SOURCE_DIR/drm_lib.sh
> +if [ -n "$BASH_SOURCE" ]; then
> + THIS_SCRIPT=$BASH_SOURCE
> +else
> + THIS_SCRIPT=$0
> +fi
> +
> +SOURCE_DIR=$(dirname "$THIS_SCRIPT")
> +. "$SOURCE_DIR/drm_lib.sh"
>
> # Testcase: wedge the hw to check the error_state reading
> -#
> +#
> # Unfortunately wedged is permanent, so this test is not run by default
> -echo 1 > ${i915_dfs_path}/i915_wedged
> -cat $i915_dfs_path/i915_error_state > /dev/null 2>&1
> +echo 1 > "${i915_dfs_path}/i915_wedged"
> +cat "$i915_dfs_path/i915_error_state" > /dev/null 2>&1
> diff --git a/tests/drm_getopt.sh b/tests/drm_getopt.sh
> index a94a759..10f367c 100755
> --- a/tests/drm_getopt.sh
> +++ b/tests/drm_getopt.sh
> @@ -1,3 +1,4 @@
> +#!/bin/sh
> # This is required for check/distcheck target as it has to --list-subtests
> # for each test. Source it if you add a new test in form of a (shell) script.
>
> @@ -9,30 +10,31 @@ IGT_EXIT_SUCCESS=0
> IGT_EXIT_INVALID=79
> IGT_EXIT_FAILURE=99
>
> -# hacked-up long option parsing
> -for arg in $@ ; do
> - case $arg in
> +PROG=$(basename "$THIS_SCRIPT")
> +
> +for arg do
> + case "$arg" in
> --list-subtests)
> - exit $IGT_EXIT_INVALID
> + exit "$IGT_EXIT_INVALID"
> ;;
> --run-subtest)
> - exit $IGT_EXIT_INVALID
> + exit "$IGT_EXIT_INVALID"
> ;;
> --debug)
> IGT_LOG_LEVEL=debug
> ;;
> --help-description)
> - echo $IGT_TEST_DESCRIPTION
> - exit $IGT_EXIT_SUCCESS
> + echo "$IGT_TEST_DESCRIPTION"
> + exit "$IGT_EXIT_SUCCESS"
> ;;
> --help)
> - echo "Usage: `basename $0` [OPTIONS]"
> + echo "Usage: $PROG [OPTIONS]"
> echo " --list-subtests"
> echo " --run-subtest <pattern>"
> echo " --debug"
> echo " --help-description"
> echo " --help"
> - exit $IGT_EXIT_SUCCESS
> + exit "$IGT_EXIT_SUCCESS"
> ;;
> esac
> done
> diff --git a/tests/drm_lib.sh b/tests/drm_lib.sh
> index af104ad..2aa073d 100755
> --- a/tests/drm_lib.sh
> +++ b/tests/drm_lib.sh
> @@ -1,23 +1,22 @@
> -#!/bin/bash
> +#!/bin/sh
>
> -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> -. $SOURCE_DIR/drm_getopt.sh
> +. "$SOURCE_DIR/drm_getopt.sh"
>
> -NAME=$(basename "$0")
> +NAME=$(basename "$THIS_SCRIPT")
>
> dynamic_debug=
>
> hda_dynamic_debug_enable() {
> if [ -e "$dynamic_debug" ]; then
> - echo -n "module snd_hda_intel +pf" > $dynamic_debug
> - echo -n "module snd_hda_core +pf" > $dynamic_debug
> + echo "module snd_hda_intel +pf" > "$dynamic_debug"
> + echo "module snd_hda_core +pf" > "$dynamic_debug"
> fi
> }
>
> hda_dynamic_debug_disable() {
> if [ -e "$dynamic_debug" ]; then
> - echo -n "module snd_hda_core =_" > $dynamic_debug
> - echo -n "module snd_hda_intel =_" > $dynamic_debug
> + echo "module snd_hda_core =_" > "$dynamic_debug"
> + echo "module snd_hda_intel =_" > "$dynamic_debug"
> fi
> }
>
> @@ -31,7 +30,9 @@ KERN_INFO="<6>"
> KERN_DEBUG="<7>"
>
> kmsg() {
> - echo "$@" > /dev/kmsg
> + if [ -w "/dev/kmsg" ]; then
> + echo "$@" > /dev/kmsg
> + fi
> }
>
> finish() {
> @@ -46,16 +47,16 @@ kmsg "${KERN_INFO}[IGT] $NAME: executing"
>
> skip() {
> echo "$@"
> - exit $IGT_EXIT_SKIP
> + exit "$IGT_EXIT_SKIP"
> }
>
> die() {
> echo "$@"
> - exit $IGT_EXIT_FAILURE
> + exit "$IGT_EXIT_FAILURE"
> }
>
> do_or_die() {
> - $@ > /dev/null 2>&1 || (echo "FAIL: $@ ($?)" && exit $IGT_EXIT_FAILURE)
> + "$@" > /dev/null 2>&1 || ( echo "FAIL: $* ($?)" && exit "$IGT_EXIT_FAILURE" )
> }
>
> if [ -d /sys/kernel/debug ]; then
> @@ -66,47 +67,47 @@ else
> skip "debugfs not found"
> fi
>
> -dynamic_debug=$debugfs_path/dynamic_debug/control
> +dynamic_debug="$debugfs_path/dynamic_debug/control"
> if [ ! -e "$dynamic_debug" ]; then
> echo "WARNING: dynamic debug control not available"
> fi
>
> -if [ ! -d $debugfs_path/dri ]; then
> +if [ ! -d "$debugfs_path/dri" ]; then
> skip "dri debugfs not found"
> fi
>
> -i915_dfs_path=x
> -for minor in `seq 0 16`; do
> - if [ -f $debugfs_path/dri/$minor/i915_error_state ] ; then
> - i915_dfs_path=$debugfs_path/dri/$minor
> +i915_dfs_path=
> +for minor in $(seq 0 16); do
> + if [ -f "$debugfs_path/dri/$minor/i915_error_state" ] ; then
> + i915_dfs_path="$debugfs_path/dri/$minor"
> break
> fi
> done
>
> -if [ $i915_dfs_path = "x" ] ; then
> - skip " i915 debugfs path not found."
> +if [ -z "$i915_dfs_path" ] ; then
> + skip "i915 debugfs path not found."
> fi
>
> # read everything we can
> -if [ `cat $i915_dfs_path/clients | wc -l` -gt "2" ] ; then
> +if [ "$(wc -l "$i915_dfs_path/clients")" -gt 2 ] ; then
> [ -n "$DRM_LIB_ALLOW_NO_MASTER" ] || \
> die "ERROR: other drm clients running"
> fi
>
> -whoami | grep -q root || ( echo ERROR: not running as root; exit $IGT_EXIT_FAILURE )
> +whoami | grep -q root || ( echo "ERROR: not running as root"; exit "$IGT_EXIT_FAILURE" )
>
> i915_sfs_path=
> if [ -d /sys/class/drm ] ; then
> sysfs_path=/sys/class/drm
> - if [ -f $sysfs_path/card$minor/error ] ; then
> + if [ -f "$sysfs_path/card$minor/error" ] ; then
> i915_sfs_path="$sysfs_path/card$minor"
> fi
> fi
> # sysfs may not exist as the 'error' is a new interface in 3.11
>
> -function drmtest_skip_on_simulation()
> +drmtest_skip_on_simulation()
> {
> - [ -n "$INTEL_SIMULATION" ] && exit $IGT_EXIT_SKIP
> + [ -n "$INTEL_SIMULATION" ] && exit "$IGT_EXIT_SKIP"
> }
>
> drmtest_skip_on_simulation
> diff --git a/tests/drv_debugfs_reader b/tests/drv_debugfs_reader
> index 6ea4e64..58d02de 100755
> --- a/tests/drv_debugfs_reader
> +++ b/tests/drv_debugfs_reader
> @@ -1,9 +1,16 @@
> -#!/bin/bash
> +#!/bin/sh
>
> -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> -. $SOURCE_DIR/drm_lib.sh
> +if [ -n "$BASH_SOURCE" ]; then
> + THIS_SCRIPT=$BASH_SOURCE
> +else
> + THIS_SCRIPT=$0
> +fi
> +
> +SOURCE_DIR=$(dirname "$THIS_SCRIPT")
> +. "$SOURCE_DIR/drm_lib.sh"
>
> # read everything we can
> -cat $i915_dfs_path/* > /dev/null 2>&1
> +cat "$i915_dfs_path"/* > /dev/null 2>&1
> +
> +exit "$IGT_EXIT_SUCCESS"
>
> -exit $IGT_EXIT_SUCCESS
> diff --git a/tests/igt_command_line.sh b/tests/igt_command_line.sh
> index a20e44c..bab3a77 100755
> --- a/tests/igt_command_line.sh
> +++ b/tests/igt_command_line.sh
> @@ -26,11 +26,11 @@
> #
>
> if [ -z "$top_builddir" ]; then
> - top_builddir="$(dirname $0)"
> + top_builddir=$(dirname "$0")
> fi
>
> # allow to run this script from top directory
> -TESTLIST=`cat $top_builddir/test-list.txt`
> +TESTLIST=$(cat "$top_builddir/test-list.txt")
> if [ $? -ne 0 ]; then
> # distcheck requires this hack
> TESTLIST=$(cat test-list.txt)
> @@ -51,7 +51,7 @@ for test in $TESTLIST; do
> fi
>
> # top_builddir is empty for distcheck
> - test=$top_builddir/$test
> + test="$top_builddir/$test"
>
> # distcheck requires this hack
> if [ ! -x "$test" ]; then
> @@ -62,29 +62,29 @@ for test in $TESTLIST; do
>
> # check invalid option handling
> echo " Checking invalid option handling..."
> - ./$test --invalid-option 2> /dev/null && fail $test
> + "./$test" --invalid-option 2> /dev/null && fail "$test"
>
> # check valid options succeed
> echo " Checking valid option handling..."
> - ./$test --help > /dev/null || fail $test
> + "./$test" --help > /dev/null || fail "$test"
>
> # check --list-subtests works correctly
> echo " Checking subtest enumeration..."
> - LIST=`./$test --list-subtests`
> + LIST=$("./$test" --list-subtests)
> RET=$?
> - if [ $RET -ne 0 -a $RET -ne 79 ]; then
> - fail $test
> + if [ "$RET" -ne 0 -a $RET -ne 79 ]; then
> + fail "$test"
> fi
>
> - if [ $RET -eq 79 -a -n "$LIST" ]; then
> - fail $test
> + if [ "$RET" -eq 79 -a -n "$LIST" ]; then
> + fail "$test"
> fi
>
> - if [ $RET -eq 0 -a -z "$LIST" ]; then
> - fail $test
> + if [ "$RET" -eq 0 -a -z "$LIST" ]; then
> + fail "$test"
> fi
>
> # check invalid subtest handling
> echo " Checking invalid subtest handling..."
> - ./$test --run-subtest invalid-subtest > /dev/null 2>&1 && fail $test
> + "./$test" --run-subtest invalid-subtest > /dev/null 2>&1 && fail "$test"
> done
> diff --git a/tests/sysfs_l3_parity b/tests/sysfs_l3_parity
> index d5f3284..37a03f1 100755
> --- a/tests/sysfs_l3_parity
> +++ b/tests/sysfs_l3_parity
> @@ -1,22 +1,30 @@
> -#!/bin/bash
> +#!/bin/sh
>
> -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> -. $SOURCE_DIR/drm_lib.sh
> +if [ -n "$BASH_SOURCE" ]; then
> + THIS_SCRIPT=$BASH_SOURCE
> +else
> + THIS_SCRIPT=$0
> +fi
> +
> +SOURCE_DIR=$(dirname "$THIS_SCRIPT")
> +. "$SOURCE_DIR/drm_lib.sh"
>
> -$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -e || exit $?
> +"$SOURCE_DIR/../tools/intel_l3_parity" -r 0 -b 0 -s 0 -e || exit $?
>
> #Check that we can remap a row
> -$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -d
> -disabled=`$SOURCE_DIR/../tools/intel_l3_parity -l | grep -c 'Row 0, Bank 0, Subbank 0 is disabled'`
> -if [ "$disabled" != "1" ] ; then
> +"$SOURCE_DIR/../tools/intel_l3_parity" -r 0 -b 0 -s 0 -d
> +disabled=$("$SOURCE_DIR/../tools/intel_l3_parity" -l | grep -c 'Row 0, Bank 0, Subbank 0 is disabled')
> +if [ "$disabled" != 1 ] ; then
> echo "Fail"
> - exit $IGT_EXIT_FAILURE
> + exit "$IGT_EXIT_FAILURE"
> fi
>
> -$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -e
> +"$SOURCE_DIR/../tools/intel_l3_parity" -r 0 -b 0 -s 0 -e
> +
>
> #Check that we can clear remaps
> -if [ `$SOURCE_DIR/../tools/intel_l3_parity -l | wc -l` != 1 ] ; then
> +can_clear=$("$SOURCE_DIR/../tools/intel_l3_parity" -l | wc -l)
> +if [ "$can_clear" != 1 ] ; then
> echo "Fail 2"
> - exit $IGT_EXIT_FAILURE
> + exit "$IGT_EXIT_FAILURE"
> fi
> diff --git a/tests/test_rte_check b/tests/test_rte_check
> deleted file mode 100755
> index 2a891b8..0000000
> --- a/tests/test_rte_check
> +++ /dev/null
> @@ -1,6 +0,0 @@
> -#!/bin/bash
> -
> -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> -. $SOURCE_DIR/drm_lib.sh
> -
> -exit $IGT_EXIT_SUCCESS
> diff --git a/tests/tools_test b/tests/tools_test
> index a27fb87..dd10531 100755
> --- a/tests/tools_test
> +++ b/tests/tools_test
> @@ -1,16 +1,21 @@
> -#!/bin/bash
> +#!/bin/sh
> # Test some of the most critical tools we have accidentally broken before.
> # TODO: Possibly make tests parse output
>
> -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> -. $SOURCE_DIR/drm_lib.sh
> +if [ -n "$BASH_SOURCE" ]; then
> + THIS_SCRIPT=$BASH_SOURCE
> +else
> + THIS_SCRIPT=$0
> +fi
> +
> +SOURCE_DIR=$(dirname "$THIS_SCRIPT")
> +. "$SOURCE_DIR/drm_lib.sh"
>
> # ARB_MODE has existed for many gens
> -PATH=$SOURCE_DIR/../tools:$PATH
> +PATH="$SOURCE_DIR/../tools:$PATH"
> do_or_die "intel_reg read 0x4030"
> do_or_die "intel_reg dump"
>
> # TODO: Add more tests
>
> -exit $IGT_EXIT_SUCCESS
> -
> +exit "$IGT_EXIT_SUCCESS"
--
Jani Nikula, Intel Open Source Technology Center
More information about the Intel-gfx
mailing list