fix(provider): preserve rows after malformed arguments

.github/workflows/check-docs.yml

@@ -9,8 +9,8 @@ on:
   workflow_dispatch:
 
 concurrency:
-  group: pages
-  cancel-in-progress: false
+  group: docs-${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
 
 jobs:
   build:

CHANGELOG.md

@@ -9,6 +9,7 @@
 
 ### Fixed
 
+- Echo data providers keep `#` and parentheses, and report malformed rows without dropping later rows. Quoted values keep carriage returns on Windows (#1419)
 - Homepage logo follows reduced-motion preferences and stops tracking the pointer after navigation
 - Primary buttons use dark text in both themes, including hover and active states
 - Quickstart commands match install.sh, npm project and npm global installs

docs/data-providers.md

@@ -81,7 +81,9 @@ function provider_function() {
 
 > **Note**: The previous variant of using `echo` to define data within a data
 > provider is still supported but deprecated, as it does not support empty values or
-> values containing spaces. Use `bashunit::data_set` instead.
+> values containing spaces. Use `bashunit::data_set` instead. A `#` word or literal
+> parentheses no longer abort a row. An unclosed quote fails only that row,
+> with its provider name and row number; later rows still run.
 
 ## Interpolating arguments in test names
 

src/runner/exec.sh

@@ -215,14 +215,15 @@ function bashunit::runner::call_test_functions() {
       continue
     fi
 
-    local data
+    local data provider_row=0
     for data in "${provider_data[@]+"${provider_data[@]}"}"; do
       if [ "${_BASHUNIT_STOP_REQUESTED:-false}" = true ]; then
         break
       fi
+      provider_row=$((provider_row + 1))
       parsed_data=()
       parsed_data_count=0
-      local transport_error=""
+      local transport_error="" parse_error=false
       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
@@ -250,14 +251,26 @@ function bashunit::runner::call_test_functions() {
             transport_error="arguments could not be read"
           fi
         else
-          transport_error="arguments could not be written"
+          if [ "$?" -eq 2 ]; then
+            parse_error=true
+            transport_error="row $provider_row could not be parsed: $data"
+          else
+            transport_error="arguments could not be written"
+          fi
         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"
+        if [ "$parse_error" = true ]; then
+          bashunit::runner::halt_if_stop_on_failure
+          if bashunit::env::is_stop_on_failure_enabled; then
+            break
+          fi
+          continue
+        fi
         break
       fi
       _test_ordinal=$((_test_ordinal + 1))

src/runner/provider.sh

@@ -23,39 +23,37 @@ function bashunit::runner::parse_data_provider_args() {
     has_metachar=true
   fi
 
-  # An odd run of trailing backslashes escapes the `)` the eval below adds, so
-  # eval sees `args=(C:\)` -- unterminated, a SYNTAX error rather than a failed
-  # command. Inside a command substitution, which is how exec.sh calls this,
-  # that kills the subshell: no output, and the argument reaches the test unset
-  # instead of as `C:` (#1134). The fallback parser handles it, so skip eval.
+  # An odd trailing backslash escapes eval's closing parenthesis.
   local trailing="${input##*[!\\]}"
   if [ $((${#trailing} % 2)) -eq 1 ]; then
     has_metachar=true
   fi
 
-  # Try eval first (needed for $'...' from printf '%q'), unless metacharacters present
-  if [ "$has_metachar" = false ] && eval "args=($input)" 2>/dev/null; then
-    # Check if args has elements after eval
-    args_count=0
-    local _tmp arg
-    for _tmp in ${args+"${args[@]}"}; do args_count=$((args_count + 1)); done
-    if [ "$args_count" -gt 0 ]; then
-      # Successfully parsed - remove sentinel if present
-      local last_idx=$((args_count - 1))
-      if [ -z "${args[$last_idx]}" ]; then
-        unset 'args[$last_idx]'
-      fi
-      # Print args and return early
-      for arg in "${args[@]+"${args[@]}"}"; do
-        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
+  # Syntax errors in eval exit its shell; serialize there so expansions run only once.
+  if [ "$has_metachar" = false ] && (
+    local -a eval_args=()
+    local eval_args_count=0
+    local arg
+    # MSYS reparses compound array assignments and drops expanded carriage returns.
+    eval 'for arg in '"$input"'; do
+      eval_args[eval_args_count]="$arg"
+      eval_args_count=$((eval_args_count + 1))
+    done' 2>/dev/null || exit 1
+    [ "$eval_args_count" -gt 0 ] || exit 1
+    local last_idx=$((eval_args_count - 1))
+    if [ -z "${eval_args[$last_idx]}" ]; then
+      unset 'eval_args[$last_idx]'
     fi
+    for arg in "${eval_args[@]+"${eval_args[@]}"}"; do
+      if [ "$output_format" = nul ]; then
+        printf '%s\0' "$arg"
+      else
+        encoded_arg="$(bashunit::helper::encode_base64 "${arg}")"
+        printf '%s\n' "$encoded_arg"
+      fi
+    done
+  ); then
+    return 0
   fi
 
   # Fallback: parse args from the input string into an array, respecting quotes and escapes
@@ -110,6 +108,9 @@ function bashunit::runner::parse_data_provider_args() {
       current_arg="$current_arg$char"
     fi
   done
+  if [ "$in_quotes" = true ]; then
+    return 2
+  fi
   args[args_count]="$current_arg"
   args_count=$((args_count + 1))
   # Remove all trailing empty strings

tests/acceptance/bashunit_provider_transport_test.sh

@@ -245,3 +245,82 @@ TEST
   done
   assert_same 0 "$leftover"
 }
+
+# @data_provider provide_transport_modes
+function test_echo_hash_rows_keep_the_header_and_summary_counts() {
+  local mode="$1" dir
+  dir="$(bashunit::temp_dir)"
+  cat >"$dir/hash_test.sh" <<'TEST'
+function provide_hash_rows() { echo 'color #fff'; echo 'price 5'; echo 'size 10'; }
+# @data_provider provide_hash_rows
+function test_hash_words() { assert_same 2 "$#"; }
+TEST
+  local output code=0
+  output=$(./bashunit "$mode" --skip-env-file --no-color "$dir/hash_test.sh" 2>&1) || code=$?
+  assert_same 0 "$code"
+  assert_contains 'Tests: 3' "$output"
+  assert_contains '3 passed' "$output"
+  assert_contains '3 total' "$output"
+}
+
+# @data_provider provide_transport_modes
+function test_a_malformed_provider_row_does_not_drop_later_rows() {
+  local mode="$1" dir
+  dir="$(bashunit::temp_dir)"
+  cat >"$dir/malformed_test.sh" <<'TEST'
+function provide_malformed_rows() { echo 'before 1'; echo 'broken "quote'; echo 'after 2'; }
+# @data_provider provide_malformed_rows
+function test_malformed_words() { assert_same 2 "$#"; }
+TEST
+  local output code=0
+  output=$(./bashunit "$mode" --skip-env-file --no-color \
+    --report-json "$dir/report.json" "$dir/malformed_test.sh" 2>&1) || code=$?
+  assert_same 1 "$code"
+  assert_contains "data provider 'provide_malformed_rows' row 2 could not be parsed: broken \"quote" "$output"
+  assert_not_contains 'arguments could not be written' "$output"
+  assert_contains 'Tests: 3' "$output"
+  assert_contains '2 passed' "$output"
+  assert_contains '1 failed' "$output"
+  assert_contains '3 total' "$output"
+  assert_file_contains "$dir/report.json" '"total": 3, "passed": 2, "failed": 1'
+}
+
+# @data_provider provide_transport_modes
+function test_echo_provider_expansion_runs_only_once_per_row() {
+  local mode="$1" dir
+  dir="$(bashunit::temp_dir)"
+  cat >"$dir/expansion_test.sh" <<'TEST'
+function expand_provider_word() { printf x >>"$PROVIDER_EXPANSION_CALLS"; printf expanded; }
+function provide_expansion_rows() { echo 'value `expand_provider_word`'; }
+# @data_provider provide_expansion_rows
+function test_expansion_words() { assert_same 2 "$#"; assert_same expanded "$2"; }
+TEST
+  local output code=0
+  output=$(PROVIDER_EXPANSION_CALLS="$dir/calls" ./bashunit "$mode" --skip-env-file --no-color \
+    "$dir/expansion_test.sh" 2>&1) || code=$?
+  assert_same 0 "$code"
+  assert_same x "$(<"$dir/calls")"
+}
+
+# @data_provider provide_transport_modes
+function test_a_malformed_provider_row_honors_stop_on_failure() {
+  local mode="$1" dir
+  dir="$(bashunit::temp_dir)"
+  cat >"$dir/stop_parse_test.sh" <<'TEST'
+function provide_stop_parse_rows() { echo 'broken "quote'; echo 'after 2'; }
+function tear_down_after_script() { printf done >"$STOP_PARSE_TEARDOWN"; }
+# @data_provider provide_stop_parse_rows
+function test_stop_parse_words() { printf ran >"$STOP_PARSE_BODY"; assert_same 2 "$#"; }
+TEST
+  local output code=0
+  output=$(STOP_PARSE_BODY="$dir/body" STOP_PARSE_TEARDOWN="$dir/teardown" \
+    ./bashunit "$mode" --stop-on-failure --skip-env-file --no-color \
+    --report-json "$dir/report.json" "$dir/stop_parse_test.sh" 2>&1) || code=$?
+  assert_same 1 "$code"
+  assert_contains "data provider 'provide_stop_parse_rows' row 1 could not be parsed" "$output"
+  assert_contains '1 failed' "$output"
+  assert_contains '1 total' "$output"
+  assert_file_not_exists "$dir/body"
+  assert_file_contains "$dir/teardown" "done"
+  assert_file_contains "$dir/report.json" '"total": 1, "passed": 0, "failed": 1'
+}

tests/acceptance/install_test.sh

@@ -233,7 +233,7 @@ fi
 case "$INSTALL_TEST_DOWNLOAD_RESULT" in
   failure) printf 'partial download\n' >"$target"; exit 22 ;;
   empty) : >"$target" ;;
-  *) printf 'new binary\n' >"$target" ;;
+  *) printf '#!/bin/sh\nexit 0\n' >"$target" ;;
 esac
 SH
   chmod +x shims/curl
@@ -249,7 +249,7 @@ SH
 
   if [ "$result" = success ]; then
     assert_successful_code "" "" "$status"
-    assert_same 'new binary' "$(cat destination/bashunit)"
+    assert_same "$(printf '#!/bin/sh\nexit 0')" "$(cat destination/bashunit)"
     assert_is_file_executable destination/bashunit
     assert_not_empty "$(find destination/bashunit -perm 0744 -print)"
   else

tests/functional/provider_test.sh

@@ -188,3 +188,44 @@ function test_provider_eval_does_not_assign_in_runner() {
   assert_same changed "$1"
   assert_same unset "${PROVIDER_EVAL_SIDE_EFFECT-unset}"
 }
+
+function provide_echo_hash_and_parentheses() {
+  echo 'color #fff'
+  echo 'shape (round)'
+  echo 'size 10'
+}
+
+# @data_provider provide_echo_hash_and_parentheses
+function test_echo_provider_keeps_hash_and_parentheses_words() {
+  assert_same 2 "$#"
+  case "$1" in
+  color) assert_same '#fff' "$2" ;;
+  shape) assert_same '(round)' "$2" ;;
+  size) assert_same 10 "$2" ;;
+  esac
+}
+
+function provide_quoted_hash_and_parentheses() {
+  bashunit::data_set hash $'first\v#fff'
+  bashunit::data_set parentheses $'first\v(round)'
+  bashunit::data_set hash_cr $'first\r#fff'
+  bashunit::data_set parentheses_cr $'first\r(round)'
+}
+
+# @data_provider provide_quoted_hash_and_parentheses
+function test_ansi_quoted_provider_values_keep_control_characters_next_to_metacharacters() {
+  assert_same 2 "$#"
+  case "$1" in
+  hash) assert_same $'first\v#fff' "$2" ;;
+  parentheses) assert_same $'first\v(round)' "$2" ;;
+  hash_cr) assert_same $'first\r#fff' "$2" ;;
+  parentheses_cr) assert_same $'first\r(round)' "$2" ;;
+  esac
+}
+
+function test_ansi_quoted_parser_values_keep_carriage_returns_next_to_metacharacters() {
+  local hash_input="\$'first\\r#fff' ''"
+  local parentheses_input="\$'first\\r(round)' ''"
+  assert_same "Zmlyc3QNI2ZmZg==" "$(bashunit::runner::parse_data_provider_args "$hash_input")"
+  assert_same "Zmlyc3QNKHJvdW5kKQ==" "$(bashunit::runner::parse_data_provider_args "$parentheses_input")"
+}