From e66f6878b59ac16f049cd04f8c05f5500450fe06 Mon Sep 17 00:00:00 2001 From: Fabian Boehm Date: Fri, 27 Dec 2024 20:54:43 +0100 Subject: [PATCH] Make tests usable with path with spaces This is somewhat subtle: The #RUN line in a littlecheck file will be run by a posix shell, which means the substitutions will also be mangled by it. Now, we *have* shell-quoted them, but unfortunately what we need is to quote them for inside a pre-existing layer of quotes, e.g. # RUN: fish -C 'set -g fish %fish' here, %fish can't be replaced with `'path with spaces/fish'`, because that ends up as # RUN: fish -C 'set -g fish 'path with spaces/fish'' which is just broken. So instead, we pass it as a variable to that fish: # RUN: fish=%fish fish... In addition, we need to not mangle the arguments in our test_driver. For that, because we insist on posix shell, which has only one array, and we source a file, we *need* to stop having that file use arguments. Which is okay - test_env.sh could previously be used to start a test, and now it no longer can because that is test_*driver*.sh's job. For the interactive tests, it's slightly different: pexpect.spawn(foo) is sensitive to shell metacharacters like space. So we shell-quote it. But if you pass any args to pexpect.spawn, it no longer uses a shell, and so we cannot shell-quote it. There could be a better way to fix this? --- tests/checks/basic.fish | 2 +- tests/checks/broken-config.fish | 2 +- tests/checks/cd.fish | 2 +- tests/checks/check-all-fish-files.fish | 2 +- tests/checks/check-completions.fish | 2 +- tests/checks/check-translations.fish | 2 +- tests/checks/command-not-found.fish | 2 +- tests/checks/complete-group-order.fish | 2 +- tests/checks/complete.fish | 2 +- tests/checks/create-base-directories.fish | 2 +- tests/checks/default-setup-path.fish | 2 +- tests/checks/exec.fish | 2 +- tests/checks/expansion.fish | 2 +- .../features-ampersand-nobg-in-token1.fish | 2 +- tests/checks/fish_exit.fish | 2 +- tests/checks/indent.fish | 2 +- tests/checks/init-unreadable-cwd.fish | 2 +- tests/checks/invocation.fish | 2 +- tests/checks/locale.fish | 2 +- tests/checks/loops.fish | 2 +- tests/checks/no-execute.fish | 2 +- tests/checks/rc-returned.fish | 2 +- tests/checks/read.fish | 4 ++-- tests/checks/return.fish | 4 ++-- tests/checks/set.fish | 7 +++---- tests/checks/sigint.fish | 2 +- tests/checks/signal.fish | 2 +- tests/checks/status-value.fish | 2 +- tests/checks/switch.fish | 4 +++- tests/checks/symlinks-not-overwritten.fish | 2 +- tests/checks/syntax-error-location.fish | 2 +- tests/checks/time.fish | 2 +- tests/checks/umask.fish | 2 +- tests/checks/vars_as_commands.fish | 2 +- tests/pexpect_helper.py | 5 +++++ tests/pexpects/private_mode.py | 2 +- tests/test_driver.sh | 7 ++----- tests/test_env.sh | 21 ------------------- 38 files changed, 48 insertions(+), 66 deletions(-) diff --git a/tests/checks/basic.fish b/tests/checks/basic.fish index 64264fab9..fb07342a6 100644 --- a/tests/checks/basic.fish +++ b/tests/checks/basic.fish @@ -1,4 +1,4 @@ -# RUN: %fish -C 'set -g fish %fish' %s +# RUN: fish=%fish %fish %s # # Test function, loops, conditionals and some basic elements # diff --git a/tests/checks/broken-config.fish b/tests/checks/broken-config.fish index 7899a8169..8d409e7c0 100644 --- a/tests/checks/broken-config.fish +++ b/tests/checks/broken-config.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -g fish %fish' %s +#RUN: fish=%fish %fish %s begin set -l dir (dirname (status -f)) set -gx XDG_CONFIG_HOME $dir/broken-config/ diff --git a/tests/checks/cd.fish b/tests/checks/cd.fish index 56e7f4397..9d7ac8dfa 100644 --- a/tests/checks/cd.fish +++ b/tests/checks/cd.fish @@ -1,4 +1,4 @@ -# RUN: %fish -C 'set -g fish %fish' %s +# RUN: fish=%fish %fish %s set -g fish (realpath $fish) diff --git a/tests/checks/check-all-fish-files.fish b/tests/checks/check-all-fish-files.fish index b6531b06f..7281e7631 100644 --- a/tests/checks/check-all-fish-files.fish +++ b/tests/checks/check-all-fish-files.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -l fish %fish' %s +#RUN: fish=%fish %fish %s # disable on CI ASAN because it's suuuper slow #REQUIRES: test -z "$FISH_CI_SAN" # Test ALL THE FISH FILES diff --git a/tests/checks/check-completions.fish b/tests/checks/check-completions.fish index 27ce6fe6f..50c4fa166 100644 --- a/tests/checks/check-completions.fish +++ b/tests/checks/check-completions.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -l fish %fish' %s +#RUN: fish=%fish %fish %s # disable on CI ASAN because it's suuuper slow #REQUIRES: test -z "$FISH_CI_SAN" # Test all completions where the command exists diff --git a/tests/checks/check-translations.fish b/tests/checks/check-translations.fish index ba34ab2ad..2ee71799a 100644 --- a/tests/checks/check-translations.fish +++ b/tests/checks/check-translations.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -l fish %fish' %s +#RUN: fish=%fish %fish %s #REQUIRES: msgfmt --help set -l fail_count 0 diff --git a/tests/checks/command-not-found.fish b/tests/checks/command-not-found.fish index e243c0657..d3223d774 100644 --- a/tests/checks/command-not-found.fish +++ b/tests/checks/command-not-found.fish @@ -1,4 +1,4 @@ -# RUN: %fish -C 'set -g fish %fish' %s | %fish %filter-control-sequences +#RUN: fish=%fish %fish %s | %fish %filter-control-sequences set -g PATH $fish -c "nonexistent-command-1234 banana rama" #CHECKERR: fish: Unknown command: nonexistent-command-1234 diff --git a/tests/checks/complete-group-order.fish b/tests/checks/complete-group-order.fish index 07c90aca5..49df4524c 100644 --- a/tests/checks/complete-group-order.fish +++ b/tests/checks/complete-group-order.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -l fish %fish' %s +#RUN: fish=%fish %fish %s function fooc; true; end; diff --git a/tests/checks/complete.fish b/tests/checks/complete.fish index e4017be66..35f190bc0 100644 --- a/tests/checks/complete.fish +++ b/tests/checks/complete.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -l fish %fish' %s +#RUN: fish=%fish %fish %s function complete_test_alpha1 echo $argv end diff --git a/tests/checks/create-base-directories.fish b/tests/checks/create-base-directories.fish index 1dc97891d..b8ed4e475 100644 --- a/tests/checks/create-base-directories.fish +++ b/tests/checks/create-base-directories.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -l fish %fish' %s +#RUN: fish=%fish %fish %s # Set a XDG_CONFIG_HOME with both pre-existing and non-existing directories. set -l dir (mktemp -d) diff --git a/tests/checks/default-setup-path.fish b/tests/checks/default-setup-path.fish index 8345abcae..dbeb29386 100644 --- a/tests/checks/default-setup-path.fish +++ b/tests/checks/default-setup-path.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -g fish %fish' %s +#RUN: fish=%fish %fish %s if command -q getconf # (no env -u, some systems don't support that) diff --git a/tests/checks/exec.fish b/tests/checks/exec.fish index ed7d8d5a2..6868f1dbd 100644 --- a/tests/checks/exec.fish +++ b/tests/checks/exec.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -l fish %fish' %s +# RUN: fish=%fish %fish %s exec cat \n' ($fish -c ' $f[a]' 2>&1) diff --git a/tests/checks/features-ampersand-nobg-in-token1.fish b/tests/checks/features-ampersand-nobg-in-token1.fish index bc352bb9e..b06e39d63 100644 --- a/tests/checks/features-ampersand-nobg-in-token1.fish +++ b/tests/checks/features-ampersand-nobg-in-token1.fish @@ -1,4 +1,4 @@ -#RUN: %fish --features=ampersand-nobg-in-token -C 'set -g fish_indent %fish_indent' %s +#RUN: fish_indent=%fish_indent %fish --features=ampersand-nobg-in-token %s echo no&background # CHECK: no&background diff --git a/tests/checks/fish_exit.fish b/tests/checks/fish_exit.fish index 77d063c3d..ac07ea20a 100644 --- a/tests/checks/fish_exit.fish +++ b/tests/checks/fish_exit.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -g fish %fish' %s +#RUN: fish=%fish %fish %s # fish_exit fires successfully. echo 'function do_exit --on-event fish_exit; echo "fish_exiting $fish_pid"; end' > /tmp/test_exit.fish diff --git a/tests/checks/indent.fish b/tests/checks/indent.fish index 9648042cd..fc29d1a04 100644 --- a/tests/checks/indent.fish +++ b/tests/checks/indent.fish @@ -1,4 +1,4 @@ -# RUN: %fish -C 'set -g fish_indent %fish_indent' %s +# RUN: fish_indent=%fish_indent %fish %s # Test file for fish_indent # Note that littlecheck ignores leading whitespace, so we have to use {{ }} to explicitly match it. diff --git a/tests/checks/init-unreadable-cwd.fish b/tests/checks/init-unreadable-cwd.fish index c30843a22..3710498bd 100644 --- a/tests/checks/init-unreadable-cwd.fish +++ b/tests/checks/init-unreadable-cwd.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -g fish %fish' %s +# RUN: fish=%fish %fish %s # Test that fish doesn't crash if cwd is unreadable at the start (#6597) set -l oldpwd $PWD diff --git a/tests/checks/invocation.fish b/tests/checks/invocation.fish index 4e67615c9..d602e1cc1 100644 --- a/tests/checks/invocation.fish +++ b/tests/checks/invocation.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -l fish %fish' %s | %fish %filter-control-sequences +#RUN: fish=%fish %fish %s | %fish %filter-control-sequences $fish -c "echo 1.2.3.4." # CHECK: 1.2.3.4. diff --git a/tests/checks/locale.fish b/tests/checks/locale.fish index 5f8cf6290..7b8d0d526 100644 --- a/tests/checks/locale.fish +++ b/tests/checks/locale.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C "set fish %fish" %s +# RUN: fish=%fish %fish %s # This hangs when running on github actions with tsan for unknown reasons, # see #7934. #REQUIRES: test -z "$GITHUB_WORKFLOW" diff --git a/tests/checks/loops.fish b/tests/checks/loops.fish index aea9d7e3a..bd4c3ada6 100644 --- a/tests/checks/loops.fish +++ b/tests/checks/loops.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -g fish %fish' %s +#RUN: fish=%fish %fish %s function never_runs while false diff --git a/tests/checks/no-execute.fish b/tests/checks/no-execute.fish index 1e5f54e01..e917575c0 100644 --- a/tests/checks/no-execute.fish +++ b/tests/checks/no-execute.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -l fish %fish' %s +#RUN: fish=%fish %fish %s # Test that fish -n doesn't check for command existence - function autoloading throws a wrench in that. echo "type foo" | $fish -n diff --git a/tests/checks/rc-returned.fish b/tests/checks/rc-returned.fish index 240e28ea1..eba2ffdb4 100644 --- a/tests/checks/rc-returned.fish +++ b/tests/checks/rc-returned.fish @@ -1,2 +1,2 @@ -#RUN: %fish -c '%fish -c false; echo RC: $status' +#RUN: fish=%fish %fish -c '$fish -c false; echo RC: $status' # CHECK: RC: 1 diff --git a/tests/checks/read.fish b/tests/checks/read.fish index 0355f30cb..845a80576 100644 --- a/tests/checks/read.fish +++ b/tests/checks/read.fish @@ -1,4 +1,4 @@ -# RUN: %fish -C "set -g fish %fish; set -g filter_ctrls %fish %filter-control-sequences" %s +# RUN: fish=%fish filter_ctrls=%filter-control-sequences %fish %s # Set term again explicitly to ensure behavior. set -gx TERM xterm # Read with no vars is not an error @@ -248,7 +248,7 @@ if test (string length "$x") -ne $fish_read_limit end # Confirm reading non-interactively works -- \#4206 regression -echo abc\ndef | $fish -i -c 'read a; read b; set --show a; set --show b' | $filter_ctrls +echo abc\ndef | $fish -i -c 'read a; read b; set --show a; set --show b' | $fish $filter_ctrls #CHECK: $a: set in global scope, unexported, with 1 elements #CHECK: $a[1]: |abc| #CHECK: $b: set in global scope, unexported, with 1 elements diff --git a/tests/checks/return.fish b/tests/checks/return.fish index 8b6232d7f..1cdcc3ec1 100644 --- a/tests/checks/return.fish +++ b/tests/checks/return.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -l fish %fish; set -l filter_ctrls %fish %filter-control-sequences' %s +#RUN: fish=%fish filter_ctrls=%filter-control-sequences %fish %s # Some tests of the "return" builtin. $fish -c 'return 5' @@ -21,7 +21,7 @@ begin # but not bar echo $status # CHECK: 69 -end | $filter_ctrls +end | $fish $filter_ctrls # Verify negative return values don't cause UB and never map to 0 function empty_return diff --git a/tests/checks/set.fish b/tests/checks/set.fish index a0ec8fc2c..d733dcfd1 100644 --- a/tests/checks/set.fish +++ b/tests/checks/set.fish @@ -1,5 +1,4 @@ -# Explicitly overriding HOME/XDG_CONFIG_HOME is only required if not invoking via `make test` -# RUN: env FISH=%fish %fish -C 'set -l filter_ctrls %fish %filter-control-sequences' %s +# RUN: env FISH=%fish filter_ctrls=%filter-control-sequences %fish %s # Environment variable tests # Test if variables can be properly set @@ -368,7 +367,7 @@ begin env SHLVL=" 3" $FISH -ic 'echo SHLVL: $SHLVL' # CHECK: SHLVL: 4 # CHECK: SHLVL: 4 -end | $filter_ctrls +end | $FISH $filter_ctrls # Non-interactive fish doesn't touch $SHLVL env SHLVL=2 $FISH -c 'echo SHLVL: $SHLVL' @@ -528,7 +527,7 @@ $FISH -c 'set -S EDITOR' | string match -r -e 'global|universal' # When the variable has been changed outside of fish we accept it. # CHECK: $EDITOR: set in global scope, exported, with 1 elements # CHECK: $EDITOR: set in universal scope, exported, with 2 elements -sh -c "EDITOR='vim -g' $FISH -c "'\'set -S EDITOR\'' | string match -r -e 'global|universal' +sh -c 'EDITOR=vim "$@" -c "set -S EDITOR"' uselessargument $FISH | string match -r -e 'global|universal' # Verify behavior of `set --show` given an invalid var name set --show 'argle bargle' diff --git a/tests/checks/sigint.fish b/tests/checks/sigint.fish index 4bd0d3299..06618fee3 100644 --- a/tests/checks/sigint.fish +++ b/tests/checks/sigint.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C "set -g helper %fish_test_helper; set -g fish %fish" %s +#RUN: fish=%fish helper=%fish_test_helper %fish %s #REQUIRES: command -v %fish_test_helper # Check that nohup is propagated. diff --git a/tests/checks/signal.fish b/tests/checks/signal.fish index b203065e7..249d1d5f8 100644 --- a/tests/checks/signal.fish +++ b/tests/checks/signal.fish @@ -1,4 +1,4 @@ -# RUN: env fish_test_helper=%fish_test_helper %fish -C 'set -l fish %fish' %s +# RUN: env fish_test_helper=%fish_test_helper fish=%fish %fish %s #REQUIRES: command -v %fish_test_helper $fish -c 'function main; exit 4; true; end; main' diff --git a/tests/checks/status-value.fish b/tests/checks/status-value.fish index ce02a45ad..a9efb7ea8 100644 --- a/tests/checks/status-value.fish +++ b/tests/checks/status-value.fish @@ -1,4 +1,4 @@ -# RUN: %fish -C 'set -g fish %fish' %s +#RUN: fish=%fish %fish %s # Empty commands should be 123 set empty_var diff --git a/tests/checks/switch.fish b/tests/checks/switch.fish index 6623b4c75..7e1d56b10 100644 --- a/tests/checks/switch.fish +++ b/tests/checks/switch.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C "set fish %fish" %s +# RUN: fish=%fish %fish %s # Check that switch with an argument expanding to nothing still works. switch $foo case a @@ -117,3 +117,5 @@ begin # CHECKERR: switch (doesnotexist) # CHECKERR: ^~~~~~~~~~~~~^ end + +exit 0 diff --git a/tests/checks/symlinks-not-overwritten.fish b/tests/checks/symlinks-not-overwritten.fish index 125be076e..f4b8556af 100644 --- a/tests/checks/symlinks-not-overwritten.fish +++ b/tests/checks/symlinks-not-overwritten.fish @@ -1,5 +1,5 @@ # Explicitly overriding HOME/XDG_CONFIG_HOME is only required if not invoking via `make test` -# RUN: %fish -C 'set -g fish %fish' %s +# RUN: fish=%fish %fish %s mkdir -p $XDG_CONFIG_HOME/fish diff --git a/tests/checks/syntax-error-location.fish b/tests/checks/syntax-error-location.fish index bcc07f882..14d0df8d2 100644 --- a/tests/checks/syntax-error-location.fish +++ b/tests/checks/syntax-error-location.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -g fish %fish' %s +#RUN: fish=%fish %fish %s # A $status used as a command should not impact the location of other errors. echo 'echo foo | exec grep # this exec is not allowed! diff --git a/tests/checks/time.fish b/tests/checks/time.fish index 587ab98fe..ac78e05bc 100644 --- a/tests/checks/time.fish +++ b/tests/checks/time.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -l fish %fish' %s +#RUN: fish=%fish %fish %s time sleep 0 # These are a tad awkward because it picks the correct unit and adapts whitespace. diff --git a/tests/checks/umask.fish b/tests/checks/umask.fish index 6bc3f895f..eeeb16d20 100644 --- a/tests/checks/umask.fish +++ b/tests/checks/umask.fish @@ -1,4 +1,4 @@ -# RUN: %fish -C 'set -g fish %fish' %s +# RUN: fish=%fish %fish %s # Test the umask command. In particular the symbolic modes since they've been # broken for four years (see issue #738) at the time I added these tests. diff --git a/tests/checks/vars_as_commands.fish b/tests/checks/vars_as_commands.fish index 0c74fa747..363cbc0a8 100644 --- a/tests/checks/vars_as_commands.fish +++ b/tests/checks/vars_as_commands.fish @@ -1,4 +1,4 @@ -#RUN: %fish -C 'set -g fish (builtin realpath %fish)' %s +#RUN: fish=%fish %fish %s # Test that using variables as command names work correctly. $EMPTY_VARIABLE diff --git a/tests/pexpect_helper.py b/tests/pexpect_helper.py index c553efef9..66332a307 100644 --- a/tests/pexpect_helper.py +++ b/tests/pexpect_helper.py @@ -159,9 +159,14 @@ class SpawnedProc(object): before giving up on some expected output. env: a string->string dictionary, describing the environment variables. """ + import shlex if name not in env: raise ValueError("'%s' variable not found in environment" % name) exe_path = env.get(name) + # HACK: If there are no args, pexpect will fail if exe_path contains any shell metachars. + # But not if there are args, in which case it probably switches spawning method? + if "args" not in kwargs: + exe_path = shlex.quote(exe_path) self.colorize = sys.stdout.isatty() or env.get("FISH_FORCE_COLOR", "0") == "1" self.messages = [] self.start_time = None diff --git a/tests/pexpects/private_mode.py b/tests/pexpects/private_mode.py index e8d8402ef..3bec2bdab 100644 --- a/tests/pexpects/private_mode.py +++ b/tests/pexpects/private_mode.py @@ -67,5 +67,5 @@ while now - start < 1: sleep(now - start) now = time.time() -sendline(r" builtin history save ; %s -c 'string join \n -- $history'" % fish_path) +sendline(r" builtin history save ; $fish -c 'string join \n -- $history'") expect_prompt("\r\n".join(reversed(recorded_history))) diff --git a/tests/test_driver.sh b/tests/test_driver.sh index bbdcedae6..069b5459b 100755 --- a/tests/test_driver.sh +++ b/tests/test_driver.sh @@ -21,9 +21,6 @@ unset CDPATH # Resolve the script now because we are going to `cd` later. fish_script="$(realpath $1)" shift 1 -script_args="${@}" -# Prevent $@ from persisting to sourced commands -shift $# 2>/dev/null die() { if test "$#" -ge 0; then @@ -86,9 +83,9 @@ export FISH_FAST_FAIL # Run the test script, but don't exec so we can clean up after it succeeds/fails. Each test is # launched directly within its TMPDIR, so that the fish tests themselves do not need to refer to # TMPDIR (to ensure their output as displayed in case of failure by littlecheck is reproducible). -if test -n "$script_args"; then +if test -n "${@}"; then (cd $TMPDIR && env HOME="$homedir" fish_test_helper="$homedir/fish_test_helper" "$fish" \ - --init-command "${fish_init_cmd}" "$fish_script" "$script_args") + --init-command "${fish_init_cmd}" "$fish_script" "${@}") else (cd $TMPDIR && env HOME="$homedir" fish_test_helper="$homedir/fish_test_helper" "$fish" \ --init-command "${fish_init_cmd}" "$fish_script") diff --git a/tests/test_env.sh b/tests/test_env.sh index 97cb0224a..18bea82f5 100755 --- a/tests/test_env.sh +++ b/tests/test_env.sh @@ -14,14 +14,6 @@ IFS="$(printf "\n\b")" # set -ex -# The first argument is the path to the script to launch; all remaining arguments are forwarded to -# the script. -if test $# -gt 0; then - target="$1" - shift 1 - target_args="${@}" -fi - die() { if test "$#" -ge 0; then printf "%s\n" "$@" 1>&2 @@ -88,16 +80,3 @@ unset LC_TERMINAL_VERSION unset TERM_PROGRAM unset TERM_PROGRAM_VERSION unset VTE_VERSION - -# If we are sourced, return without executing -if test -z ${target}; then - return 0 -fi -echo "Proceeding with target execution" - -# Otherwise execute target -("${target}" "${target_args}") -test_status="$?" - -rm -rf "$homedir" -exit "$test_status"