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.
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.
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.
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.
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.
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