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

Re-raise encountered exceptions in pilot tests #2754

Merged
merged 13 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- Fixed setting `TreeNode.label` on an existing `Tree` node not immediately https://github.com/Textualize/textual/pull/2713
- Correctly implement `__eq__` protocol in DataTable https://github.com/Textualize/textual/pull/2705
- Fixed exceptions in Pilot tests being silently ignored https://github.com/Textualize/textual/pull/2754
- Fixed issue where internal data of `OptionList` could be invalid for short window after `clear_options` https://github.com/Textualize/textual/pull/2754
- Fixed `Tooltip` causing a `query_one` on a lone `Static` to fail https://github.com/Textualize/textual/issues/2723

### Changed
Expand Down
19 changes: 18 additions & 1 deletion src/textual/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@ class CssPathError(Exception):

ReturnType = TypeVar("ReturnType")


CSSPathType = Union[
str,
PurePath,
Expand Down Expand Up @@ -367,6 +366,13 @@ def __init__(
self._animate = self._animator.bind(self)
self.mouse_position = Offset(0, 0)

self._exception: Exception | None = None
"""The unhandled exception which is leading to the app shutting down,
or None if the app is still running with no unhandled exceptions."""

self._exception_event: asyncio.Event = asyncio.Event()
"""An event that will be set when the first exception is encountered."""

self.title = (
self.TITLE if self.TITLE is not None else f"{self.__class__.__name__}"
)
Expand Down Expand Up @@ -1108,6 +1114,9 @@ async def run_app(app) -> None:
# Shutdown the app cleanly
await app._shutdown()
await app_task
# Re-raise the exception which caused panic so test frameworks are aware
if self._exception:
raise self._exception

async def run_async(
self,
Expand Down Expand Up @@ -1782,9 +1791,17 @@ def render(renderable: RenderableType) -> list[Segment]:
def _handle_exception(self, error: Exception) -> None:
"""Called with an unhandled exception.

Always results in the app exiting.

Args:
error: An exception instance.
"""
# If we're running via pilot and this is the first exception encountered,
# take note of it so that we can re-raise for test frameworks later.
if self.is_headless and self._exception is None:
rodrigogiraoserrao marked this conversation as resolved.
Show resolved Hide resolved
self._exception = error
self._exception_event.set()

if hasattr(error, "__rich__"):
# Exception has a rich method, so we can defer to that for the rendering
self.panic(error)
Expand Down
1 change: 1 addition & 0 deletions src/textual/message_pump.py
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ async def _process_messages_loop(self) -> None:
"""Process messages until the queue is closed."""
_rich_traceback_guard = True
self._thread_id = threading.get_ident()

while not self._closed:
try:
message = await self._get_message()
Expand Down
39 changes: 33 additions & 6 deletions src/textual/pilot.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ def _get_mouse_message_arguments(
return message_arguments


class WaitForScreenTimeout(Exception):
pass


@rich.repr.auto(angular=True)
class Pilot(Generic[ReturnType]):
"""Pilot object to drive an app."""
Expand Down Expand Up @@ -134,13 +138,17 @@ async def hover(
await self.pause()

async def _wait_for_screen(self, timeout: float = 30.0) -> bool:
"""Wait for the current screen to have processed all pending events.
"""Wait for the current screen and its children to have processed all pending events.

Args:
timeout: A timeout in seconds to wait.

Returns:
`True` if all events were processed, or `False` if the wait timed out.
`True` if all events were processed. `False` if an exception occurred,
meaning that not all events could be processed.

Raises:
WaitForScreenTimeout: If the screen and its children didn't finish processing within the timeout.
"""
children = [self.app, *self.app.screen.walk_children(with_self=True)]
count = 0
Expand All @@ -160,10 +168,29 @@ def decrement_counter() -> None:
count += 1

if count:
# Wait for the count to return to zero, or a timeout
try:
await asyncio.wait_for(count_zero_event.wait(), timeout=timeout)
except asyncio.TimeoutError:
# Wait for the count to return to zero, or a timeout, or an exception
wait_for = [
asyncio.create_task(count_zero_event.wait()),
asyncio.create_task(self.app._exception_event.wait()),
]
_, pending = await asyncio.wait(
wait_for,
timeout=timeout,
return_when=asyncio.FIRST_COMPLETED,
)

for task in pending:
task.cancel()

timed_out = len(wait_for) == len(pending)
if timed_out:
raise WaitForScreenTimeout(
"Timed out while waiting for widgets to process pending messages."
)

# We've either timed out, encountered an exception, or we've finished
# decrementing all the counters (all events processed in children).
if count > 0:
return False

return True
Expand Down
9 changes: 1 addition & 8 deletions src/textual/widgets/_option_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -630,14 +630,7 @@ def clear_options(self) -> Self:
self.highlighted = None
self._mouse_hovering_over = None
self.virtual_size = Size(self.scrollable_content_region.width, 0)
# TODO: See https://github.com/Textualize/textual/issues/2582 -- it
# should not be necessary to do this like this here; ideally here in
# clear_options it would be a forced refresh, and also in a
# `on_show` it would be the same (which, I think, would actually
# solve the problem we're seeing). But, until such a time as we get
# to the bottom of 2582... this seems to delay the refresh enough
# that things fall into place.
self._request_content_tracking_refresh()
self._refresh_content_tracking(force=True)
return self

def _set_option_disabled(self, index: int, disabled: bool) -> Self:
Expand Down
4 changes: 1 addition & 3 deletions src/textual/widgets/_tabs.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,9 +422,7 @@ def watch_active(self, previously_active: str, active: str) -> None:
active_tab = self.query_one(f"#tabs-list > #{active}", Tab)
self.query("#tabs-list > Tab.-active").remove_class("-active")
active_tab.add_class("-active")
self.call_after_refresh(
self._highlight_active, animate=previously_active != ""
)
self.call_later(self._highlight_active, animate=previously_active != "")
self.post_message(self.TabActivated(self, active_tab))
else:
underline = self.query_one(Underline)
Expand Down
35 changes: 34 additions & 1 deletion tests/test_pilot.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
from string import punctuation

import pytest

from textual import events
from textual.app import App
from textual.app import App, ComposeResult
from textual.binding import Binding
from textual.widgets import Label

KEY_CHARACTERS_TO_TEST = "akTW03" + punctuation
"""Test some "simple" characters (letters + digits) and all punctuation."""
Expand All @@ -19,3 +23,32 @@ def on_key(self, event: events.Key) -> None:
for char in KEY_CHARACTERS_TO_TEST:
await pilot.press(char)
assert keys_pressed[-1] == char


async def test_pilot_exception_catching_compose():
"""Ensuring that test frameworks are aware of exceptions
inside compose methods when running via Pilot run_test()."""

class FailingApp(App):
def compose(self) -> ComposeResult:
1 / 0
yield Label("Beep")

with pytest.raises(ZeroDivisionError):
async with FailingApp().run_test():
pass


async def test_pilot_exception_catching_action():
"""Ensure that exceptions inside action handlers are presented
to the test framework when running via Pilot run_test()."""

class FailingApp(App):
BINDINGS = [Binding("b", "beep", "beep")]

def action_beep(self) -> None:
1 / 0

with pytest.raises(ZeroDivisionError):
async with FailingApp().run_test() as pilot:
await pilot.press("b")
2 changes: 1 addition & 1 deletion tests/test_screen_modes.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def compose(self) -> ComposeResult:
yield Label("fast")

def on_mount(self) -> None:
self.set_interval(0.01, self.ping)
self.call_later(self.set_interval, 0.01, self.ping)

def ping(self) -> None:
pings.append(str(self.app.query_one(Label).renderable))
Expand Down