-
Notifications
You must be signed in to change notification settings - Fork 109
[Reverted] Waiter 0.1 #582
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
Conversation
Reviewer's Guide by SourceryThis pull request enhances the terminal content waiting utility by introducing a more fluent API, improved error handling, and support for multiple content patterns. It also includes comprehensive documentation and examples to demonstrate the new features. Sequence diagram for wait_for_pane_content with regexsequenceDiagram
participant User
participant PaneContentWaiter
participant Pane
participant re
User->>PaneContentWaiter: wait_for_pane_content(pane, pattern, ContentMatchType.REGEX, ...)
PaneContentWaiter->>Pane: capture_pane(start, end)
Pane->>Pane: Get content
Pane-->>PaneContentWaiter: content: list[str]
alt pattern is str
PaneContentWaiter->>re: compile(pattern)
re-->>PaneContentWaiter: regex: re.Pattern
end
PaneContentWaiter->>re: search(content_str)
re-->>PaneContentWaiter: match: Match[str] | None
alt match is not None
PaneContentWaiter->>PaneContentWaiter: result.matched_content = match.group(0)
PaneContentWaiter->>PaneContentWaiter: Find matching line
PaneContentWaiter-->>User: WaitResult(success=True, ...)
else match is None
PaneContentWaiter-->>User: WaitResult(success=False, ...)
end
Sequence diagram for fluent API usagesequenceDiagram
participant User
participant expect(pane)
participant PaneContentWaiter
participant wait_for_text
participant wait_for_pane_content
participant Pane
User->>expect(pane): expect(pane)
activate expect(pane)
expect(pane)->>PaneContentWaiter: PaneContentWaiter(pane)
activate PaneContentWaiter
expect(pane)-->>User: PaneContentWaiter instance
deactivate expect(pane)
User->>PaneContentWaiter: with_timeout(timeout)
PaneContentWaiter->>PaneContentWaiter: self.timeout = timeout
PaneContentWaiter-->>User: PaneContentWaiter instance
User->>PaneContentWaiter: wait_for_text(text)
activate wait_for_text
wait_for_text->>wait_for_pane_content: wait_for_pane_content(pane, text, ContentMatchType.CONTAINS, timeout=self.timeout, ...)
activate wait_for_pane_content
wait_for_pane_content->>Pane: capture_pane()
Pane-->>wait_for_pane_content: content
wait_for_pane_content->>wait_for_pane_content: Check content for text
alt text found in content
wait_for_pane_content-->>wait_for_text: WaitResult(success=True, ...)
else text not found in content
wait_for_pane_content-->>wait_for_text: WaitResult(success=False, ...)
end
deactivate wait_for_pane_content
wait_for_text-->>User: WaitResult
deactivate wait_for_text
deactivate PaneContentWaiter
Updated class diagram for PaneContentWaiterclassDiagram
class PaneContentWaiter {
-pane: Pane
-timeout: float
-interval: float
-raises: bool
-start_line: int | None
-end_line: int | None
__init__(pane: Pane) : void
+with_timeout(timeout: float) : PaneContentWaiter
+with_interval(interval: float) : PaneContentWaiter
+without_raising() : PaneContentWaiter
+with_line_range(start: int | str | None, end: int | str | None) : PaneContentWaiter
+wait_for_text(text: str) : WaitResult
+wait_for_exact_text(text: str) : WaitResult
+wait_for_regex(pattern: str | re.Pattern[str]) : WaitResult
+wait_for_predicate(predicate: Callable[[list[str]], bool]) : WaitResult
+wait_until_ready(shell_prompt: str | re.Pattern[str] | None) : WaitResult
}
class WaitResult {
+success: bool
+content: list[str] | None
+matched_content: str | list[str] | None
+match_line: int | None
+elapsed_time: float | None
+error: str | None
+matched_pattern_index: int | None
}
class ContentMatchType {
<<enumeration>>
EXACT
CONTAINS
REGEX
PREDICATE
}
PaneContentWaiter -- WaitResult : Returns
PaneContentWaiter -- ContentMatchType : Uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
cd876dc
to
f155d15
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #582 +/- ##
==========================================
+ Coverage 79.83% 81.56% +1.73%
==========================================
Files 22 37 +15
Lines 1914 2430 +516
Branches 294 368 +74
==========================================
+ Hits 1528 1982 +454
- Misses 266 307 +41
- Partials 120 141 +21 ☔ View full report in Codecov by Sentry. |
@sourcery-ai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @tony - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a helper function to reduce duplication in tests that involve sending commands to a pane and waiting for a result.
- The new fluent API looks great, but ensure that the documentation clearly explains when to use the fluent API vs. the existing functions.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 3 issues found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@sourcery-ai dismiss |
7c755e7
to
9213b88
Compare
@sourcery-ai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @tony - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a helper function to encapsulate the logic for creating a new window and pane, as this pattern is repeated throughout the tests.
- The addition of the waiter module and its tests is a significant enhancement, but it also adds complexity. Ensure that the documentation is comprehensive and easy to understand for new contributors.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 5 issues found
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @tony - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a helper function to encapsulate the logic for creating a new window and pane, as this pattern is repeated throughout the tests.
- The addition of the waiter module and its tests is a significant enhancement, but it also adds complexity. Ensure that the documentation is comprehensive and easy to understand for new contributors.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@sourcery-ai plan |
43eca42
to
962d643
Compare
07e3af1
to
67350d4
Compare
@sourcery-ai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @tony - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a helper function to encapsulate the logic for creating and cleaning up test windows and panes to reduce code duplication across tests.
- The new waiter module introduces a lot of new code - consider adding a diagram or high-level overview to the documentation to help users understand the different components and how they fit together.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
e8c23de
to
c0de412
Compare
@sourcery-ai review |
…essaging - Add descriptive timeout message to WaitTimeout exception - Ensure consistent handling of timeout errors - Fix type hints for function return values
…I and multi-pattern support - Implement Playwright-inspired fluent API for more expressive test code - Add wait_for_any_content and wait_for_all_content for composable waiting - Fix type annotations for all wait_for functions - Improve WaitResult class to handle different return types - Fix doctest examples to prevent execution failures - Enhance error handling with better timeout messages
- Fix test_wait_for_pane_content_exact to use correct match type - Update test_wait_for_any_content to check matched_pattern_index - Fix test_wait_for_all_content to handle list of matched patterns - Add comprehensive type annotations to all test functions - Ensure proper handling of None checks for Pane objects
…iters - Create detailed markdown documentation in docs/test-helpers/waiter.md - Add key features section highlighting main capabilities - Include quick start examples for all functions - Document fluent API with Playwright-inspired design - Explain wait_for_any_content and wait_for_all_content with practical examples - Add detailed API reference for all waiters - Include testing best practices section
- Adds a conftest.py file in tests/examples to register the pytest.mark.example marker - Eliminates pytest warnings about unknown markers in example tests - Improves test output by removing noise from warnings
- Each test file focuses on a single feature or concept of the waiter module - Added descriptive docstrings to all test functions for better documentation - Created conftest.py with session fixture for waiter examples - Added helpers.py with utility functions for the test examples - Test files now follow a consistent naming convention for easier reference - Each test file is self-contained and demonstrates a single concept - All tests are marked with @pytest.mark.example for filtering This restructuring supports the documentation update to use literalinclude directives, making the documentation more maintainable and ensuring it stays in sync with actual code.
@sourcery-ai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @tony - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a helper function to encapsulate the logic for killing windows safely, to avoid repetition in test cleanup.
- The addition of comprehensive tests and examples is great, but ensure that these are also regularly reviewed and updated to reflect any API changes.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Note