On Thu, Aug 13, 2020 at 10:30 AM +0200, Andrew Jones <drjones@xxxxxxxxxx> wrote:
> On Wed, Aug 12, 2020 at 11:27:04AM +0200, Marc Hartmayer wrote:
>> This allows us, for example, to auto generate a new test case based on
>> an existing test case.
>>
>> Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxx>
>> ---
>> run_tests.sh | 2 +-
>> scripts/common.bash | 13 +++++++++++++
>> scripts/mkstandalone.sh | 2 +-
>> 3 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/run_tests.sh b/run_tests.sh
>> index 24aba9cc3a98..23658392c488 100755
>> --- a/run_tests.sh
>> +++ b/run_tests.sh
>> @@ -160,7 +160,7 @@ trap "wait; exit 130" SIGINT
>> # preserve stdout so that process_test_output output can write TAP to it
>> exec 3>&1
>> test "$tap_output" == "yes" && exec > /dev/null
>> - for_each_unittest $config run_task
>> + for_each_unittest $config run_task arch_cmd
>
> Let's just require that arch cmd hook be specified by the "$arch_cmd"
> variable. Then we don't need to pass it to for_each_unittest.
Where is it then specified?
>
>> ) | postprocess_suite_output
>>
>> # wait until all tasks finish
>> diff --git a/scripts/common.bash b/scripts/common.bash
>> index f9c15fd304bd..62931a40b79a 100644
>> --- a/scripts/common.bash
>> +++ b/scripts/common.bash
>> @@ -1,8 +1,15 @@
>> +function arch_cmd()
>> +{
>> + # Dummy function, can be overwritten by architecture dependent
>> + # code
>> + return
>> +}
>
> This dummy function appears unused and can be dropped.
So what is then used if the function is not defined by the architecture
specific code?
>
>>
>> function for_each_unittest()
>> {
>> local unittests="$1"
>> local cmd="$2"
>> + local arch_cmd="${3-}"
>> local testname
>> local smp
>> local kernel
>> @@ -19,6 +26,9 @@ function for_each_unittest()
>> if [[ "$line" =~ ^\[(.*)\]$ ]]; then
>> if [ -n "${testname}" ]; then
>> "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>> + if [ "${arch_cmd}" ]; then
>> + "${arch_cmd}" "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>> + fi
>
> Rather than assuming we should run both $cmd ... and $arch_cmd $cmd ...,
> let's just run $arch_cmd $cmd ..., when it exists. If $arch_cmd wants to
> run $cmd ... first, then it can do so itself.
Yep, makes sense.
>
>> fi
>> testname=${BASH_REMATCH[1]}
>> smp=1
>> @@ -49,6 +59,9 @@ function for_each_unittest()
>> done
>> if [ -n "${testname}" ]; then
>> "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>> + if [ "${arch_cmd}" ]; then
>> + "${arch_cmd}" "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>> + fi
>> fi
>> exec {fd}<&-
>> }
>> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
>> index cefdec30cb33..3b18c0cf090b 100755
>> --- a/scripts/mkstandalone.sh
>> +++ b/scripts/mkstandalone.sh
>> @@ -128,4 +128,4 @@ fi
>>
>> mkdir -p tests
>>
>> -for_each_unittest $cfg mkstandalone
>> +for_each_unittest $cfg mkstandalone arch_cmd
>> --
>> 2.25.4
>>
>
> In summary, I think this patch should just be
>
> diff --git a/scripts/common.bash b/scripts/common.bash
> index 9a6ebbd7f287..b409b0529ea6 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -17,7 +17,7 @@ function for_each_unittest()
>
> while read -r -u $fd line; do
> if [[ "$line" =~ ^\[(.*)\]$ ]]; then
> - "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> + "$arch_cmd" "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
If we remove the $arch_cmd variable we can directly use:
arch_cmd "$cmd" …
> testname=${BASH_REMATCH[1]}
> smp=1
> kernel=""
> @@ -45,6 +45,6 @@ function for_each_unittest()
> timeout=${BASH_REMATCH[1]}
> fi
> done
> - "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> + "$arch_cmd" "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> exec {fd}<&-
> }
>
>
> Thanks,
> drew
>
--
Kind regards / Beste Grüße
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
