Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests: Improve shell interoperability #577

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

tony
Copy link
Member

@tony tony commented Feb 23, 2025

Summary by Sourcery

Improve shell interoperability in tests by using a consistent prompt across shells and adding retry logic to wait for the shell to be ready and for the command output to be available before asserting the pane contents.

Tests:

  • Improve shell interoperability in tests by using PROMPT_COMMAND and PS1 to set a consistent prompt across shells.
  • Add retry logic to wait for the shell to be ready and for the command output to be available before asserting the pane contents.

Copy link

sourcery-ai bot commented Feb 23, 2025

Reviewer's Guide by Sourcery

The pull request improves the reliability of the test_capture_pane test by ensuring consistent shell behavior across different environments. It achieves this by setting a custom prompt using PROMPT_COMMAND and PS1, and then waiting for the shell to become ready before sending commands. It also waits for the command output before asserting the final state.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Improved shell interoperability in test_capture_pane by setting a consistent prompt and waiting for the shell to be ready.
  • Used PROMPT_COMMAND and PS1 to set a consistent prompt across shells.
  • Added a wait_for_prompt function to wait for the shell to be ready with the custom prompt.
  • Added a wait_for_output function to wait for the command output and the new prompt.
  • Modified assertions to check for the custom prompt and command output.
tests/test_pane.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

codecov bot commented Feb 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.83%. Comparing base (0e4a118) to head (82da140).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #577   +/-   ##
=======================================
  Coverage   79.83%   79.83%           
=======================================
  Files          22       22           
  Lines        1914     1914           
  Branches      294      294           
=======================================
  Hits         1528     1528           
  Misses        266      266           
  Partials      120      120           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tony tony force-pushed the test-terminal-interoperability branch 3 times, most recently from c11f0dc to ac631e0 Compare February 24, 2025 00:33
tony added 7 commits February 25, 2025 16:16
why: Tests were failing when using fish shell due to prompt differences

what:
- Use PROMPT_COMMAND='' to clear shell-specific prompt commands
- Set custom PS1='READY>' that works across shells
- Add retry logic to wait for prompt and command output
- Make assertions more flexible by checking content not exact matches
why: Test was flaky due to timing issues with shell initialization

what:
- Add small delay after window creation
- Add error handling around capture_pane calls
- Increase retry timeouts from 1s to 2s
- Add more comprehensive output checks
- Add check for non-empty pane contents
Add setup_shell_window helper function to standardize shell setup in tests:
Set consistent shell environment and prompt, add robust waiting for shell
readiness, handle environment variables consistently, prevent shell-specific
prompt modifications.

Update test_new_window_with_environment to use new helper:
Add proper waiting for command output, fix linting issues with loop variables,
make tests more reliable against timing issues.
@tony tony force-pushed the test-terminal-interoperability branch from ac631e0 to 82da140 Compare February 25, 2025 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant