perf(runner): transfer provider args without codecs

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Unreleased
 
 ### Changed
+- Data providers pass arguments without per-argument base64 processes, preserving quoting, empty values and parser isolation (#1370)
 - JSON reports write ordinary filenames, test names and empty messages without per-field processes, preserving existing escaping (#1369)
 
 ### Fixed

src/runner/exec.sh

@@ -52,13 +52,6 @@ function bashunit::runner::order_functions_for_script() {
   _BASHUNIT_RUNNER_ORDERED_FNS_OUT="${ordered[*]+${ordered[*]}}"
 }
 
-##
-# Runs the given test functions of a script (sequentially, or one background
-# worker per test under --parallel).
-# Arguments: $1 script path, $2 space-separated test function names, already
-# filter/tag/rerun-filtered by load_test_files (never empty: the caller skips
-# the file when no function survives filtering).
-##
 function bashunit::runner::report_unusable_provider() {
   local test_file="$1"
   local fn_name="$2"
@@ -73,6 +66,14 @@ function bashunit::runner::report_unusable_provider() {
     reason="data provider '$provider' is not defined, so the test never ran"
   fi
 
+  bashunit::runner::report_provider_error "$test_file" "$fn_name" "$reason"
+}
+
+function bashunit::runner::report_provider_error() {
+  local test_file="$1"
+  local fn_name="$2"
+  local reason="$3"
+
   bashunit::state::add_tests_failed
   bashunit::console_results::print_error_test "$fn_name" "$reason"
   local _normalized_fn
@@ -95,7 +96,13 @@ function bashunit::runner::report_unusable_provider() {
   fi
 }
 
-
+##
+# Runs the given test functions of a script (sequentially, or one background
+# worker per test under --parallel).
+# Arguments: $1 script path, $2 space-separated test function names, already
+# filter/tag/rerun-filtered by load_test_files (never empty: the caller skips
+# the file when no function survives filtering).
+##
 function bashunit::runner::call_test_functions() {
   local script="$1"
   local cached_functions="${2:-}"
@@ -122,6 +129,7 @@ function bashunit::runner::call_test_functions() {
   local provider_data_count=0
   local -a parsed_data=()
   local parsed_data_count=0
+  local provider_arg_file=""
   # Monotonic within this file; names each parallel worker's .result file.
   local _test_ordinal=0
 
@@ -201,17 +209,48 @@ function bashunit::runner::call_test_functions() {
       continue
     fi
 
-    # Execute the test function for each line of data
     local data
     for data in "${provider_data[@]+"${provider_data[@]}"}"; do
       parsed_data=()
       parsed_data_count=0
+      local transport_error=""
+      if ! bashunit::env::ensure_run_output_dir; then
+        transport_error="argument storage could not be created"
+      elif [ -z "$provider_arg_file" ] || [ ! -f "$provider_arg_file" ]; then
+        local provider_arg_dir="$_BASHUNIT_RUN_OUTPUT_DIR"
+        case "$provider_arg_dir" in
+        /*) ;;
+        *) provider_arg_dir="$BASHUNIT_WORKING_DIR/$provider_arg_dir" ;;
+        esac
+        provider_arg_file="$("$MKTEMP" "$provider_arg_dir/provider-args.XXXXXXX")" ||
+          transport_error="argument storage could not be created"
+      fi
       local line
-      while IFS= read -r line; do
-        [ -z "$line" ] && continue
-        parsed_data[parsed_data_count]="$(bashunit::helper::decode_base64 "${line}")"
-        parsed_data_count=$((parsed_data_count + 1))
-      done <<<"$(bashunit::runner::parse_data_provider_args "$data")"
+      # The parser's eval must stay in a subshell so expansions cannot change runner state.
+      if [ -z "$transport_error" ]; then
+        # Bash 3 does not invert a compound command's redirect failure with `!`.
+        if (bashunit::runner::parse_data_provider_args "$data" nul) >"$provider_arg_file"; then
+          if {
+            while IFS= read -r -d '' line; do
+              parsed_data[parsed_data_count]="$line"
+              parsed_data_count=$((parsed_data_count + 1))
+            done
+          } <"$provider_arg_file"; then
+            :
+          else
+            transport_error="arguments could not be read"
+          fi
+        else
+          transport_error="arguments could not be written"
+        fi
+      fi
+      if [ -n "$transport_error" ]; then
+        _test_ordinal=$((_test_ordinal + 1))
+        _BASHUNIT_RUNNER_RESULT_ORDINAL=$_test_ordinal
+        bashunit::runner::report_provider_error \
+          "$script" "$fn_name" "data provider '$_BASHUNIT_PROVIDER_FN_OUT' $transport_error"
+        break
+      fi
       if bashunit::parallel::is_enabled && [ "$allow_test_parallel" = true ]; then
         bashunit::runner::wait_for_job_slot
         _test_ordinal=$((_test_ordinal + 1))

src/runner/provider.sh

@@ -11,6 +11,7 @@ function bashunit::runner::parse_data_provider_args() {
   local i=0
   local arg=""
   local encoded_arg
+  local output_format="${2:-base64}"
   local -a args=()
   local args_count=0
 
@@ -46,8 +47,12 @@ function bashunit::runner::parse_data_provider_args() {
       fi
       # Print args and return early
       for arg in "${args[@]+"${args[@]}"}"; do
-        encoded_arg="$(bashunit::helper::encode_base64 "${arg}")"
-        printf '%s\n' "$encoded_arg"
+        if [ "$output_format" = nul ]; then
+          printf '%s\0' "$arg"
+        else
+          encoded_arg="$(bashunit::helper::encode_base64 "${arg}")"
+          printf '%s\n' "$encoded_arg"
+        fi
       done
       return
     fi
@@ -117,10 +122,14 @@ function bashunit::runner::parse_data_provider_args() {
       break
     fi
   done
-  # Print one arg per line to stdout, base64-encoded to preserve newlines in the data
+  # Keep the original base64 stdout format for callers without a requested transport.
   local arg
   for arg in ${args+"${args[@]}"}; do
-    encoded_arg="$(bashunit::helper::encode_base64 "${arg}")"
-    printf '%s\n' "$encoded_arg"
+    if [ "$output_format" = nul ]; then
+      printf '%s\0' "$arg"
+    else
+      encoded_arg="$(bashunit::helper::encode_base64 "${arg}")"
+      printf '%s\n' "$encoded_arg"
+    fi
   done
 }

tests/acceptance/bashunit_provider_transport_test.sh

@@ -0,0 +1,137 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+function set_up_before_script() {
+  PROVIDER_TRANSPORT_FIXTURES="$(bashunit::temp_dir provider_transport)"
+}
+
+function provide_transport_setup_failures() {
+  bashunit::data_set --no-parallel mktemp
+  bashunit::data_set --parallel mktemp
+  bashunit::data_set --no-parallel directory
+  bashunit::data_set --parallel directory
+}
+
+# @data_provider provide_transport_setup_failures
+function test_provider_storage_setup_failure_is_reported_once() {
+  local mode="$1" failure="$2"
+  local dir="$PROVIDER_TRANSPORT_FIXTURES/setup_${mode#--}_$failure"
+  mkdir -p "$dir"
+  {
+    if [ "$failure" = mktemp ]; then
+      printf '%s\n' 'function fail_provider_mktemp() { return 1; }
+function set_up_before_script() { MKTEMP=fail_provider_mktemp; }'
+    else
+      printf '%s\n' 'function set_up_before_script() {
+  function bashunit::env::ensure_run_output_dir() { return 1; }
+}'
+    fi
+    printf '%s\n' 'function test_before_provider_storage_failure() { assert_same 1 1; }
+function provide_setup_failure_rows() { bashunit::data_set first; bashunit::data_set second; }
+# @data_provider provide_setup_failure_rows
+function test_row_with_unavailable_storage() {
+  printf executed >"$PROVIDER_BODY_MARKER"
+  assert_same first "${1-}"
+}'
+  } >"$dir/setup_failure_test.sh"
+
+  local output code=0
+  output="$(PROVIDER_BODY_MARKER="$dir/body" ./bashunit "$mode" \
+    --env tests/acceptance/fixtures/.env.default --report-json "$dir/report.json" \
+    "$dir/setup_failure_test.sh" 2>&1)" || code=$?
+  output="$(printf '%s' "$output" | strip_ansi)"
+
+  assert_same 1 "$code"
+  assert_contains "data provider 'provide_setup_failure_rows'" "$output"
+  assert_contains "1 passed" "$output"
+  assert_contains "1 failed" "$output"
+  assert_contains "2 total" "$output"
+  assert_contains '"total": 2, "passed": 1, "failed": 1' "$(<"$dir/report.json")"
+  assert_file_not_exists "$dir/body"
+}
+
+function provide_transport_io_failures() {
+  bashunit::data_set --no-parallel write
+  bashunit::data_set --parallel write
+  bashunit::data_set --no-parallel read
+  bashunit::data_set --parallel read
+}
+
+# @data_provider provide_transport_io_failures
+function test_provider_storage_io_failure_does_not_execute_the_test_body() {
+  local mode="$1" failure="$2"
+  local dir="$PROVIDER_TRANSPORT_FIXTURES/io_${mode#--}_$failure"
+  mkdir -p "$dir"
+  {
+    if [ "$failure" = write ]; then
+      printf '%s\n' 'function unusable_provider_storage() { printf "%s\n" "$_BASHUNIT_RUN_OUTPUT_DIR"; }
+function set_up_before_script() { MKTEMP=unusable_provider_storage; }'
+    else
+      printf '%s\n' 'function set_up_before_script() {
+  function bashunit::runner::parse_data_provider_args() {
+    rm -f "$provider_arg_file"
+    printf "%s\0" row
+  }
+}'
+    fi
+    printf '%s\n' 'function test_before_provider_io_failure() { assert_same 1 1; }
+function provide_io_failure_rows() { bashunit::data_set first; bashunit::data_set second; }
+# @data_provider provide_io_failure_rows
+function test_row_with_unreadable_storage() {
+  printf executed >"$PROVIDER_BODY_MARKER"
+  assert_same 0 "$#"
+}'
+  } >"$dir/io_failure_test.sh"
+
+  local output code=0
+  output="$(PROVIDER_BODY_MARKER="$dir/body" ./bashunit "$mode" \
+    --env tests/acceptance/fixtures/.env.default --report-json "$dir/report.json" \
+    "$dir/io_failure_test.sh" 2>&1)" || code=$?
+  output="$(printf '%s' "$output" | strip_ansi)"
+
+  assert_same 1 "$code"
+  assert_contains "data provider 'provide_io_failure_rows'" "$output"
+  assert_contains "1 passed" "$output"
+  assert_contains "1 failed" "$output"
+  assert_contains "2 total" "$output"
+  assert_contains '"total": 2, "passed": 1, "failed": 1' "$(<"$dir/report.json")"
+  assert_file_not_exists "$dir/body"
+}
+
+function provide_transport_modes() {
+  printf '%s\n' --no-parallel --parallel
+}
+
+# @data_provider provide_transport_modes
+function test_provider_rows_restore_a_scratch_directory_removed_by_the_previous_row() {
+  local mode="$1"
+  local dir="$PROVIDER_TRANSPORT_FIXTURES/restore_${mode#--}"
+  mkdir -p "$dir"
+  : >"$dir/public_files"
+  printf '%s\n' '# bashunit: no-parallel-tests
+function provide_rows_across_scratch_loss() { bashunit::data_set first; bashunit::data_set second; }
+# @data_provider provide_rows_across_scratch_loss
+function test_row_after_scratch_loss() {
+  assert_same 1 "$#"
+  assert_not_empty "${1-}"
+  printf "%s\n" "$1" >>"$PROVIDER_ROWS_MARKER"
+  if ! bashunit::check_os::is_windows; then
+    find "$_BASHUNIT_RUN_OUTPUT_DIR" -name "provider-args.*" -perm -004 -print >>"$PROVIDER_PUBLIC_FILES"
+  fi
+  if [ "${1-}" = first ]; then
+    printf "Removing scratch directory: %s\n" "$_BASHUNIT_RUN_OUTPUT_DIR"
+    bashunit::env::cleanup_run_output_dir
+  fi
+}' >"$dir/scratch_loss_test.sh"
+
+  local output code=0
+  output="$(PROVIDER_ROWS_MARKER="$dir/rows" PROVIDER_PUBLIC_FILES="$dir/public_files" \
+    ./bashunit "$mode" --env tests/acceptance/fixtures/.env.default \
+    "$dir/scratch_loss_test.sh" 2>&1)" || code=$?
+  output="$(printf '%s' "$output" | strip_ansi)"
+
+  assert_same 0 "$code"
+  assert_same $'first\nsecond' "$(<"$dir/rows")"
+  assert_empty "$(<"$dir/public_files")"
+  assert_not_contains "No such file or directory" "$output"
+}

tests/acceptance/bashunit_run_forks_test.sh

@@ -328,6 +328,45 @@ function test_reports_do_not_fork_base64_per_field() {
   assert_less_or_equal_than 16 "$calls"
 }
 
+function test_provider_arguments_do_not_fork_base64_per_value() {
+  if bashunit::check_os::is_windows; then
+    bashunit::skip "PATH shims are unreliable under Git Bash" && return
+  fi
+
+  local dir
+  dir="$(bashunit::temp_dir)"
+  local count_file="$dir/base64_calls"
+  local real_base64
+  real_base64="$(command -v base64)"
+  {
+    echo '#!/usr/bin/env bash'
+    echo "case \"\$*\" in --help) ;; *) echo x >> \"$count_file\" ;; esac"
+    echo "exec \"$real_base64\" \"\$@\""
+  } >"$dir/base64"
+  chmod +x "$dir/base64"
+
+  local fixture="$dir/provider_forks_test.sh"
+  {
+    echo 'function provide_rows() {'
+    echo '  bashunit::data_set a b'
+    echo '  bashunit::data_set c d'
+    echo '  bashunit::data_set e f'
+    echo '}'
+    echo '# @data_provider provide_rows'
+    echo 'function test_row() { assert_not_empty "$1"; assert_not_empty "$2"; }'
+  } >"$fixture"
+
+  local code=0
+  PATH="$dir:$PATH" ./bashunit --no-parallel "$fixture" >/dev/null 2>&1 || code=$?
+  assert_same 0 "$code"
+
+  local calls=0
+  if [ -f "$count_file" ]; then
+    calls="$(grep -c . "$count_file" || true)"
+  fi
+  assert_equals 0 "$calls"
+}
+
 # Regression guard for the per-test hook path. A test in a file that defines
 # `set_up` or `tear_down` used to cost five process forks: each hook minted its
 # output file with `mktemp` and removed it with `rm -f`, and the temp-owner

tests/functional/provider_test.sh

@@ -153,10 +153,6 @@ function provide_two_args_with_spaces() {
   bashunit::data_set "first test" "second test"
 }
 
-# A value ending in an odd run of backslashes escaped the `)` of the parser's
-# `eval "args=($input)"` fast path, making it a syntax error -- which kills the
-# command substitution the runner calls the parser inside, so the argument
-# reached the test unset rather than as `C:` (#1134).
 function provide_trailing_backslash() {
   # shellcheck disable=SC1003  # a lone trailing backslash is the case under test
   echo 'C:\'
@@ -167,3 +163,28 @@ function provide_trailing_backslash() {
 function test_a_value_ending_in_a_backslash_still_arrives() {
   assert_not_empty "${1-}"
 }
+
+function provide_lossless_values() {
+  bashunit::data_set "" $'unit\037separator' $'line\none\n' $'trailing \t ' "C:\\path\\" '*;|&'
+}
+
+# @data_provider provide_lossless_values
+function test_provider_values_arrive_without_record_delimiter_collisions() {
+  assert_same 6 "$#"
+  assert_same "" "$1"
+  assert_same $'unit\037separator' "$2"
+  assert_same $'line\none\n' "$3"
+  assert_same $'trailing \t ' "$4"
+  assert_same "C:\\path\\" "$5"
+  assert_same '*;|&' "$6"
+}
+
+function provide_eval_assignment() {
+  printf '%s\n' '${PROVIDER_EVAL_SIDE_EFFECT:=changed}'
+}
+
+# @data_provider provide_eval_assignment
+function test_provider_eval_does_not_assign_in_runner() {
+  assert_same changed "$1"
+  assert_same unset "${PROVIDER_EVAL_SIDE_EFFECT-unset}"
+}