From 53cc240b958d74a423d0f22752c080f4e3a986db Mon Sep 17 00:00:00 2001 From: Michael Jeanson Date: Wed, 3 Jul 2019 18:09:56 -0400 Subject: [PATCH] tests: replace xargs workaround with bash array expansion Bash has a nice feature called 'Quoted array expansion' which we can use to pass parameters between functions without convoluted quote escaping. The syntax might not be extremmely clear at first though, for example: "${myarray[@]}" will be expanded to: "${myarray[1]}" "${myarray[2]}" ... "${myarray[n]}" Which can be passed as a series of parameters to a function and then converted back to a array for further use with : local params=("$@") This uses the special '$@' variable which is an array of all the parameters to a function, 'shift' can be used to skip some parameters at the beginning of the array. An array can also be expanded as a single string with the members separated by a space, with this syntax: "${myarray[*]}" Using this, I reworked the bt_diff functions to get rid of the xargs call, I had to move the expected file parameter to the beginning so it could be shifted and the rest of the parameters passed directly to CLI executable. Signed-off-by: Michael Jeanson Change-Id: Ie107c5259c9561fe158f923a53eeb0eaf7ee8b00 Reviewed-on: https://review.lttng.org/c/babeltrace/+/1627 Tested-by: jenkins Reviewed-by: Philippe Proulx --- tests/plugins/flt.utils.trimmer/test_trimming | 4 +- .../plugins/sink.ctf.fs/succeed/test_succeed | 6 +-- tests/plugins/src.ctf.fs/succeed/test_succeed | 8 ++-- tests/utils/utils.sh | 45 +++++++++---------- 4 files changed, 29 insertions(+), 34 deletions(-) diff --git a/tests/plugins/flt.utils.trimmer/test_trimming b/tests/plugins/flt.utils.trimmer/test_trimming index 28c3fd2d..1dbb72e7 100755 --- a/tests/plugins/flt.utils.trimmer/test_trimming +++ b/tests/plugins/flt.utils.trimmer/test_trimming @@ -58,9 +58,7 @@ function run_test # with_stream_msgs_cs is set to "true" or "false" by the tests. - cli_args="-c src.test-trimmer.TheSourceOfAllEvil -p 'with-stream-msgs-cs=$with_stream_msgs_cs' -c sink.text.details '--params=compact=true,with-metadata=false' '--plugin-path=$data_dir' $begin $end" - - bt_diff_cli "$cli_args" "$temp_expected" + bt_diff_cli "$temp_expected" -c src.test-trimmer.TheSourceOfAllEvil -p with-stream-msgs-cs=$with_stream_msgs_cs -c sink.text.details --params=compact=true,with-metadata=false --plugin-path=$data_dir $begin $end ok $? "$test_name" } diff --git a/tests/plugins/sink.ctf.fs/succeed/test_succeed b/tests/plugins/sink.ctf.fs/succeed/test_succeed index c04522ac..fa5f3751 100755 --- a/tests/plugins/sink.ctf.fs/succeed/test_succeed +++ b/tests/plugins/sink.ctf.fs/succeed/test_succeed @@ -59,9 +59,9 @@ test_ctf_gen_single() { converted_test_name="Converted trace '$name' gives the expected output" if [ $ret -eq 0 ]; then - bt_diff_details_ctf_single "$temp_out_trace_dir" \ - "$expect_dir/trace-$name.expect" \ - '-p with-uuid=no,with-trace-name=no,with-stream-name=no' + bt_diff_details_ctf_single "$expect_dir/trace-$name.expect" \ + "$temp_out_trace_dir" \ + "-p" "with-uuid=no,with-trace-name=no,with-stream-name=no" ok $? "$converted_test_name" else fail "$converted_test_name" diff --git a/tests/plugins/src.ctf.fs/succeed/test_succeed b/tests/plugins/src.ctf.fs/succeed/test_succeed index 9cd77ba9..4b438785 100755 --- a/tests/plugins/src.ctf.fs/succeed/test_succeed +++ b/tests/plugins/src.ctf.fs/succeed/test_succeed @@ -38,7 +38,7 @@ this_dir_build="$BT_TESTS_BUILDDIR/$this_dir_relative" succeed_trace_dir="$BT_CTF_TRACES_PATH/succeed" expect_dir="$BT_TESTS_DATADIR/$this_dir_relative" -test_ctf_common_details_args="-p with-trace-name=no,with-stream-name=no" +test_ctf_common_details_args=("-p" "with-trace-name=no,with-stream-name=no") test_ctf_gen_single() { name="$1" @@ -46,15 +46,15 @@ test_ctf_gen_single() { diag "Generating trace '$name'" bt_diff_details_ctf_gen_single "$this_dir_build/gen-trace-$name" \ "$expect_dir/trace-$name.expect" \ - "$test_ctf_common_details_args -p with-uuid=no" + "${test_ctf_common_details_args[@]}" "-p" "with-uuid=no" ok $? "Generated trace '$name' gives the expected output" } test_ctf_single() { name="$1" - bt_diff_details_ctf_single "$succeed_trace_dir/$name" \ - "$expect_dir/trace-$name.expect" "$test_ctf_common_details_args" + bt_diff_details_ctf_single "$expect_dir/trace-$name.expect" \ + "$succeed_trace_dir/$name" "${test_ctf_common_details_args[@]}" ok $? "Trace '$name' gives the expected output" } diff --git a/tests/utils/utils.sh b/tests/utils/utils.sh index 65949bd0..84c3c52b 100644 --- a/tests/utils/utils.sh +++ b/tests/utils/utils.sh @@ -103,17 +103,17 @@ BT_DEBUG_INFO_PATH="${BT_TESTS_DATADIR}/debug-info" ### Diff Functions ### -# Checks the difference between: -# -# 1. What the CLI outputs when given the arguments "$1" (passed to -# `xargs`, so they can include quoted arguments). -# 2. The file with path "$2". +# Checks the difference between the content of the file with path "$1" +# and the output of the CLI when called with the rest of arguments +# to this function. # # Returns 0 if there's no difference, and 1 if there is, also printing # said difference to the standard error. bt_diff_cli() { - local args="$1" - local expected_file="$2" + local expected_file="$1" + shift 1 + local args=("$@") + local temp_output_file local temp_diff local ret=0 @@ -124,11 +124,11 @@ bt_diff_cli() { # Run the CLI to get a detailed file. Strip any \r present due to # Windows (\n -> \r\n). "diff --string-trailing-cr" is not used since it # is not present on Solaris. - echo "$args" | xargs "$BT_TESTS_BT2_BIN" | tr -d "\r" > "$temp_output_file" + "$BT_TESTS_BT2_BIN" "${args[@]}" | tr -d "\r" > "$temp_output_file" # Compare output with expected output if ! diff -u "$temp_output_file" "$expected_file" 2>/dev/null >"$temp_diff"; then - echo "ERROR: for '$args': actual and expected outputs differ:" >&2 + echo "ERROR: for '${args[*]}': actual and expected outputs differ:" >&2 cat "$temp_diff" >&2 ret=1 fi @@ -138,25 +138,21 @@ bt_diff_cli() { return $ret } -# Checks the difference between: -# -# 1. What the CLI outputs when given the arguments: -# -# "$1" -c sink.text.details $3 -# -# 2. The file with path "$2". -# -# Parameter 3 is optional. +# Checks the difference between the content of the file with path "$1" +# and the output of the CLI when called on the directory path "$2" with +# the arguments '-c sink.text.details' and the rest of the arguments to +# this function. # # Returns 0 if there's no difference, and 1 if there is, also printing # said difference to the standard error. bt_diff_details_ctf_single() { - local trace_dir="$1" - local expected_file="$2" - local extra_details_args="${3:-}" + local expected_file="$1" + local trace_dir="$2" + shift 2 + local extra_details_args=("$@") # Compare using the CLI with `sink.text.details` - bt_diff_cli "\"$trace_dir\" -c sink.text.details $extra_details_args" "$expected_file" + bt_diff_cli "$expected_file" "$trace_dir" "-c" "sink.text.details" "${extra_details_args[@]}" } # Calls bt_diff_details_ctf_single(), except that "$1" is the path to a @@ -166,7 +162,8 @@ bt_diff_details_ctf_single() { bt_diff_details_ctf_gen_single() { local ctf_gen_prog_path="$1" local expected_file="$2" - local extra_details_args="${3:-}" + shift 2 + local extra_details_args=("$@") local temp_trace_dir local ret @@ -181,7 +178,7 @@ bt_diff_details_ctf_gen_single() { fi # Compare using the CLI with `sink.text.details` - bt_diff_details_ctf_single "$temp_trace_dir" "$expected_file" "$extra_details_args" + bt_diff_details_ctf_single "$expected_file" "$temp_trace_dir" "${extra_details_args[@]}" ret=$? rm -rf "$temp_trace_dir" return $ret -- 2.34.1