Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix(tests): explicitly state the shell to use#844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
zappolowski wants to merge1 commit intotmux-python:master
base:master
Choose a base branch
Loading
fromzappolowski:fix/do-not-assume-bash-is-default-shell

Conversation

@zappolowski
Copy link
Contributor

tmux uses the default login shell and thus tests might fail if this is not bash (or something compatible). Explicitly stating which shell to use circumvents issues arising from accidentally using another shell.

@codecov
Copy link

codecovbot commentedNov 2, 2022•
edited
Loading

Codecov Report

Merging#844 (0b129c6) intomaster (d63f9f5) willnot change coverage.
The diff coverage isn/a.

❗ Current head0b129c6 differs from pull request most recent head727ab39. Consider uploading reports for the commit727ab39 to get more accurate results

@@           Coverage Diff           @@##           master     #844   +/-   ##=======================================  Coverage   76.46%   76.46%           =======================================  Files          24       24             Lines        1606     1606             Branches      362      362           =======================================  Hits         1228     1228             Misses        269      269             Partials      109      109

📣 We’re building smart automated test selection to slash your CI/CD build times.Learn more

@tony
Copy link
Member

tony commentedNov 3, 2022•
edited
Loading

tmux uses the default login shell and thus tests might fail if this is not bash (or something compatible). Explicitly stating which shell to use circumvents issues arising from accidentally using another shell.

Perhaps it may be beneficial or necessary to set adefault-shell in tests.

Out of curiousity, without this, what error do you get? What distro / system are you running into an issue on?

As for/bin/bash, it isn't available in a default OpenBSD / FreeBSD installation, for instance. It may not be the most portable solution.

@zappolowski
Copy link
ContributorAuthor

zappolowski commentedNov 3, 2022•
edited
Loading

Out of curiousity, without this, what error do you get? What distro / system are you running into an issue on?

I'm on Arch, but I think the issue is my (login) shell:/usr/bin/fish ... as using/bin/bash avoids the issue.

Running (on current master):
py.test tests/workspace/test_builder.py -k 'test_load_workspace_enter[pane_enter_default_shortform]' > pytest.log
yieldspytest.log

There are two issues with fish for me:

  • it doesn't support the$((nummeric_stuff)) syntax used in the test (I'm not sure which shells support this, but fish and e.g. tcsh don't)
  • fish prints out a welcome message, which interferes with the capturing (as far as I can see there's already fixture code for suppressing something similar for zsh)

As for /bin/bash, it isn't available in a default OpenBSD / FreeBSD installation, for instance. It may not be the most portable solution.

I think,/bin/sh could be worth a try, though it might also end up using a shell which doesn't support everything currently used in the tests.

tony reacted with thumbs up emoji

@tony
Copy link
Member

tony commentedNov 4, 2022

Thank you!

I will look closer during the weekend. I'd like to get arch / perhaps freebsd setup locally

Perhapsdefault-shell: /bin/bash is the best approach. Another option would be to have a CI job that sets up arch and a few other distros to run pytest.

@tony
Copy link
Member

tony commentedNov 5, 2022•
edited
Loading

@zappolowski On a fresh Arch installation, I initially got a similar error. I don't have fish. But I think it's due to not havingtmux(1) installed yet.

It started working after installing a few packages / configurations. Likely since thearch package for tmuxp installs tmux.

$sudo pacman -Qearch-install-scripts 27-1base 3-1dhcpcd 9.4.1-1diffutils 3.8-1git 2.38.1-2groff 1.22.4-7inetutils 2.3-1keychain 2.8.5-2logrotate 3.20.1-1make 4.3-3man-db 2.11.0-1man-pages 6.01-1nano 6.4-1netctl 1.28-1nodejs 19.0.1-1openresolv 3.12.0-1openssh 9.1p1-3python 3.10.8-3sudo 1.9.12-5sysfsutils 2.1.1-1texinfo 6.8-2vcspull 1.18.0-1vi 1:070224-6vim 9.0.0814-1which 2.21-5yarn 1.22.19-1

I created a second user:py.test works fine also.

@zappolowski
Copy link
ContributorAuthor

zappolowski commentedNov 5, 2022•
edited
Loading

As I've said the culprit is not Arch, but my login-shell, which isfish, which is (intentionally) not POSIX-compatible.

I've created two (hopefully) portable containers to verify this (TBH it was a bit of a PITA to figure out, why the tests failed as man pages were missing):
build_tmuxp_arch_fish_image.sh.txt
build_tmuxp_arch_bash_image.sh.txt

If you havebuildah andpodman installed, you can just build the images by executing the script and then run them by

$ podman run --rm --volume$(pwd):/src:ro localhost/arch-bash-tmuxp-dev:latest$ podman run --rm --volume$(pwd):/src:ro localhost/arch-fish-tmuxp-dev:latest

The first one should succeed, the second one should fail.

Edit: Using the bash image I can locally run the test-suite without issues, which would be okay for me. It's up to you to decide whether it"s worth the effort of supporting "special" configurations like mine.

Edit2: A note on running the containers. As/src is mounted read-only you have to setin-project = false inpoetry.toml.

tony reacted with eyes emoji

@tony
Copy link
Member

tony commentedNov 5, 2022•
edited
Loading

This is the error I got withfish(1) above

podman run --rm --interactive --tty --volume $(pwd):/src:rw localhost/arch-fish-tmuxp-dev:latest
Installing the current project: libtmux (0.15.9)============================================================================================================ test session starts ============================================================================================================collected 180 itemssrc/libtmux/pane.py .RRF                                                                                                                                                                                                              [  1%]src/libtmux/pytest_plugin.py ..                                                                                                                                                                                                       [  2%]src/libtmux/server.py ..                                                                                                                                                                                                              [  3%]src/libtmux/session.py .                                                                                                                                                                                                              [  3%]src/libtmux/test.py ......                                                                                                                                                                                                            [  7%]src/libtmux/window.py ..                                                                                                                                                                                                              [  8%]tests/test_common.py ....................                                                                                                                                                                                             [ 19%]tests/test_pane.py .....                                                                                                                                                                                                              [ 22%]tests/test_pytest_plugin.py .                                                                                                                                                                                                         [ 22%]tests/test_server.py ............                                                                                                                                                                                                     [ 29%]tests/test_session.py .........................                                                                                                                                                                                       [ 43%]tests/test_test.py .....                                                                                                                                                                                                              [ 46%]tests/test_tmuxobject.py .....                                                                                                                                                                                                        [ 48%]tests/test_window.py .......................                                                                                                                                                                                          [ 61%]docs/index.md .............                                                                                                                                                                                                           [ 68%]docs/quickstart.md .....................                                                                                                                                                                                              [ 80%]docs/reference/properties.md .............                                                                                                                                                                                            [ 87%]docs/topics/traversal.md .........                                                                                                                                                                                                    [ 92%]README.md .............                                                                                                                                                                                                               [100%]================================================================================================================= FAILURES ==================================================================================================================___________________________________________________________________________________________________ [doctest] libtmux.pane.Pane.send_keys ___________________________________________________________________________________________________150             .. versionchanged:: 0.14151152                Default changed from True to False.153         literal : bool, optional154             Send keys literally, default True.155156         Examples157         --------158         >>> pane = window.split_window(shell='sh')159         >>> pane.capture_pane()Expected:    ['$']Got:    ['sh-5.1$']/src/src/libtmux/pane.py:159: DocTestFailure============================================================================================================= warnings summary ==============================================================================================================../home/arch/.cache/pypoetry/virtualenvs/libtmux-VsnhxLU2-py3.10/lib/python3.10/site-packages/_pytest/cacheprovider.py:433  /home/arch/.cache/pypoetry/virtualenvs/libtmux-VsnhxLU2-py3.10/lib/python3.10/site-packages/_pytest/cacheprovider.py:433: PytestCacheWarning: cache could not write path /src/.pytest_cache/v/cache/nodeids    config.cache.set("cache/nodeids", sorted(self.cached_nodeids))../home/arch/.cache/pypoetry/virtualenvs/libtmux-VsnhxLU2-py3.10/lib/python3.10/site-packages/_pytest/cacheprovider.py:387  /home/arch/.cache/pypoetry/virtualenvs/libtmux-VsnhxLU2-py3.10/lib/python3.10/site-packages/_pytest/cacheprovider.py:387: PytestCacheWarning: cache could not write path /src/.pytest_cache/v/cache/lastfailed    config.cache.set("cache/lastfailed", self.lastfailed)../home/arch/.cache/pypoetry/virtualenvs/libtmux-VsnhxLU2-py3.10/lib/python3.10/site-packages/_pytest/stepwise.py:52  /home/arch/.cache/pypoetry/virtualenvs/libtmux-VsnhxLU2-py3.10/lib/python3.10/site-packages/_pytest/stepwise.py:52: PytestCacheWarning: cache could not write path /src/.pytest_cache/v/cache/stepwise    session.config.cache.set(STEPWISE_CACHE_DIR, [])-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html========================================================================================================== short test summary info ==========================================================================================================FAILED src/libtmux/pane.py::libtmux.pane.Pane.send_keys============================================================================================ 1 failed, 179 passed, 3 warnings, 2 rerun in 42.16s ============================================================================================

@zappolowski
Copy link
ContributorAuthor

You've run the tests onlibtmux, but the issue is ontmuxp. 😃

But yeah, that error I also get onlibtmux but I couldn't figure out how to make it work properly. As it's an easy to spot false-positive I'm fine with ignoring it (for me locally). I think it works on Ubuntu as they switched to using/bin/sh -> dash some years ago. On Arch/bin/sh -> bash which usesPS1=\s-\v$.

tony reacted with eyes emoji

@tonytonyforce-pushed thefix/do-not-assume-bash-is-default-shell branch fromb1b3775 toe2b8600CompareNovember 5, 2022 14:49
@tony
Copy link
Member

tony commentedNov 5, 2022

Edit2: A note on running the containers. As /src is mounted read-only you have to set in-project = false in poetry.toml.

I removedin-project = true frompoetry.toml in libtmux and tmuxp and rebased the PRs

If you're in the PR's branches checked out:

git pull --rebase --autostash

@tony
Copy link
Member

tony commentedNov 5, 2022

Thosepodman andbuildah commands were nifty.

@tony
Copy link
Member

tony commentedNov 5, 2022

Do you have a list of which tests you issues with when you usefish? At the moment, here's what I get:

FAILED tests/workspace/test_builder.py::test_load_workspace_enter[pane_enter_default_shortform] - libtmux.exc.WaitTimeoutFAILED tests/workspace/test_builder.py::test_load_workspace_enter[pane_enter_default_longform] - libtmux.exc.WaitTimeoutFAILED tests/workspace/test_builder.py::test_load_workspace_enter[pane_command_enter_default_shortform] - libtmux.exc.WaitTimeoutFAILED tests/workspace/test_builder.py::test_load_workspace_enter[pane_command_enter_default_longform] - libtmux.exc.WaitTimeoutFAILED tests/workspace/test_builder.py::test_load_workspace_sleep[command_level_sleep_shortform] - assert '___4___' in 'Welcome to fish, the friendly interactive shell\nType help for instructions on how to use fish\nt@t /h/t/w/p/tmuxp (m..._" echo "___$((1 + 3))___"\n          ^\nt@t /h/t/w/p/tmuxp (master)>  echo "___$((1 + 5))_...FAILED tests/workspace/test_builder.py::test_load_workspace_sleep[command_level_pane_sleep_longform] - assert '___4___' in 'Welcome to fish, the friendly interactive shell\nType help for instructions on how to use fish\nt@t /h/t/w/p/tmuxp (m..._" echo "___$((1 + 3))___"\n          ^\nt@t /h/t/w/p/tmuxp (master)>  echo "___$((1 + 5))_...FAILED tests/workspace/test_builder.py::test_load_workspace_sleep[pane_sleep_shortform] - assert '___4___' in 'Welcome to fish, the friendly interactive shell\nType help for instructions on how to use fish\nt@t /h/t/w/p/tmuxp (m...lease use \'((1 + 3))\'.\n echo "___$((1 + 3))___"\n          ^\nt@t /h/t/w/p/tmuxp (master...FAILED tests/workspace/test_builder.py::test_load_workspace_sleep[pane_sleep_longform] - assert '___4___' in 'Welcome to fish, the friendly interactive shell\nType help for instructions on how to use fish\nt@t /h/t/w/p/tmuxp (m...lease use \'((1 + 3))\'.\n echo "___$((1 + 3))___"\n          ^\nt@t /h/t/w/p/tmuxp (master...FAILED tests/workspace/test_builder.py::test_load_workspace_sleep[shell_before_before_command_level] - assert '___4___' in 'Welcome to fish, the friendly interactive shell\nType help for instructions on how to use fish\nt@t /h/t/w/p/tmuxp (m...lease use \'((1 + 3))\'.\n echo "___$((1 + 3))___"\n          ^\nt@t /h/t/w/p/tmuxp (master...

@zappolowski
Copy link
ContributorAuthor

zappolowski commentedNov 5, 2022•
edited
Loading

Yes, these are the same tests failing for me.

tony reacted with thumbs up emoji

`tmux` uses the default login shell and thus tests might fail if this isnot bash (or something compatible). Explicitly stating which shell touse circumvents issues arising from accidentally using another shell.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zappolowski@tony
[8]ページ先頭
©2009-2025 Movatter.jp