[RFC] test: explicitly set 'shell' to sh #6176
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nhooyr
changed the title
justinmk
reviewed
Feb 26, 2017
test/functional/eval/system_spec.lua
Outdated
| @@ -178,7 +178,7 @@ describe('system()', function() | |||
|
|
|||
| describe('passing no input', function() | |||
| it('returns the program output', function() | |||
| eq("echoed", eval('system("echo -n echoed")')) | |||
| eq("echoed\n", eval('system("echo echoed")')) | |||
There was a problem hiding this comment.
The -n was to avoid a newline. Try printf instead, it's POSIX-specified and therefore much more reliable that echo (which we really shouldn't be using anyway).
eq("echoed", eval('system("printf echoed")'))
|
Done, I've also included a fix for #6177. |
justinmk
reviewed
Feb 26, 2017
| @@ -21,7 +21,7 @@ local start_dir = lfs.currentdir() | |||
| -- XXX: NVIM_PROG takes precedence, QuickBuild sets it. | |||
| local nvim_prog = os.getenv('NVIM_PROG') or os.getenv('NVIM_PRG') or 'build/bin/nvim' | |||
| local nvim_argv = {nvim_prog, '-u', 'NONE', '-i', 'NONE', '-N', | |||
| '--cmd', 'set shortmess+=I background=light noswapfile noautoindent laststatus=1 undodir=. directory=. viewdir=. backupdir=.', | |||
| '--cmd', 'set shell=sh shortmess+=I background=light noswapfile noautoindent laststatus=1 undodir=. directory=. viewdir=. backupdir=.', | |||
There was a problem hiding this comment.
Instead of this, try modifying the clear() function in this file so that it clears $SHELL. Then the Windows build won't break.
justinmk
reviewed
Feb 26, 2017
| @@ -8,7 +8,7 @@ describe("shell command :!", function() | |||
| before_each(function() | |||
| session.clear() | |||
| screen = child_session.screen_setup(0, '["'..session.nvim_prog.. | |||
| '", "-u", "NONE", "-i", "NONE", "--cmd", "set noswapfile"]') | |||
| '", "-u", "NONE", "-i", "NONE", "--cmd", "set noswapfile shell=sh"]') | |||
There was a problem hiding this comment.
This might not be needed after making the suggested change to clear()
|
@nhooyr |
justinmk
pushed a commit
that referenced
this pull request
Sep 11, 2023
Problem: Some tests fail with $SHELL=fish #6172 Related: #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: #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>
glepnir
pushed a commit
to glepnir/neovim
that referenced
this pull request
Sep 14, 2023
Problem: Some tests fail with $SHELL=fish neovim#6172 Related: neovim#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 neovim#6172 TODO: Other test failures may be related to "session leader" issue: neovim#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>
ur4ltz
pushed a commit
to ur4ltz/neovim-1
that referenced
this pull request
Sep 15, 2023
Problem: Some tests fail with $SHELL=fish neovim#6172 Related: neovim#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 neovim#6172 TODO: Other test failures may be related to "session leader" issue: neovim#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>
dundargoc
pushed a commit
to dundargoc/neovim
that referenced
this pull request
Sep 27, 2023
Problem: Some tests fail with $SHELL=fish neovim#6172 Related: neovim#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 neovim#6172 TODO: Other test failures may be related to "session leader" issue: neovim#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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Though, as described in #6172 (comment) this still has issues.
This will probably need to be updated for windows too. See #6172 (comment)