From f859d16aea0d58e572edc9aaf1de3542569e10a9 Mon Sep 17 00:00:00 2001
From: Sergey Slipchenko <faergeek@gmail.com>
Date: Mon, 11 Sep 2023 21:01:00 +0400
Subject: [PATCH] fix(tests): set SHELL=sh #24941

Problem:
Some tests fail with $SHELL=fish #6172
Related: https://github.com/neovim/neovim/pull/6176

Solution:
Replace "echo -n" with "printf", because "echo" in sh may be provided
as a shell builtin, which does not accept an "-n" flag to avoid a
trailing newline (e.g. on macos). "printf" is more portable (defined by
POSIX) and it does not output a trailing newline by itself.
Fixes #6172

TODO:
Other test failures may be related to "session leader" issue: https://github.com/neovim/neovim/issues/2354
Checked by running `:terminal ./build/bin/tty-test` from Nvim with
`shell=/bin/fish` (inherited from `$SHELL`) and it indeed complains
about "process does not own the terminal". With `shell=sh` it doesn't complain. And
unsetting `$SHELL` seems to make `nvim` to fall back to `shell=sh`.

    FAILED   test/functional/terminal/tui_spec.lua @ 1017: TUI paste: terminal mode
    test/functional/terminal/tui_spec.lua:1024: Row 1 did not match.
    Expected:
      |*tty ready                                         |
      |*{1: }                                                 |
      |*                                                  |
      |                                                  |
      |{5:^^^^^^^                                           }|
      |{3:-- TERMINAL --}                                    |
      |{3:-- TERMINAL --}                                    |
    Actual:
      |*process does not own the terminal                 |
      |*                                                  |
      |*[Process exited 2]{1: }                               |
      |                                                  |
      |{5:^^^^^^^                                           }|
      |{3:-- TERMINAL --}                                    |
      |{3:-- TERMINAL --}                                    |

    To print the expect() call that would assert the current screen state, use
    screen:snapshot_util(). In case of non-deterministic failures, use
    screen:redraw_debug() to show all intermediate screen states.

    stack traceback:
        test/functional/ui/screen.lua:622: in function '_wait'
        test/functional/ui/screen.lua:352: in function 'expect'
        test/functional/terminal/tui_spec.lua:1024: in function <test/functional/terminal/tui_spec.lua:1017>

    FAILED   test/functional/terminal/tui_spec.lua @ 1551: TUI forwards :term palette colors with termguicolors
    test/functional/terminal/tui_spec.lua:1567: Row 1 did not match.
    Expected:
      |*{1:t}ty ready                                         |
      |                                                  |
      |*                                                  |
      |                                                  |
      |{2:^^^^^^^                                           }|
      |                                                  |
      |{3:-- TERMINAL --}                                    |
    Actual:
      |*{1:p}rocess does not own the terminal                 |
      |                                                  |
      |*[Process exited 2]                                |
      |                                                  |
      |{2:^^^^^^^                                           }|
      |                                                  |
      |{3:-- TERMINAL --}                                    |

    To print the expect() call that would assert the current screen state, use
    screen:snapshot_util(). In case of non-deterministic failures, use
    screen:redraw_debug() to show all intermediate screen states.

    stack traceback:
        test/functional/ui/screen.lua:622: in function '_wait'
        test/functional/ui/screen.lua:352: in function 'expect'
        test/functional/terminal/tui_spec.lua:1567: in function <test/functional/terminal/tui_spec.lua:1551>
---
 cmake/RunTests.cmake                      | 5 +++++
 test/functional/vimscript/system_spec.lua | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/cmake/RunTests.cmake b/cmake/RunTests.cmake
index d470793a27..8d5b0d2402 100644
--- a/cmake/RunTests.cmake
+++ b/cmake/RunTests.cmake
@@ -64,6 +64,11 @@ endif()
 
 set(ENV{SYSTEM_NAME} ${CMAKE_HOST_SYSTEM_NAME})  # used by test/helpers.lua.
 
+if(NOT WIN32)
+  # Tests assume POSIX "sh" and may fail if SHELL=fish. #24941 #6172
+  set(ENV{SHELL} sh)
+endif()
+
 execute_process(
   # Note: because of "-ll" (low-level interpreter mode), some modules like
   # _editor.lua are not loaded.
diff --git a/test/functional/vimscript/system_spec.lua b/test/functional/vimscript/system_spec.lua
index 762e8877ce..90aab48d61 100644
--- a/test/functional/vimscript/system_spec.lua
+++ b/test/functional/vimscript/system_spec.lua
@@ -335,12 +335,12 @@ describe('system()', function()
       if is_os('win') then
         eq("echoed\n", eval('system("echo echoed")'))
       else
-        eq("echoed", eval('system("echo -n echoed")'))
+        eq("echoed", eval('system("printf echoed")'))
       end
     end)
     it('to backgrounded command does not crash', function()
       -- This is indeterminate, just exercise the codepath. May get E5677.
-      feed_command('call system(has("win32") ? "start /b /wait cmd /c echo echoed" : "echo -n echoed &")')
+      feed_command('call system(has("win32") ? "start /b /wait cmd /c echo echoed" : "printf echoed &")')
       local v_errnum = string.match(eval("v:errmsg"), "^E%d*:")
       if v_errnum then
         eq("E5677:", v_errnum)