Skip to content

test(test_capture_pane): Remove bashism(s) #455

New issue

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

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Nov 5, 2022
Merged

test(test_capture_pane): Remove bashism(s) #455

merged 3 commits into from
Nov 5, 2022

Conversation

tony
Copy link
Member

@tony tony commented Nov 5, 2022

Fixes #454

@codecov
Copy link

codecov bot commented Nov 5, 2022

Codecov Report

Merging #455 (bdd7122) into master (5869e01) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #455   +/-   ##
=======================================
  Coverage   87.46%   87.46%           
=======================================
  Files          20       20           
  Lines        1803     1803           
  Branches      280      280           
=======================================
  Hits         1577     1577           
  Misses        154      154           
  Partials       72       72           
Impacted Files Coverage Δ
src/libtmux/common.py 84.11% <ø> (ø)
tests/test_pane.py 100.00% <ø> (ø)

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

@tony tony changed the title test(test_capture_pane): Remove bashism test(test_capture_pane): Remove bashism(s) Nov 5, 2022
@tony
Copy link
Member Author

tony commented Nov 5, 2022

@zappolowski How is this for you? (in re: tmux-python/tmuxp#844)

Do any tests fail w/ libtmux?

@zappolowski
Copy link
Contributor

It's still the doctest from src/libtmux/pane.py:159 which fails. That test also seems a bit flaky as the captured value either is empty [] or (as mentioned somewhere else) the default prompt of bash which is PS1=\s-\v$ and thus ['sh-5.1$'].

I still have no clue how this issue could be resolved, but as already stated its easy to ignore for humans - as long as the CI uses dash it won't fail.

@tony
Copy link
Member Author

tony commented Nov 5, 2022

I will take care of it - but I don't have an ETA yet (until I understand it better / reproduce it reliably)

@tony
Copy link
Member Author

tony commented Nov 5, 2022

@zappolowski Moved this to #456 so it has a dedicated issue

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove BASH-isms from tests
2 participants