From 74ad6f73faf68b27ebfd4452e32db95e61363973 Mon Sep 17 00:00:00 2001 From: Olivier Philippon Date: Thu, 12 May 2022 11:53:07 +0100 Subject: [PATCH 01/10] [e2e] Add a way to accelerate time in our integration tests --- src/textual/_animator.py | 11 +-- src/textual/_timer.py | 52 +++++++++----- src/textual/message.py | 8 ++- tests/test_animator.py | 7 -- tests/test_integration_layout.py | 2 - tests/test_integration_scrolling.py | 5 +- tests/utilities/test_app.py | 106 ++++++++++++++++++++++------ 7 files changed, 131 insertions(+), 60 deletions(-) diff --git a/src/textual/_animator.py b/src/textual/_animator.py index c03378953..65b00244b 100644 --- a/src/textual/_animator.py +++ b/src/textual/_animator.py @@ -3,14 +3,11 @@ from __future__ import annotations from abc import ABC, abstractmethod import asyncio import sys -from time import monotonic from typing import Any, Callable, TypeVar from dataclasses import dataclass -from . import log from ._easing import DEFAULT_EASING, EASING -from ._profile import timer from ._timer import Timer from ._types import MessageTarget @@ -139,10 +136,6 @@ class Animator: pause=True, ) - def get_time(self) -> float: - """Get the current wall clock time.""" - return monotonic() - async def start(self) -> None: """Start the animator task.""" @@ -189,7 +182,7 @@ class Animator: if final_value is ...: final_value = value - start_time = self.get_time() + start_time = self._timer.get_time() animation_key = (id(obj), attribute) @@ -240,7 +233,7 @@ class Animator: if not self._animations: self._timer.pause() else: - animation_time = self.get_time() + animation_time = self._timer.get_time() animation_keys = list(self._animations.keys()) for animation_key in animation_keys: animation = self._animations[animation_key] diff --git a/src/textual/_timer.py b/src/textual/_timer.py index ef4d41a2b..3a4f453a9 100644 --- a/src/textual/_timer.py +++ b/src/textual/_timer.py @@ -8,7 +8,6 @@ from asyncio import ( sleep, Task, ) -from functools import partial from time import monotonic from typing import Awaitable, Callable, Union @@ -28,6 +27,8 @@ class EventTargetGone(Exception): @rich_repr class Timer: _timer_count: int = 1 + # Used to mock Timers' behaviour in a Textual app's integration test: + _instances: weakref.WeakSet[Timer] = weakref.WeakSet() def __init__( self, @@ -63,6 +64,7 @@ class Timer: self._repeat = repeat self._skip = skip self._active = Event() + Timer._instances.add(self) if not pause: self._active.set() @@ -104,37 +106,53 @@ class Timer: """Result a paused timer.""" self._active.set() + @staticmethod + def get_time() -> float: + """Get the current wall clock time.""" + # N.B. This method will likely be a mocking target in integration tests. + return monotonic() + + @staticmethod + async def _sleep(duration: float) -> None: + # N.B. This method will likely be a mocking target in integration tests. + await sleep(duration) + async def _run(self) -> None: """Run the timer.""" count = 0 _repeat = self._repeat _interval = self._interval - start = monotonic() + start = self.get_time() try: while _repeat is None or count <= _repeat: next_timer = start + ((count + 1) * _interval) - if self._skip and next_timer < monotonic(): + now = self.get_time() + if self._skip and next_timer < now: count += 1 continue - wait_time = max(0, next_timer - monotonic()) + wait_time = max(0, next_timer - now) if wait_time: - await sleep(wait_time) - event = events.Timer( - self.sender, - timer=self, - time=next_timer, - count=count, - callback=self._callback, - ) + await self._sleep(wait_time) count += 1 try: - if self._callback is not None: - await invoke(self._callback) - else: - await self.target.post_priority_message(event) - + await self._tick(next_timer=next_timer, count=count) except EventTargetGone: break await self._active.wait() except CancelledError: pass + + async def _tick(self, *, next_timer: float, count: int) -> None: + """Triggers the Timer's action: either call its callback, or sends an event to its target""" + if self._callback is not None: + await invoke(self._callback) + else: + event = events.Timer( + self.sender, + timer=self, + time=next_timer, + count=count, + callback=self._callback, + ) + + await self.target.post_priority_message(event) diff --git a/src/textual/message.py b/src/textual/message.py index 21a35c735..eef6ff48d 100644 --- a/src/textual/message.py +++ b/src/textual/message.py @@ -39,7 +39,7 @@ class Message: self.sender = sender self.name = camel_to_snake(self.__class__.__name__.replace("Message", "")) - self.time = monotonic() + self.time = self._get_time() self._forwarded = False self._no_default_action = False self._stop_propagation = False @@ -99,3 +99,9 @@ class Message: """ self._stop_propagation = stop return self + + @staticmethod + def _get_time() -> float: + """Get the current wall clock time.""" + # N.B. This method will likely be a mocking target in integration tests. + return monotonic() diff --git a/tests/test_animator.py b/tests/test_animator.py index 76caf31dd..84a242fee 100644 --- a/tests/test_animator.py +++ b/tests/test_animator.py @@ -245,10 +245,3 @@ def test_bound_animator(): easing=EASING[DEFAULT_EASING], ) assert animator._animations[(id(animate_test), "foo")] == expected - - -def test_animator_get_time(): - target = Mock() - animator = Animator(target) - assert isinstance(animator.get_time(), float) - assert animator.get_time() <= animator.get_time() diff --git a/tests/test_integration_layout.py b/tests/test_integration_layout.py index 4dc5d01c0..7fde168fa 100644 --- a/tests/test_integration_layout.py +++ b/tests/test_integration_layout.py @@ -1,5 +1,4 @@ from __future__ import annotations -import asyncio from typing import cast, List import pytest @@ -107,7 +106,6 @@ async def test_composition_of_vertical_container_with_children( expected_placeholders_size: tuple[int, int], expected_root_widget_virtual_size: tuple[int, int], expected_placeholders_offset_x: int, - event_loop: asyncio.AbstractEventLoop, ): class VerticalContainer(Widget): CSS = ( diff --git a/tests/test_integration_scrolling.py b/tests/test_integration_scrolling.py index 3ee0cab31..60cf1154a 100644 --- a/tests/test_integration_scrolling.py +++ b/tests/test_integration_scrolling.py @@ -21,7 +21,6 @@ from textual.widgets import Placeholder SCREEN_SIZE = Size(100, 30) -@pytest.mark.skip("flaky test") @pytest.mark.asyncio @pytest.mark.integration_test # this is a slow test, we may want to skip them in some contexts @pytest.mark.parametrize( @@ -48,7 +47,7 @@ SCREEN_SIZE = Size(100, 30) # The state of the screen at this "halfway there" timing looks to not be deterministic though, # depending on the environment - so let's only assert stuff for the middle placeholders # and the first and last ones, but without being too specific about the others: - [SCREEN_SIZE, 10, "placeholder_9", True, 0.1, (5, 6, 7), (1, 2, 9)], + [SCREEN_SIZE, 10, "placeholder_9", True, 0.1, (6, 7, 8), (1, 2, 5, 9)], ), ) async def test_scroll_to_widget( @@ -94,7 +93,7 @@ async def test_scroll_to_widget( id_: f"placeholder_{id_}" in last_display_capture for id_ in range(placeholders_count) } - + print(f"placeholders_visibility_by_id={placeholders_visibility_by_id}") # Let's start by checking placeholders that should be visible: for placeholder_id in last_screen_expected_placeholder_ids: assert ( diff --git a/tests/utilities/test_app.py b/tests/utilities/test_app.py index 802578842..a63cff6c9 100644 --- a/tests/utilities/test_app.py +++ b/tests/utilities/test_app.py @@ -4,12 +4,15 @@ import asyncio import contextlib import io from pathlib import Path -from typing import AsyncContextManager, cast +from time import monotonic +from typing import AsyncContextManager, cast, ContextManager, Callable +from unittest import mock from rich.console import Console from textual import events, errors -from textual.app import App, ReturnType, ComposeResult +from textual._timer import Timer +from textual.app import App, ComposeResult from textual.driver import Driver from textual.geometry import Size @@ -63,33 +66,52 @@ class AppTest(App): *, waiting_duration_after_initialisation: float = 0.1, waiting_duration_post_yield: float = 0, + time_acceleration: bool = True, + time_acceleration_factor: float = 10, + # force_timers_tick_after_yield: bool = True, ) -> AsyncContextManager: async def run_app() -> None: await self.process_messages() + if time_acceleration: + waiting_duration_after_initialisation /= time_acceleration_factor + waiting_duration_post_yield /= time_acceleration_factor + + time_acceleration_context: ContextManager = ( + textual_timers_accelerate_time(acceleration_factor=time_acceleration_factor) + if time_acceleration + else contextlib.nullcontext() + ) + @contextlib.asynccontextmanager async def get_running_state_context_manager(): self._set_active() - run_task = asyncio.create_task(run_app()) - timeout_before_yielding_task = asyncio.create_task( - asyncio.sleep(waiting_duration_after_initialisation) - ) - done, pending = await asyncio.wait( - ( - run_task, - timeout_before_yielding_task, - ), - return_when=asyncio.FIRST_COMPLETED, - ) - if run_task in done or run_task not in pending: - raise RuntimeError( - "TestApp is no longer running after its initialization period" + with time_acceleration_context: + run_task = asyncio.create_task(run_app()) + timeout_before_yielding_task = asyncio.create_task( + asyncio.sleep(waiting_duration_after_initialisation) ) - yield - if waiting_duration_post_yield: - await asyncio.sleep(waiting_duration_post_yield) - assert not run_task.done() - await self.shutdown() + done, pending = await asyncio.wait( + ( + run_task, + timeout_before_yielding_task, + ), + return_when=asyncio.FIRST_COMPLETED, + ) + if run_task in done or run_task not in pending: + raise RuntimeError( + "TestApp is no longer running after its initialization period" + ) + yield + waiting_duration = max( + waiting_duration_post_yield or 0, + self.screen._update_timer._interval, + ) + await asyncio.sleep(waiting_duration) + # if force_timers_tick_after_yield: + # await textual_timers_force_tick() + assert not run_task.done() + await self.shutdown() return get_running_state_context_manager() @@ -207,3 +229,45 @@ class DriverTest(Driver): def stop_application_mode(self) -> None: pass + + +async def textual_timers_force_tick() -> None: + timer_instances_tick_tasks: list[asyncio.Task] = [] + for timer in Timer._instances: + task = asyncio.create_task(timer._tick(next_timer=0, count=0)) + timer_instances_tick_tasks.append(task) + await asyncio.wait(timer_instances_tick_tasks) + + +def textual_timers_accelerate_time( + *, acceleration_factor: float = 10 +) -> ContextManager: + @contextlib.contextmanager + def accelerate_time_for_timer_context_manager(): + starting_time = monotonic() + + # Our replacement for "textual._timer.Timer._sleep": + async def timer_sleep(duration: float) -> None: + await asyncio.sleep(duration / acceleration_factor) + + # Our replacement for "textual._timer.Timer.get_time": + def timer_get_time() -> float: + real_now = monotonic() + real_elapsed_time = real_now - starting_time + accelerated_elapsed_time = real_elapsed_time * acceleration_factor + print( + f"timer_get_time:: accelerated_elapsed_time={accelerated_elapsed_time}" + ) + return starting_time + accelerated_elapsed_time + + with mock.patch("textual._timer.Timer._sleep") as timer_sleep_mock, mock.patch( + "textual._timer.Timer.get_time" + ) as timer_get_time_mock, mock.patch( + "textual.message.Message._get_time" + ) as message_get_time_mock: + timer_sleep_mock.side_effect = timer_sleep + timer_get_time_mock.side_effect = timer_get_time + message_get_time_mock.side_effect = timer_get_time + yield + + return accelerate_time_for_timer_context_manager() From 15df75919744fbea824bbf029cfb56029a3d0dc8 Mon Sep 17 00:00:00 2001 From: Olivier Philippon Date: Fri, 13 May 2022 10:35:06 +0100 Subject: [PATCH 02/10] =?UTF-8?q?[App]=20Finally,=20time=20mocking=20in=20?= =?UTF-8?q?tests=20seems=20to=20be=20working!=20=F0=9F=98=85?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I had to add a flag in the `_timer` module that allows us to completely disable the "skip" feature of Timers, though - but it shouldn't cause too much trouble 🤞 --- src/textual/_animator.py | 13 +- src/textual/_timer.py | 11 +- src/textual/screen.py | 14 +- tests/test_animator.py | 6 +- tests/test_integration_scrolling.py | 73 +++++---- tests/utilities/test_app.py | 232 +++++++++++++++++----------- 6 files changed, 211 insertions(+), 138 deletions(-) diff --git a/src/textual/_animator.py b/src/textual/_animator.py index 65b00244b..0c031b046 100644 --- a/src/textual/_animator.py +++ b/src/textual/_animator.py @@ -179,10 +179,13 @@ class Animator: raise AttributeError( f"Can't animate attribute {attribute!r} on {obj!r}; attribute does not exist" ) + assert not all( + (duration, speed) + ), "An Animation should have a duration OR a speed, received both" if final_value is ...: final_value = value - start_time = self._timer.get_time() + start_time = self._get_time() animation_key = (id(obj), attribute) @@ -233,9 +236,15 @@ class Animator: if not self._animations: self._timer.pause() else: - animation_time = self._timer.get_time() + animation_time = self._get_time() animation_keys = list(self._animations.keys()) for animation_key in animation_keys: animation = self._animations[animation_key] if animation(animation_time): del self._animations[animation_key] + + def _get_time(self) -> float: + """Get the current wall clock time, via the internal Timer.""" + # N.B. We could remove this method and always call `self._timer.get_time()` internally, + # but it's handy to have in mocking situations + return self._timer.get_time() diff --git a/src/textual/_timer.py b/src/textual/_timer.py index 3a4f453a9..ec8b3a63b 100644 --- a/src/textual/_timer.py +++ b/src/textual/_timer.py @@ -19,6 +19,9 @@ from ._types import MessageTarget TimerCallback = Union[Callable[[], Awaitable[None]], Callable[[], None]] +# /!\ This should only be changed in an "integration tests" context, in which we mock time +_TIMERS_CAN_SKIP: bool = True + class EventTargetGone(Exception): pass @@ -27,8 +30,6 @@ class EventTargetGone(Exception): @rich_repr class Timer: _timer_count: int = 1 - # Used to mock Timers' behaviour in a Textual app's integration test: - _instances: weakref.WeakSet[Timer] = weakref.WeakSet() def __init__( self, @@ -64,7 +65,6 @@ class Timer: self._repeat = repeat self._skip = skip self._active = Event() - Timer._instances.add(self) if not pause: self._active.set() @@ -126,11 +126,10 @@ class Timer: try: while _repeat is None or count <= _repeat: next_timer = start + ((count + 1) * _interval) - now = self.get_time() - if self._skip and next_timer < now: + if self._skip and _TIMERS_CAN_SKIP and next_timer < self.get_time(): count += 1 continue - wait_time = max(0, next_timer - now) + wait_time = max(0, next_timer - self.get_time()) if wait_time: await self._sleep(wait_time) count += 1 diff --git a/src/textual/screen.py b/src/textual/screen.py index e784d3a9b..a33b236f9 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -1,5 +1,7 @@ from __future__ import annotations +import sys + from rich.console import RenderableType import rich.repr from rich.style import Style @@ -12,6 +14,14 @@ from ._compositor import Compositor, MapGeometry from .reactive import Reactive from .widget import Widget +if sys.version_info >= (3, 8): + from typing import Final +else: + from typing_extensions import Final + +# Screen updates will be batched so that they don't happen more often than 20 times per second: +UPDATE_PERIOD: Final = 1 / 20 + @rich.repr.auto class Screen(Widget): @@ -158,7 +168,9 @@ class Screen(Widget): self.check_idle() def on_mount(self, event: events.Mount) -> None: - self._update_timer = self.set_interval(1 / 20, self._on_update, pause=True) + self._update_timer = self.set_interval( + UPDATE_PERIOD, self._on_update, pause=True + ) async def on_resize(self, event: events.Resize) -> None: self.size_updated(event.size, event.virtual_size, event.container_size) diff --git a/tests/test_animator.py b/tests/test_animator.py index 84a242fee..fd3f7c038 100644 --- a/tests/test_animator.py +++ b/tests/test_animator.py @@ -177,12 +177,12 @@ class MockAnimator(Animator): self._time = 0.0 self._on_animation_frame_called = False - def get_time(self): - return self._time - def on_animation_frame(self): self._on_animation_frame_called = True + def _get_time(self): + return self._time + def test_animator(): diff --git a/tests/test_integration_scrolling.py b/tests/test_integration_scrolling.py index 60cf1154a..038270954 100644 --- a/tests/test_integration_scrolling.py +++ b/tests/test_integration_scrolling.py @@ -1,17 +1,8 @@ from __future__ import annotations - -import sys from typing import Sequence, cast -if sys.version_info >= (3, 8): - from typing import Literal -else: - from typing_extensions import Literal # pragma: no cover - - import pytest -from sandbox.vertical_container import VerticalContainer from tests.utilities.test_app import AppTest from textual.app import ComposeResult from textual.geometry import Size @@ -31,23 +22,19 @@ SCREEN_SIZE = Size(100, 30) "scroll_to_animate", "waiting_duration", "last_screen_expected_placeholder_ids", - "last_screen_expected_out_of_viewport_placeholder_ids", ), ( - [SCREEN_SIZE, 10, None, None, 0.01, (0, 1, 2, 3, 4), "others"], - [SCREEN_SIZE, 10, "placeholder_3", False, 0.01, (0, 1, 2, 3, 4), "others"], - [SCREEN_SIZE, 10, "placeholder_5", False, 0.01, (1, 2, 3, 4, 5), "others"], - [SCREEN_SIZE, 10, "placeholder_7", False, 0.01, (3, 4, 5, 6, 7), "others"], - [SCREEN_SIZE, 10, "placeholder_9", False, 0.01, (5, 6, 7, 8, 9), "others"], + [SCREEN_SIZE, 10, None, None, 0.01, (0, 1, 2, 3, 4)], + [SCREEN_SIZE, 10, "placeholder_3", False, 0.01, (0, 1, 2, 3, 4)], + [SCREEN_SIZE, 10, "placeholder_5", False, 0.01, (1, 2, 3, 4, 5)], + [SCREEN_SIZE, 10, "placeholder_7", False, 0.01, (3, 4, 5, 6, 7)], + [SCREEN_SIZE, 10, "placeholder_9", False, 0.01, (5, 6, 7, 8, 9)], # N.B. Scroll duration is hard-coded to 0.2 in the `scroll_to_widget` method atm # Waiting for this duration should allow us to see the scroll finished: - [SCREEN_SIZE, 10, "placeholder_9", True, 0.21, (5, 6, 7, 8, 9), "others"], + [SCREEN_SIZE, 10, "placeholder_9", True, 0.21, (5, 6, 7, 8, 9)], # After having waited for approximately half of the scrolling duration, we should # see the middle Placeholders as we're scrolling towards the last of them. - # The state of the screen at this "halfway there" timing looks to not be deterministic though, - # depending on the environment - so let's only assert stuff for the middle placeholders - # and the first and last ones, but without being too specific about the others: - [SCREEN_SIZE, 10, "placeholder_9", True, 0.1, (6, 7, 8), (1, 2, 5, 9)], + [SCREEN_SIZE, 10, "placeholder_9", True, 0.1, (4, 5, 6, 7, 8)], ), ) async def test_scroll_to_widget( @@ -57,9 +44,19 @@ async def test_scroll_to_widget( scroll_to_placeholder_id: str | None, waiting_duration: float | None, last_screen_expected_placeholder_ids: Sequence[int], - last_screen_expected_out_of_viewport_placeholder_ids: Sequence[int] - | Literal["others"], ): + class VerticalContainer(Widget): + CSS = """ + VerticalContainer { + layout: vertical; + overflow: hidden auto; + } + VerticalContainer Placeholder { + margin: 1 0; + height: 5; + } + """ + class MyTestApp(AppTest): CSS = """ Placeholder { @@ -77,7 +74,7 @@ async def test_scroll_to_widget( app = MyTestApp(size=screen_size, test_name="scroll_to_widget") - async with app.in_running_state(waiting_duration_post_yield=waiting_duration or 0): + async with app.in_running_state(waiting_duration_after_yield=waiting_duration or 0): if scroll_to_placeholder_id: target_widget_container = cast(Widget, app.query("#root").first()) target_widget = cast( @@ -93,24 +90,24 @@ async def test_scroll_to_widget( id_: f"placeholder_{id_}" in last_display_capture for id_ in range(placeholders_count) } - print(f"placeholders_visibility_by_id={placeholders_visibility_by_id}") + # Let's start by checking placeholders that should be visible: for placeholder_id in last_screen_expected_placeholder_ids: - assert ( - placeholders_visibility_by_id[placeholder_id] is True - ), f"Placeholder '{placeholder_id}' should be visible but isn't" + assert placeholders_visibility_by_id[placeholder_id] is True, ( + f"Placeholder '{placeholder_id}' should be visible but isn't" + f" :: placeholders_visibility_by_id={placeholders_visibility_by_id}" + ) # Ok, now for placeholders that should *not* be visible: - if last_screen_expected_out_of_viewport_placeholder_ids == "others": - # We're simply going to check that all the placeholders that are not in - # `last_screen_expected_placeholder_ids` are not on the screen: - last_screen_expected_out_of_viewport_placeholder_ids = sorted( - tuple( - set(range(placeholders_count)) - - set(last_screen_expected_placeholder_ids) - ) + # We're simply going to check that all the placeholders that are not in + # `last_screen_expected_placeholder_ids` are not on the screen: + last_screen_expected_out_of_viewport_placeholder_ids = sorted( + tuple( + set(range(placeholders_count)) - set(last_screen_expected_placeholder_ids) ) + ) for placeholder_id in last_screen_expected_out_of_viewport_placeholder_ids: - assert ( - placeholders_visibility_by_id[placeholder_id] is False - ), f"Placeholder '{placeholder_id}' should not be visible but is" + assert placeholders_visibility_by_id[placeholder_id] is False, ( + f"Placeholder '{placeholder_id}' should not be visible but is" + f" :: placeholders_visibility_by_id={placeholders_visibility_by_id}" + ) diff --git a/tests/utilities/test_app.py b/tests/utilities/test_app.py index a63cff6c9..2040f87ea 100644 --- a/tests/utilities/test_app.py +++ b/tests/utilities/test_app.py @@ -3,19 +3,25 @@ from __future__ import annotations import asyncio import contextlib import io +import sys +from math import ceil from pathlib import Path from time import monotonic -from typing import AsyncContextManager, cast, ContextManager, Callable +from typing import AsyncContextManager, cast, ContextManager from unittest import mock from rich.console import Console from textual import events, errors -from textual._timer import Timer from textual.app import App, ComposeResult from textual.driver import Driver from textual.geometry import Size +if sys.version_info >= (3, 8): + from typing import Protocol +else: + from typing_extensions import Protocol + # N.B. These classes would better be named TestApp/TestConsole/TestDriver/etc, # but it makes pytest emit warning as it will try to collect them as classes containing test cases :-/ @@ -24,6 +30,12 @@ from textual.geometry import Size CLEAR_SCREEN_SEQUENCE = "\x1bP=1s\x1b\\" +class MockedTimeMoveClockForward(Protocol): + async def __call__(self, *, seconds: float) -> tuple[float, int]: + """Returns the new current (mocked) monotonic time and the number of activated Timers""" + ... + + class AppTest(App): def __init__( self, @@ -64,52 +76,57 @@ class AppTest(App): def in_running_state( self, *, + time_mocking_ticks_granularity_fps: int = 60, # i.e. when moving forward by 1 second we'll do it though 60 ticks waiting_duration_after_initialisation: float = 0.1, - waiting_duration_post_yield: float = 0, - time_acceleration: bool = True, - time_acceleration_factor: float = 10, - # force_timers_tick_after_yield: bool = True, - ) -> AsyncContextManager: + waiting_duration_after_yield: float = 0, + ) -> AsyncContextManager[MockedTimeMoveClockForward]: async def run_app() -> None: await self.process_messages() - if time_acceleration: - waiting_duration_after_initialisation /= time_acceleration_factor - waiting_duration_post_yield /= time_acceleration_factor - - time_acceleration_context: ContextManager = ( - textual_timers_accelerate_time(acceleration_factor=time_acceleration_factor) - if time_acceleration - else contextlib.nullcontext() - ) - @contextlib.asynccontextmanager async def get_running_state_context_manager(): self._set_active() - with time_acceleration_context: + + with mock_textual_timers( + ticks_granularity_fps=time_mocking_ticks_granularity_fps + ) as move_time_forward: run_task = asyncio.create_task(run_app()) - timeout_before_yielding_task = asyncio.create_task( - asyncio.sleep(waiting_duration_after_initialisation) - ) - done, pending = await asyncio.wait( - ( - run_task, - timeout_before_yielding_task, - ), - return_when=asyncio.FIRST_COMPLETED, - ) - if run_task in done or run_task not in pending: - raise RuntimeError( - "TestApp is no longer running after its initialization period" - ) - yield - waiting_duration = max( - waiting_duration_post_yield or 0, - self.screen._update_timer._interval, - ) - await asyncio.sleep(waiting_duration) + await asyncio.sleep(0.001) + # timeout_before_yielding_task = asyncio.create_task( + # asyncio.sleep(waiting_duration_after_initialisation) + # ) + # done, pending = await asyncio.wait( + # ( + # run_task, + # timeout_before_yielding_task, + # ), + # return_when=asyncio.FIRST_COMPLETED, + # ) + # if run_task in done or run_task not in pending: + # raise RuntimeError( + # "TestApp is no longer running after its initialization period" + # ) + + await move_time_forward(seconds=waiting_duration_after_initialisation) + + assert self._driver is not None + + self.force_screen_update() + + yield move_time_forward + + await move_time_forward(seconds=waiting_duration_after_yield) + + self.force_screen_update() + # waiting_duration = max( + # waiting_duration_post_yield or 0, + # self.screen._update_timer._interval, + # ) + # await asyncio.sleep(waiting_duration) + # if force_timers_tick_after_yield: # await textual_timers_force_tick() + assert not run_task.done() await self.shutdown() @@ -124,27 +141,10 @@ class AppTest(App): """Just a commodity shortcut for `async with app.in_running_state(): pass`, for simple cases""" async with self.in_running_state( waiting_duration_after_initialisation=waiting_duration_after_initialisation, - waiting_duration_post_yield=waiting_duration_before_shutdown, + waiting_duration_after_yield=waiting_duration_before_shutdown, ): pass - def run(self): - raise NotImplementedError( - "Use `async with my_test_app.in_running_state()` rather than `my_test_app.run()`" - ) - - @property - def total_capture(self) -> str | None: - return self.console.file.getvalue() - - @property - def last_display_capture(self) -> str | None: - total_capture = self.total_capture - if not total_capture: - return None - last_display_start_index = total_capture.rindex(CLEAR_SCREEN_SEQUENCE) - return total_capture[last_display_start_index:] - def get_char_at(self, x: int, y: int) -> str: """Get the character at the given cell or empty string @@ -175,6 +175,34 @@ class AppTest(App): return segment.text[0] return "" + def force_screen_update(self, *, repaint: bool = True, layout: bool = True) -> None: + try: + self.screen.refresh(repaint=repaint, layout=layout) + self.screen._on_update() + except IndexError: + pass # the app may not have a screen yet + + def on_exception(self, error: Exception) -> None: + # In tests we want the errors to be raised, rather than printed to a Console + raise error + + def run(self): + raise NotImplementedError( + "Use `async with my_test_app.in_running_state()` rather than `my_test_app.run()`" + ) + + @property + def total_capture(self) -> str | None: + return self.console.file.getvalue() + + @property + def last_display_capture(self) -> str | None: + total_capture = self.total_capture + if not total_capture: + return None + last_display_start_index = total_capture.rindex(CLEAR_SCREEN_SEQUENCE) + return total_capture[last_display_start_index:] + @property def console(self) -> ConsoleTest: return self._console @@ -231,43 +259,71 @@ class DriverTest(Driver): pass -async def textual_timers_force_tick() -> None: - timer_instances_tick_tasks: list[asyncio.Task] = [] - for timer in Timer._instances: - task = asyncio.create_task(timer._tick(next_timer=0, count=0)) - timer_instances_tick_tasks.append(task) - await asyncio.wait(timer_instances_tick_tasks) +def mock_textual_timers( + *, + ticks_granularity_fps: int = 60, +) -> ContextManager[MockedTimeMoveClockForward]: + single_tick_duration = 1.0 / ticks_granularity_fps + pending_sleep_events: list[tuple[float, asyncio.Event]] = [] -def textual_timers_accelerate_time( - *, acceleration_factor: float = 10 -) -> ContextManager: @contextlib.contextmanager - def accelerate_time_for_timer_context_manager(): - starting_time = monotonic() + def mock_textual_timers_context_manager(): + # N.B. `start_time` is not used, but it is useful to have when we set breakpoints there :-) + start_time = current_time = monotonic() # Our replacement for "textual._timer.Timer._sleep": - async def timer_sleep(duration: float) -> None: - await asyncio.sleep(duration / acceleration_factor) + async def sleep_mock(duration: float) -> None: + event = asyncio.Event() + target_event_monotonic_time = current_time + duration + pending_sleep_events.append((target_event_monotonic_time, event)) + # Ok, let's wait for this Event + # - which can only be "unlocked" by calls to `move_clock_forward()` + await event.wait() - # Our replacement for "textual._timer.Timer.get_time": - def timer_get_time() -> float: - real_now = monotonic() - real_elapsed_time = real_now - starting_time - accelerated_elapsed_time = real_elapsed_time * acceleration_factor - print( - f"timer_get_time:: accelerated_elapsed_time={accelerated_elapsed_time}" - ) - return starting_time + accelerated_elapsed_time + # Our replacement for "textual._timer.Timer.get_time" and "textual.message.Message._get_time": + def get_time_mock() -> float: + return current_time - with mock.patch("textual._timer.Timer._sleep") as timer_sleep_mock, mock.patch( - "textual._timer.Timer.get_time" - ) as timer_get_time_mock, mock.patch( - "textual.message.Message._get_time" - ) as message_get_time_mock: - timer_sleep_mock.side_effect = timer_sleep - timer_get_time_mock.side_effect = timer_get_time - message_get_time_mock.side_effect = timer_get_time - yield + async def move_clock_forward(*, seconds: float) -> tuple[float, int]: + nonlocal current_time, start_time - return accelerate_time_for_timer_context_manager() + ticks_count = ceil(seconds * ticks_granularity_fps) + activated_timers_count_total = 0 + for tick_counter in range(ticks_count): + current_time += single_tick_duration + activated_timers_count_total += check_sleep_timers_to_activate() + + # Let's give an opportunity to asyncio-related stuff to happen, + # now that we unlocked some occurrences of `await sleep(duration)`: + await asyncio.sleep(0.0001) + + return current_time, activated_timers_count_total + + def check_sleep_timers_to_activate() -> int: + nonlocal pending_sleep_events + + activated_timers_count = 0 + for i, (target_event_monotonic_time, event) in enumerate( + pending_sleep_events + ): + if target_event_monotonic_time < current_time: + continue + # Right, let's release this waiting event! + event.set() + activated_timers_count += 1 + # ...and remove it from our pending sleep events list: + del pending_sleep_events[i] + + return activated_timers_count + + with mock.patch("textual._timer._TIMERS_CAN_SKIP", new=False), mock.patch( + "textual._timer.Timer._sleep", side_effect=sleep_mock + ), mock.patch( + "textual._timer.Timer.get_time", side_effect=get_time_mock + ), mock.patch( + "textual.message.Message._get_time", side_effect=get_time_mock + ): + yield move_clock_forward + + return mock_textual_timers_context_manager() From 5789816333538a78c76e6dd67eb1f80bbd50b998 Mon Sep 17 00:00:00 2001 From: Olivier Philippon Date: Tue, 17 May 2022 09:54:14 +0100 Subject: [PATCH 03/10] [App] Integration tests now work on Windows too --- src/textual/_context.py | 5 ++ src/textual/dom.py | 7 ++- src/textual/message_pump.py | 24 ++++++-- src/textual/screen.py | 2 +- tests/utilities/test_app.py | 106 ++++++++++++++++++++---------------- 5 files changed, 89 insertions(+), 55 deletions(-) diff --git a/src/textual/_context.py b/src/textual/_context.py index e16817631..04b264d33 100644 --- a/src/textual/_context.py +++ b/src/textual/_context.py @@ -5,4 +5,9 @@ from contextvars import ContextVar if TYPE_CHECKING: from .app import App + +class NoActiveAppError(RuntimeError): + pass + + active_app: ContextVar["App"] = ContextVar("active_app") diff --git a/src/textual/dom.py b/src/textual/dom.py index 760bc3def..700534398 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -9,6 +9,7 @@ from rich.style import Style from rich.text import Text from rich.tree import Tree +from ._context import NoActiveAppError from ._node_list import NodeList from .color import Color from .css._error_tools import friendly_list @@ -463,7 +464,7 @@ class DOMNode(MessagePump): try: self.app.stylesheet.update(self.app, animate=True) self.refresh() - except LookupError: + except NoActiveAppError: pass def remove_class(self, *class_names: str) -> None: @@ -477,7 +478,7 @@ class DOMNode(MessagePump): try: self.app.stylesheet.update(self.app, animate=True) self.refresh() - except LookupError: + except NoActiveAppError: pass def toggle_class(self, *class_names: str) -> None: @@ -491,7 +492,7 @@ class DOMNode(MessagePump): try: self.app.stylesheet.update(self.app, animate=True) self.refresh() - except LookupError: + except NoActiveAppError: pass def has_pseudo_class(self, *class_names: str) -> bool: diff --git a/src/textual/message_pump.py b/src/textual/message_pump.py index 02d533e4c..0c04dbbcd 100644 --- a/src/textual/message_pump.py +++ b/src/textual/message_pump.py @@ -12,7 +12,7 @@ from . import events from . import log from ._timer import Timer, TimerCallback from ._callback import invoke -from ._context import active_app +from ._context import active_app, NoActiveAppError from .message import Message from . import messages @@ -74,8 +74,16 @@ class MessagePump: @property def app(self) -> "App": - """Get the current app.""" - return active_app.get() + """ + Get the current app. + + Raises: + NoActiveAppError: if no active app could be found for the current asyncio context + """ + try: + return active_app.get() + except LookupError: + raise NoActiveAppError() @property def is_parent_active(self): @@ -152,7 +160,13 @@ class MessagePump: pause: bool = False, ) -> Timer: timer = Timer( - self, delay, self, name=name, callback=callback, repeat=0, pause=pause + self, + delay, + self, + name=name or f"set_timer#{Timer._timer_count}", + callback=callback, + repeat=0, + pause=pause, ) self._child_tasks.add(timer.start()) return timer @@ -170,7 +184,7 @@ class MessagePump: self, interval, self, - name=name, + name=name or f"set_interval#{Timer._timer_count}", callback=callback, repeat=repeat or None, pause=pause, diff --git a/src/textual/screen.py b/src/textual/screen.py index a33b236f9..6f9725647 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -169,7 +169,7 @@ class Screen(Widget): def on_mount(self, event: events.Mount) -> None: self._update_timer = self.set_interval( - UPDATE_PERIOD, self._on_update, pause=True + UPDATE_PERIOD, self._on_update, name="screen_update", pause=True ) async def on_resize(self, event: events.Resize) -> None: diff --git a/tests/utilities/test_app.py b/tests/utilities/test_app.py index 2040f87ea..9f624d715 100644 --- a/tests/utilities/test_app.py +++ b/tests/utilities/test_app.py @@ -13,7 +13,8 @@ from unittest import mock from rich.console import Console from textual import events, errors -from textual.app import App, ComposeResult +from textual.app import App, ComposeResult, WINDOWS +from textual._context import active_app from textual.driver import Driver from textual.geometry import Size @@ -77,7 +78,7 @@ class AppTest(App): self, *, time_mocking_ticks_granularity_fps: int = 60, # i.e. when moving forward by 1 second we'll do it though 60 ticks - waiting_duration_after_initialisation: float = 0.1, + waiting_duration_after_initialisation: float = 1, waiting_duration_after_yield: float = 0, ) -> AsyncContextManager[MockedTimeMoveClockForward]: async def run_app() -> None: @@ -85,50 +86,35 @@ class AppTest(App): @contextlib.asynccontextmanager async def get_running_state_context_manager(): - self._set_active() - with mock_textual_timers( ticks_granularity_fps=time_mocking_ticks_granularity_fps - ) as move_time_forward: + ) as move_clock_forward: run_task = asyncio.create_task(run_app()) - await asyncio.sleep(0.001) - # timeout_before_yielding_task = asyncio.create_task( - # asyncio.sleep(waiting_duration_after_initialisation) - # ) - # done, pending = await asyncio.wait( - # ( - # run_task, - # timeout_before_yielding_task, - # ), - # return_when=asyncio.FIRST_COMPLETED, - # ) - # if run_task in done or run_task not in pending: - # raise RuntimeError( - # "TestApp is no longer running after its initialization period" - # ) - await move_time_forward(seconds=waiting_duration_after_initialisation) + # We have to do this because `run_app()` is running in its own async task, and our test is going to + # run in this one - so the app must also be the active App in our current context: + self._set_active() + await move_clock_forward(seconds=waiting_duration_after_initialisation) + # make sure the App has entered its main loop at this stage: assert self._driver is not None - self.force_screen_update() + await self.force_screen_update() - yield move_time_forward + # And now it's time to pass the torch on to the test function! + # We provide the `move_clock_forward` function to it, + # so it can also do some time-based Textual stuff if it needs to: + yield move_clock_forward - await move_time_forward(seconds=waiting_duration_after_yield) + await move_clock_forward(seconds=waiting_duration_after_yield) - self.force_screen_update() - # waiting_duration = max( - # waiting_duration_post_yield or 0, - # self.screen._update_timer._interval, - # ) - # await asyncio.sleep(waiting_duration) + # Make sure our screen is up to date before exiting the context manager, + # so tests using our `last_display_capture` for example can assert things on an up to date screen: + await self.force_screen_update() - # if force_timers_tick_after_yield: - # await textual_timers_force_tick() - - assert not run_task.done() - await self.shutdown() + # End of simulated time: we just shut down ourselves: + assert not run_task.done() + await self.shutdown() return get_running_state_context_manager() @@ -175,12 +161,17 @@ class AppTest(App): return segment.text[0] return "" - def force_screen_update(self, *, repaint: bool = True, layout: bool = True) -> None: + async def force_screen_update( + self, *, repaint: bool = True, layout: bool = True + ) -> None: try: - self.screen.refresh(repaint=repaint, layout=layout) - self.screen._on_update() + screen = self.screen except IndexError: - pass # the app may not have a screen yet + return # the app may not have a screen yet + screen.refresh(repaint=repaint, layout=layout) + screen._on_update() + + await let_asyncio_process_some_events() def on_exception(self, error: Exception) -> None: # In tests we want the errors to be raised, rather than printed to a Console @@ -191,6 +182,10 @@ class AppTest(App): "Use `async with my_test_app.in_running_state()` rather than `my_test_app.run()`" ) + @property + def active_app(self) -> App | None: + return active_app.get() + @property def total_capture(self) -> str | None: return self.console.file.getvalue() @@ -259,6 +254,16 @@ class DriverTest(Driver): pass +# > The resolution of the monotonic clock on Windows is usually around 15.6 msec. +# > The best resolution is 0.5 msec. +# @link https://docs.python.org/3/library/asyncio-platforms.html: +ASYNCIO_EVENTS_PROCESSING_REQUIRED_PERIOD = 0.025 if WINDOWS else 0.002 + + +async def let_asyncio_process_some_events() -> None: + await asyncio.sleep(ASYNCIO_EVENTS_PROCESSING_REQUIRED_PERIOD) + + def mock_textual_timers( *, ticks_granularity_fps: int = 60, @@ -278,11 +283,17 @@ def mock_textual_timers( target_event_monotonic_time = current_time + duration pending_sleep_events.append((target_event_monotonic_time, event)) # Ok, let's wait for this Event - # - which can only be "unlocked" by calls to `move_clock_forward()` + # (which can only be "unlocked" by calls to `move_clock_forward()`) await event.wait() # Our replacement for "textual._timer.Timer.get_time" and "textual.message.Message._get_time": def get_time_mock() -> float: + nonlocal current_time + + # let's make the time advance slightly between 2 consecutive calls of this function, + # within the same order of magnitude than 2 consecutive calls to ` timer.monotonic()`: + current_time += 1.1e-06 + return current_time async def move_clock_forward(*, seconds: float) -> tuple[float, int]: @@ -292,11 +303,14 @@ def mock_textual_timers( activated_timers_count_total = 0 for tick_counter in range(ticks_count): current_time += single_tick_duration - activated_timers_count_total += check_sleep_timers_to_activate() + activated_timers_count = check_sleep_timers_to_activate() + activated_timers_count_total += activated_timers_count + # Let's give an opportunity to asyncio-related stuff to happen, + # now that we likely unlocked some occurrences of `await sleep(duration)`: + if activated_timers_count: + await let_asyncio_process_some_events() - # Let's give an opportunity to asyncio-related stuff to happen, - # now that we unlocked some occurrences of `await sleep(duration)`: - await asyncio.sleep(0.0001) + await let_asyncio_process_some_events() return current_time, activated_timers_count_total @@ -307,8 +321,8 @@ def mock_textual_timers( for i, (target_event_monotonic_time, event) in enumerate( pending_sleep_events ): - if target_event_monotonic_time < current_time: - continue + if current_time < target_event_monotonic_time: + continue # not time for you yet, dear awaiter... # Right, let's release this waiting event! event.set() activated_timers_count += 1 From 4549f2d4783713253ef7c93817aecd1217fabdac Mon Sep 17 00:00:00 2001 From: Olivier Philippon Date: Tue, 17 May 2022 17:07:06 +0100 Subject: [PATCH 04/10] [clock] Add a centralised Clock, responsible for anything related to time This makes time quite easier to mock during integration tests :-) --- pyproject.toml | 2 +- sandbox/scroll_to_widget.py | 2 +- src/textual/_animator.py | 9 +- src/textual/_clock.py | 58 +++++++++++ src/textual/_timer.py | 27 ++--- src/textual/message.py | 10 +- tests/utilities/test_app.py | 198 +++++++++++++++++++----------------- 7 files changed, 180 insertions(+), 126 deletions(-) create mode 100644 src/textual/_clock.py diff --git a/pyproject.toml b/pyproject.toml index f0ac188a2..2ed370665 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -49,7 +49,7 @@ includes = "src" asyncio_mode = "auto" testpaths = ["tests"] markers = [ - "integration_test: marks tests as slow integration tests(deselect with '-m \"not integration_test\"')", + "integration_test: marks tests as slow integration tests (deselect with '-m \"not integration_test\"')", ] [build-system] diff --git a/sandbox/scroll_to_widget.py b/sandbox/scroll_to_widget.py index 81b0bf83c..5e75847cd 100644 --- a/sandbox/scroll_to_widget.py +++ b/sandbox/scroll_to_widget.py @@ -35,7 +35,7 @@ class Introduction(Widget): } """ - def render(self) -> RenderableType: + def render(self, styles) -> RenderableType: return Text( "Press keys 0 to 9 to scroll to the Placeholder with that ID.", justify="center", diff --git a/src/textual/_animator.py b/src/textual/_animator.py index 0c031b046..9b5c59a47 100644 --- a/src/textual/_animator.py +++ b/src/textual/_animator.py @@ -7,6 +7,7 @@ from typing import Any, Callable, TypeVar from dataclasses import dataclass +from . import _clock from ._easing import DEFAULT_EASING, EASING from ._timer import Timer from ._types import MessageTarget @@ -179,9 +180,9 @@ class Animator: raise AttributeError( f"Can't animate attribute {attribute!r} on {obj!r}; attribute does not exist" ) - assert not all( - (duration, speed) - ), "An Animation should have a duration OR a speed, received both" + assert (duration is not None and speed is None) or ( + duration is None and speed is not None + ), "An Animation should have a duration OR a speed" if final_value is ...: final_value = value @@ -247,4 +248,4 @@ class Animator: """Get the current wall clock time, via the internal Timer.""" # N.B. We could remove this method and always call `self._timer.get_time()` internally, # but it's handy to have in mocking situations - return self._timer.get_time() + return _clock.get_time() diff --git a/src/textual/_clock.py b/src/textual/_clock.py new file mode 100644 index 000000000..339b720d1 --- /dev/null +++ b/src/textual/_clock.py @@ -0,0 +1,58 @@ +import asyncio +from time import monotonic + + +""" +A module that serves as the single source of truth for everything time-related in a Textual app. +Having this logic centralised makes it easier to simulate time in integration tests, +by mocking the few functions exposed by this module. +""" + + +# N.B. This class and its singleton instance have to be hidden APIs because we want to be able to mock time, +# even for Python modules that imported functions such as `get_time` *before* we mocked this internal _Clock. +# (so mocking public APIs such as `get_time` wouldn't affect direct references to then that were done during imports) +class _Clock: + def get_time(self) -> float: + return monotonic() + + async def aget_time(self) -> float: + return self.get_time() + + async def sleep(self, seconds: float) -> None: + await asyncio.sleep(seconds) + + +# That's our target for mocking time! :-) +_clock = _Clock() + + +def get_time() -> float: + """ + Get the current wall clock time. + + Returns: + float: the value (in fractional seconds) of a monotonic clock, i.e. a clock that cannot go backwards. + """ + return _clock.get_time() + + +async def aget_time() -> float: + """ + Asynchronous version of `get_time`. Useful in situations where we want asyncio to be + able to "do things" elsewhere right before we fetch the time. + + Returns: + float: the value (in fractional seconds) of a monotonic clock, i.e. a clock that cannot go backwards. + """ + return await _clock.aget_time() + + +async def sleep(seconds: float) -> None: + """ + Coroutine that completes after a given time (in seconds). + + Args: + seconds (float): the duration we should wait for before unblocking the awaiter + """ + return await _clock.sleep(seconds) diff --git a/src/textual/_timer.py b/src/textual/_timer.py index ec8b3a63b..a932be047 100644 --- a/src/textual/_timer.py +++ b/src/textual/_timer.py @@ -5,23 +5,19 @@ import weakref from asyncio import ( CancelledError, Event, - sleep, Task, ) -from time import monotonic from typing import Awaitable, Callable, Union from rich.repr import Result, rich_repr from . import events from ._callback import invoke +from . import _clock from ._types import MessageTarget TimerCallback = Union[Callable[[], Awaitable[None]], Callable[[], None]] -# /!\ This should only be changed in an "integration tests" context, in which we mock time -_TIMERS_CAN_SKIP: bool = True - class EventTargetGone(Exception): pass @@ -106,32 +102,23 @@ class Timer: """Result a paused timer.""" self._active.set() - @staticmethod - def get_time() -> float: - """Get the current wall clock time.""" - # N.B. This method will likely be a mocking target in integration tests. - return monotonic() - - @staticmethod - async def _sleep(duration: float) -> None: - # N.B. This method will likely be a mocking target in integration tests. - await sleep(duration) - async def _run(self) -> None: """Run the timer.""" count = 0 _repeat = self._repeat _interval = self._interval - start = self.get_time() + start = await _clock.aget_time() try: while _repeat is None or count <= _repeat: next_timer = start + ((count + 1) * _interval) - if self._skip and _TIMERS_CAN_SKIP and next_timer < self.get_time(): + now = await _clock.aget_time() + if self._skip and next_timer < now: count += 1 continue - wait_time = max(0, next_timer - self.get_time()) + now = await _clock.aget_time() + wait_time = max(0, next_timer - now) if wait_time: - await self._sleep(wait_time) + await _clock.sleep(wait_time) count += 1 try: await self._tick(next_timer=next_timer, count=count) diff --git a/src/textual/message.py b/src/textual/message.py index eef6ff48d..10a480cd6 100644 --- a/src/textual/message.py +++ b/src/textual/message.py @@ -1,10 +1,10 @@ from __future__ import annotations -from time import monotonic from typing import ClassVar import rich.repr +from . import _clock from .case import camel_to_snake from ._types import MessageTarget @@ -39,7 +39,7 @@ class Message: self.sender = sender self.name = camel_to_snake(self.__class__.__name__.replace("Message", "")) - self.time = self._get_time() + self.time = _clock.get_time() self._forwarded = False self._no_default_action = False self._stop_propagation = False @@ -99,9 +99,3 @@ class Message: """ self._stop_propagation = stop return self - - @staticmethod - def _get_time() -> float: - """Get the current wall clock time.""" - # N.B. This method will likely be a mocking target in integration tests. - return monotonic() diff --git a/tests/utilities/test_app.py b/tests/utilities/test_app.py index 9f624d715..f53c96469 100644 --- a/tests/utilities/test_app.py +++ b/tests/utilities/test_app.py @@ -3,7 +3,6 @@ from __future__ import annotations import asyncio import contextlib import io -import sys from math import ceil from pathlib import Path from time import monotonic @@ -13,16 +12,11 @@ from unittest import mock from rich.console import Console from textual import events, errors +from textual._clock import _Clock from textual.app import App, ComposeResult, WINDOWS from textual._context import active_app from textual.driver import Driver -from textual.geometry import Size - -if sys.version_info >= (3, 8): - from typing import Protocol -else: - from typing_extensions import Protocol - +from textual.geometry import Size, Region # N.B. These classes would better be named TestApp/TestConsole/TestDriver/etc, # but it makes pytest emit warning as it will try to collect them as classes containing test cases :-/ @@ -31,12 +25,6 @@ else: CLEAR_SCREEN_SEQUENCE = "\x1bP=1s\x1b\\" -class MockedTimeMoveClockForward(Protocol): - async def __call__(self, *, seconds: float) -> tuple[float, int]: - """Returns the new current (mocked) monotonic time and the number of activated Timers""" - ... - - class AppTest(App): def __init__( self, @@ -80,7 +68,7 @@ class AppTest(App): time_mocking_ticks_granularity_fps: int = 60, # i.e. when moving forward by 1 second we'll do it though 60 ticks waiting_duration_after_initialisation: float = 1, waiting_duration_after_yield: float = 0, - ) -> AsyncContextManager[MockedTimeMoveClockForward]: + ) -> AsyncContextManager[ClockMock]: async def run_app() -> None: await self.process_messages() @@ -88,29 +76,33 @@ class AppTest(App): async def get_running_state_context_manager(): with mock_textual_timers( ticks_granularity_fps=time_mocking_ticks_granularity_fps - ) as move_clock_forward: + ) as clock_mock: run_task = asyncio.create_task(run_app()) # We have to do this because `run_app()` is running in its own async task, and our test is going to # run in this one - so the app must also be the active App in our current context: self._set_active() - await move_clock_forward(seconds=waiting_duration_after_initialisation) + await clock_mock.move_clock_forward( + seconds=waiting_duration_after_initialisation + ) # make sure the App has entered its main loop at this stage: assert self._driver is not None - await self.force_screen_update() + await self.force_full_screen_update() # And now it's time to pass the torch on to the test function! # We provide the `move_clock_forward` function to it, # so it can also do some time-based Textual stuff if it needs to: - yield move_clock_forward + yield clock_mock - await move_clock_forward(seconds=waiting_duration_after_yield) + await clock_mock.move_clock_forward( + seconds=waiting_duration_after_yield + ) # Make sure our screen is up to date before exiting the context manager, # so tests using our `last_display_capture` for example can assert things on an up to date screen: - await self.force_screen_update() + await self.force_full_screen_update() # End of simulated time: we just shut down ourselves: assert not run_task.done() @@ -161,14 +153,21 @@ class AppTest(App): return segment.text[0] return "" - async def force_screen_update( + async def force_full_screen_update( self, *, repaint: bool = True, layout: bool = True ) -> None: try: screen = self.screen except IndexError: return # the app may not have a screen yet + + # We artificially tell the Compositor that the whole area should be refreshed + screen._compositor._dirty_regions = { + Region(0, 0, screen.size.width, screen.size.height), + } screen.refresh(repaint=repaint, layout=layout) + # We also have to make sure we have at least one dirty widget, or `screen._on_update()` will early return: + screen._dirty_widgets.add(screen) screen._on_update() await let_asyncio_process_some_events() @@ -254,6 +253,9 @@ class DriverTest(Driver): pass +# It seems that we have to give _way more_ time to `asyncio` on Windows in order to see our different awaiters +# properly triggered when we pause our own "move clock forward" loop. +# It could be caused by the fact that the time resolution for `asyncio` on this platform seems rather low: # > The resolution of the monotonic clock on Windows is usually around 15.6 msec. # > The best resolution is 0.5 msec. # @link https://docs.python.org/3/library/asyncio-platforms.html: @@ -264,80 +266,92 @@ async def let_asyncio_process_some_events() -> None: await asyncio.sleep(ASYNCIO_EVENTS_PROCESSING_REQUIRED_PERIOD) +class ClockMock(_Clock): + def __init__( + self, + *, + ticks_granularity_fps: int = 60, + ): + self._ticks_granularity_fps = ticks_granularity_fps + self._single_tick_duration = 1.0 / ticks_granularity_fps + self._start_time = self._current_time = None + self._pending_sleep_events: list[tuple[float, asyncio.Event]] = [] + + def get_time(self) -> float: + if self._current_time is None: + self._start_clock() + + # let's make the time advance _very_ slightly between 2 consecutive calls of this function, + # within the same order of magnitude than 2 consecutive calls to ` timer.monotonic()`: + self._current_time += 1.1e-06 + + return self._current_time + + async def sleep(self, seconds: float) -> None: + event = asyncio.Event() + target_event_monotonic_time = self._current_time + seconds + self._pending_sleep_events.append((target_event_monotonic_time, event)) + # Ok, let's wait for this Event + # (which can only be "unlocked" by calls to `move_clock_forward()`) + await event.wait() + + async def move_clock_forward(self, *, seconds: float) -> tuple[float, int]: + """ + Artificially moves the Textual clock forward. + + Args: + seconds: for each second we will artificially tick `ticks_granularity_fps` times + + Returns: + tuple[float, int]: a tuple giving the new mocked current time and the number of sleep awaiters + that were unblocked by this call to `move_clock_forward` + """ + if self._current_time is None: + self._start_clock() + + ticks_count = ceil(seconds * self._ticks_granularity_fps) + activated_timers_count_total = 0 + for tick_counter in range(ticks_count): + self._current_time += self._single_tick_duration + activated_timers_count = self._check_sleep_timers_to_activate() + activated_timers_count_total += activated_timers_count + # Let's give an opportunity to asyncio-related stuff to happen, + # now that we likely unlocked some occurrences of `await sleep(duration)`: + if activated_timers_count: + await let_asyncio_process_some_events() + + await let_asyncio_process_some_events() + + return self._current_time, activated_timers_count_total + + def _start_clock(self) -> None: + # N.B. `start_time` is not used, but it is useful to have when we set breakpoints there :-) + self._start_time = self._current_time = monotonic() + + def _check_sleep_timers_to_activate(self) -> int: + activated_timers_count = 0 + for i, (target_event_monotonic_time, event) in enumerate( + self._pending_sleep_events + ): + if self._current_time < target_event_monotonic_time: + continue # not time for you yet, dear awaiter... + # Right, let's release this waiting event! + event.set() + activated_timers_count += 1 + # ...and remove it from our pending sleep events list: + del self._pending_sleep_events[i] + + return activated_timers_count + + def mock_textual_timers( *, ticks_granularity_fps: int = 60, -) -> ContextManager[MockedTimeMoveClockForward]: - single_tick_duration = 1.0 / ticks_granularity_fps - - pending_sleep_events: list[tuple[float, asyncio.Event]] = [] - +) -> ContextManager[ClockMock]: @contextlib.contextmanager def mock_textual_timers_context_manager(): - # N.B. `start_time` is not used, but it is useful to have when we set breakpoints there :-) - start_time = current_time = monotonic() - - # Our replacement for "textual._timer.Timer._sleep": - async def sleep_mock(duration: float) -> None: - event = asyncio.Event() - target_event_monotonic_time = current_time + duration - pending_sleep_events.append((target_event_monotonic_time, event)) - # Ok, let's wait for this Event - # (which can only be "unlocked" by calls to `move_clock_forward()`) - await event.wait() - - # Our replacement for "textual._timer.Timer.get_time" and "textual.message.Message._get_time": - def get_time_mock() -> float: - nonlocal current_time - - # let's make the time advance slightly between 2 consecutive calls of this function, - # within the same order of magnitude than 2 consecutive calls to ` timer.monotonic()`: - current_time += 1.1e-06 - - return current_time - - async def move_clock_forward(*, seconds: float) -> tuple[float, int]: - nonlocal current_time, start_time - - ticks_count = ceil(seconds * ticks_granularity_fps) - activated_timers_count_total = 0 - for tick_counter in range(ticks_count): - current_time += single_tick_duration - activated_timers_count = check_sleep_timers_to_activate() - activated_timers_count_total += activated_timers_count - # Let's give an opportunity to asyncio-related stuff to happen, - # now that we likely unlocked some occurrences of `await sleep(duration)`: - if activated_timers_count: - await let_asyncio_process_some_events() - - await let_asyncio_process_some_events() - - return current_time, activated_timers_count_total - - def check_sleep_timers_to_activate() -> int: - nonlocal pending_sleep_events - - activated_timers_count = 0 - for i, (target_event_monotonic_time, event) in enumerate( - pending_sleep_events - ): - if current_time < target_event_monotonic_time: - continue # not time for you yet, dear awaiter... - # Right, let's release this waiting event! - event.set() - activated_timers_count += 1 - # ...and remove it from our pending sleep events list: - del pending_sleep_events[i] - - return activated_timers_count - - with mock.patch("textual._timer._TIMERS_CAN_SKIP", new=False), mock.patch( - "textual._timer.Timer._sleep", side_effect=sleep_mock - ), mock.patch( - "textual._timer.Timer.get_time", side_effect=get_time_mock - ), mock.patch( - "textual.message.Message._get_time", side_effect=get_time_mock - ): - yield move_clock_forward + clock_mock = ClockMock(ticks_granularity_fps=ticks_granularity_fps) + with mock.patch("textual._clock._clock", new=clock_mock): + yield clock_mock return mock_textual_timers_context_manager() From aed252874be0c15d4a81d5984108111801e27411 Mon Sep 17 00:00:00 2001 From: Olivier Philippon Date: Thu, 19 May 2022 09:55:38 +0100 Subject: [PATCH 05/10] [clock] Address PR feedback after the initial implementation of the centralised Clock --- src/textual/_animator.py | 2 +- src/textual/_clock.py | 16 +++++----- src/textual/_timer.py | 6 ++-- src/textual/message.py | 2 +- tests/utilities/test_app.py | 60 ++++++++++++++++--------------------- 5 files changed, 39 insertions(+), 47 deletions(-) diff --git a/src/textual/_animator.py b/src/textual/_animator.py index 9b5c59a47..dbc5d9670 100644 --- a/src/textual/_animator.py +++ b/src/textual/_animator.py @@ -248,4 +248,4 @@ class Animator: """Get the current wall clock time, via the internal Timer.""" # N.B. We could remove this method and always call `self._timer.get_time()` internally, # but it's handy to have in mocking situations - return _clock.get_time() + return _clock.get_time_no_wait() diff --git a/src/textual/_clock.py b/src/textual/_clock.py index 339b720d1..4e1ac2224 100644 --- a/src/textual/_clock.py +++ b/src/textual/_clock.py @@ -13,11 +13,11 @@ by mocking the few functions exposed by this module. # even for Python modules that imported functions such as `get_time` *before* we mocked this internal _Clock. # (so mocking public APIs such as `get_time` wouldn't affect direct references to then that were done during imports) class _Clock: - def get_time(self) -> float: - return monotonic() + async def get_time(self) -> float: + return self.get_time_no_wait() - async def aget_time(self) -> float: - return self.get_time() + def get_time_no_wait(self) -> float: + return monotonic() async def sleep(self, seconds: float) -> None: await asyncio.sleep(seconds) @@ -27,17 +27,17 @@ class _Clock: _clock = _Clock() -def get_time() -> float: +def get_time_no_wait() -> float: """ Get the current wall clock time. Returns: float: the value (in fractional seconds) of a monotonic clock, i.e. a clock that cannot go backwards. """ - return _clock.get_time() + return _clock.get_time_no_wait() -async def aget_time() -> float: +async def get_time() -> float: """ Asynchronous version of `get_time`. Useful in situations where we want asyncio to be able to "do things" elsewhere right before we fetch the time. @@ -45,7 +45,7 @@ async def aget_time() -> float: Returns: float: the value (in fractional seconds) of a monotonic clock, i.e. a clock that cannot go backwards. """ - return await _clock.aget_time() + return await _clock.get_time() async def sleep(seconds: float) -> None: diff --git a/src/textual/_timer.py b/src/textual/_timer.py index a932be047..3231fe016 100644 --- a/src/textual/_timer.py +++ b/src/textual/_timer.py @@ -107,15 +107,15 @@ class Timer: count = 0 _repeat = self._repeat _interval = self._interval - start = await _clock.aget_time() + start = _clock.get_time_no_wait() try: while _repeat is None or count <= _repeat: next_timer = start + ((count + 1) * _interval) - now = await _clock.aget_time() + now = await _clock.get_time() if self._skip and next_timer < now: count += 1 continue - now = await _clock.aget_time() + now = await _clock.get_time() wait_time = max(0, next_timer - now) if wait_time: await _clock.sleep(wait_time) diff --git a/src/textual/message.py b/src/textual/message.py index 10a480cd6..13bd9c537 100644 --- a/src/textual/message.py +++ b/src/textual/message.py @@ -39,7 +39,7 @@ class Message: self.sender = sender self.name = camel_to_snake(self.__class__.__name__.replace("Message", "")) - self.time = _clock.get_time() + self.time = _clock.get_time_no_wait() self._forwarded = False self._no_default_action = False self._stop_propagation = False diff --git a/tests/utilities/test_app.py b/tests/utilities/test_app.py index f53c96469..7c567a874 100644 --- a/tests/utilities/test_app.py +++ b/tests/utilities/test_app.py @@ -83,9 +83,7 @@ class AppTest(App): # run in this one - so the app must also be the active App in our current context: self._set_active() - await clock_mock.move_clock_forward( - seconds=waiting_duration_after_initialisation - ) + await clock_mock.advance_clock(waiting_duration_after_initialisation) # make sure the App has entered its main loop at this stage: assert self._driver is not None @@ -96,12 +94,10 @@ class AppTest(App): # so it can also do some time-based Textual stuff if it needs to: yield clock_mock - await clock_mock.move_clock_forward( - seconds=waiting_duration_after_yield - ) + await clock_mock.advance_clock(waiting_duration_after_yield) - # Make sure our screen is up to date before exiting the context manager, - # so tests using our `last_display_capture` for example can assert things on an up to date screen: + # Make sure our screen is up-to-date before exiting the context manager, + # so tests using our `last_display_capture` for example can assert things on a fully refreshed screen: await self.force_full_screen_update() # End of simulated time: we just shut down ourselves: @@ -267,66 +263,62 @@ async def let_asyncio_process_some_events() -> None: class ClockMock(_Clock): + # To avoid issues with floats we will store the current time as an integer internally. + # Tenths of microseconds should be a good enough granularity: + TIME_RESOLUTION = 10_000_000 + def __init__( self, *, ticks_granularity_fps: int = 60, ): self._ticks_granularity_fps = ticks_granularity_fps - self._single_tick_duration = 1.0 / ticks_granularity_fps - self._start_time = self._current_time = None - self._pending_sleep_events: list[tuple[float, asyncio.Event]] = [] + self._single_tick_duration = int(self.TIME_RESOLUTION / ticks_granularity_fps) + self._start_time: int = -1 + self._current_time: int = -1 + self._pending_sleep_events: list[tuple[int, asyncio.Event]] = [] - def get_time(self) -> float: - if self._current_time is None: + def get_time_no_wait(self) -> float: + if self._current_time == -1: self._start_clock() - # let's make the time advance _very_ slightly between 2 consecutive calls of this function, - # within the same order of magnitude than 2 consecutive calls to ` timer.monotonic()`: - self._current_time += 1.1e-06 - - return self._current_time + return self._current_time / self.TIME_RESOLUTION async def sleep(self, seconds: float) -> None: event = asyncio.Event() - target_event_monotonic_time = self._current_time + seconds + internal_waiting_duration = int(seconds * self.TIME_RESOLUTION) + target_event_monotonic_time = self._current_time + internal_waiting_duration self._pending_sleep_events.append((target_event_monotonic_time, event)) # Ok, let's wait for this Event - # (which can only be "unlocked" by calls to `move_clock_forward()`) + # (which can only be "unlocked" by calls to `advance_clock()`) await event.wait() - async def move_clock_forward(self, *, seconds: float) -> tuple[float, int]: + async def advance_clock(self, seconds: float) -> None: """ - Artificially moves the Textual clock forward. + Artificially advance the Textual clock forward. Args: seconds: for each second we will artificially tick `ticks_granularity_fps` times - - Returns: - tuple[float, int]: a tuple giving the new mocked current time and the number of sleep awaiters - that were unblocked by this call to `move_clock_forward` """ - if self._current_time is None: + if self._current_time == -1: self._start_clock() ticks_count = ceil(seconds * self._ticks_granularity_fps) - activated_timers_count_total = 0 + activated_timers_count_total = 0 # useful when debugging this code :-) for tick_counter in range(ticks_count): self._current_time += self._single_tick_duration activated_timers_count = self._check_sleep_timers_to_activate() activated_timers_count_total += activated_timers_count - # Let's give an opportunity to asyncio-related stuff to happen, - # now that we likely unlocked some occurrences of `await sleep(duration)`: + # Now that we likely unlocked some occurrences of `await sleep(duration)`, + # let's give an opportunity to asyncio-related stuff to happen: if activated_timers_count: await let_asyncio_process_some_events() await let_asyncio_process_some_events() - return self._current_time, activated_timers_count_total - def _start_clock(self) -> None: - # N.B. `start_time` is not used, but it is useful to have when we set breakpoints there :-) - self._start_time = self._current_time = monotonic() + # N.B. `start_time` is not actually used, but it is useful to have when we set breakpoints there :-) + self._start_time = self._current_time = int(monotonic() * self.TIME_RESOLUTION) def _check_sleep_timers_to_activate(self) -> int: activated_timers_count = 0 From 0ee9c57f597b26088c076e934f7f4492c869ae6d Mon Sep 17 00:00:00 2001 From: Olivier Philippon Date: Fri, 20 May 2022 14:45:40 +0100 Subject: [PATCH 06/10] [clock] Address PR feedback after the initial implementation of the centralised Clock, episode II --- tests/utilities/test_app.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/utilities/test_app.py b/tests/utilities/test_app.py index 7c567a874..25948b4aa 100644 --- a/tests/utilities/test_app.py +++ b/tests/utilities/test_app.py @@ -255,7 +255,7 @@ class DriverTest(Driver): # > The resolution of the monotonic clock on Windows is usually around 15.6 msec. # > The best resolution is 0.5 msec. # @link https://docs.python.org/3/library/asyncio-platforms.html: -ASYNCIO_EVENTS_PROCESSING_REQUIRED_PERIOD = 0.025 if WINDOWS else 0.002 +ASYNCIO_EVENTS_PROCESSING_REQUIRED_PERIOD = 0.025 if WINDOWS else 0.005 async def let_asyncio_process_some_events() -> None: @@ -276,7 +276,9 @@ class ClockMock(_Clock): self._single_tick_duration = int(self.TIME_RESOLUTION / ticks_granularity_fps) self._start_time: int = -1 self._current_time: int = -1 - self._pending_sleep_events: list[tuple[int, asyncio.Event]] = [] + # For each call to our `sleep` method we will store an asyncio.Event + # and the time at which we should trigger it: + self._pending_sleep_events: dict[asyncio.Event, int] = {} def get_time_no_wait(self) -> float: if self._current_time == -1: @@ -288,7 +290,7 @@ class ClockMock(_Clock): event = asyncio.Event() internal_waiting_duration = int(seconds * self.TIME_RESOLUTION) target_event_monotonic_time = self._current_time + internal_waiting_duration - self._pending_sleep_events.append((target_event_monotonic_time, event)) + self._pending_sleep_events[event] = target_event_monotonic_time # Ok, let's wait for this Event # (which can only be "unlocked" by calls to `advance_clock()`) await event.wait() @@ -322,16 +324,14 @@ class ClockMock(_Clock): def _check_sleep_timers_to_activate(self) -> int: activated_timers_count = 0 - for i, (target_event_monotonic_time, event) in enumerate( - self._pending_sleep_events - ): + for event, target_event_monotonic_time in self._pending_sleep_events: if self._current_time < target_event_monotonic_time: continue # not time for you yet, dear awaiter... # Right, let's release this waiting event! event.set() activated_timers_count += 1 # ...and remove it from our pending sleep events list: - del self._pending_sleep_events[i] + del self._pending_sleep_events[event] return activated_timers_count From 631f7a5af8a2af17954167449b0c75b0778c163a Mon Sep 17 00:00:00 2001 From: Olivier Philippon Date: Fri, 20 May 2022 15:48:36 +0100 Subject: [PATCH 07/10] =?UTF-8?q?[clock]=20Misc=20bug=20fixes=20?= =?UTF-8?q?=F0=9F=90=9B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/utilities/test_app.py | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/tests/utilities/test_app.py b/tests/utilities/test_app.py index 25948b4aa..ec4c0bdfc 100644 --- a/tests/utilities/test_app.py +++ b/tests/utilities/test_app.py @@ -190,8 +190,12 @@ class AppTest(App): total_capture = self.total_capture if not total_capture: return None - last_display_start_index = total_capture.rindex(CLEAR_SCREEN_SEQUENCE) - return total_capture[last_display_start_index:] + screen_captures = total_capture.split(CLEAR_SCREEN_SEQUENCE) + for single_screen_capture in reversed(screen_captures): + if len(single_screen_capture) > 30: + # let's return the last occurrence of a screen that seem to be properly "fully-paint" + return single_screen_capture + return None @property def console(self) -> ConsoleTest: @@ -278,7 +282,7 @@ class ClockMock(_Clock): self._current_time: int = -1 # For each call to our `sleep` method we will store an asyncio.Event # and the time at which we should trigger it: - self._pending_sleep_events: dict[asyncio.Event, int] = {} + self._pending_sleep_events: dict[int, list[asyncio.Event]] = {} def get_time_no_wait(self) -> float: if self._current_time == -1: @@ -290,7 +294,9 @@ class ClockMock(_Clock): event = asyncio.Event() internal_waiting_duration = int(seconds * self.TIME_RESOLUTION) target_event_monotonic_time = self._current_time + internal_waiting_duration - self._pending_sleep_events[event] = target_event_monotonic_time + self._pending_sleep_events.setdefault(target_event_monotonic_time, []).append( + event + ) # Ok, let's wait for this Event # (which can only be "unlocked" by calls to `advance_clock()`) await event.wait() @@ -324,14 +330,20 @@ class ClockMock(_Clock): def _check_sleep_timers_to_activate(self) -> int: activated_timers_count = 0 - for event, target_event_monotonic_time in self._pending_sleep_events: - if self._current_time < target_event_monotonic_time: + activated_events_times_to_clear: list[int] = [] + for (monotonic_time, target_events) in self._pending_sleep_events.items(): + if self._current_time < monotonic_time: continue # not time for you yet, dear awaiter... - # Right, let's release this waiting event! - event.set() - activated_timers_count += 1 - # ...and remove it from our pending sleep events list: - del self._pending_sleep_events[event] + # Right, let's release these waiting events! + for event in target_events: + event.set() + activated_timers_count += len(target_events) + # ...and let's mark it for removal: + activated_events_times_to_clear.append(monotonic_time) + + if activated_events_times_to_clear: + for event_time_to_clear in activated_events_times_to_clear: + del self._pending_sleep_events[event_time_to_clear] return activated_timers_count From af6a9a850d7bdba8b46cfa46a25e1f55425689c5 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sun, 22 May 2022 08:59:36 +0100 Subject: [PATCH 08/10] compensated in box mode for scrollbars --- sandbox/basic.css | 6 ++--- sandbox/basic.py | 4 +--- src/textual/box_model.py | 7 +++--- src/textual/widget.py | 51 ++++++++++++++++++++++++++++++---------- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/sandbox/basic.css b/sandbox/basic.css index 3f78e4bca..cd32cce1b 100644 --- a/sandbox/basic.css +++ b/sandbox/basic.css @@ -84,7 +84,7 @@ Tweet { /* border: outer $primary; */ padding: 1; border: wide $panel-darken-2; - overflow-y: auto; + overflow: auto; /* scrollbar-gutter: stable; */ align-horizontal: center; box-sizing: border-box; @@ -114,10 +114,10 @@ TweetHeader { } TweetBody { - width: 100w; + width: 130%; background: $panel; color: $text-panel; - height: auto; + height: auto; padding: 0 1 0 0; } diff --git a/sandbox/basic.py b/sandbox/basic.py index 63aed9561..612d82de8 100644 --- a/sandbox/basic.py +++ b/sandbox/basic.py @@ -159,9 +159,7 @@ class BasicApp(App): app = BasicApp( - css_path="basic.css", - watch_css=True, - log_path="textual.log", + css_path="basic.css", watch_css=True, log_path="textual.log", log_verbosity=1 ) if __name__ == "__main__": diff --git a/src/textual/box_model.py b/src/textual/box_model.py index f239123fa..bdd795a5a 100644 --- a/src/textual/box_model.py +++ b/src/textual/box_model.py @@ -17,6 +17,7 @@ def get_box_model( styles: StylesBase, container: Size, viewport: Size, + scrollbars: tuple[int, int], get_content_width: Callable[[Size, Size], int], get_content_height: Callable[[Size, Size, int], int], ) -> BoxModel: @@ -96,8 +97,8 @@ def get_box_model( content_height = max(1, content_height) # Get box dimensions by adding gutter - width = content_width + gutter.width - height = content_height + gutter.height - model = BoxModel(Size(width, height), margin) + size = Size(content_width, content_height) + gutter.totals + + model = BoxModel(size, margin) return model diff --git a/src/textual/widget.py b/src/textual/widget.py index 9afb55831..5684fb048 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -168,6 +168,7 @@ class Widget(DOMNode): self.styles, container, viewport, + self.scrollbar_dimensions, self.get_content_width, self.get_content_height, ) @@ -184,7 +185,10 @@ class Widget(DOMNode): int: The optimal width of the content. """ if self.is_container: - return self.layout.get_content_width(self, container, viewport) + return ( + self.layout.get_content_width(self, container, viewport) + + self.scrollbar_width + ) cache_key = container.width if self._content_width_cache[0] == cache_key: @@ -214,11 +218,14 @@ class Widget(DOMNode): """ if self.is_container: assert self.layout is not None - height = self.layout.get_content_height( - self, - container, - viewport, - width, + height = ( + self.layout.get_content_height( + self, + container, + viewport, + width, + ) + + self.scrollbar_height ) else: cache_key = width @@ -255,11 +262,19 @@ class Widget(DOMNode): @property def max_scroll_x(self) -> float: - return max(0, self.virtual_size.width - self.container_size.width) + return max( + 0, + self.virtual_size.width - self.container_size.width + self.scrollbar_width, + ) @property def max_scroll_y(self) -> float: - return max(0, self.virtual_size.height - self.container_size.height) + return max( + 0, + self.virtual_size.height + - self.container_size.height + + self.scrollbar_height, + ) @property def vertical_scrollbar(self) -> ScrollBar: @@ -335,12 +350,27 @@ class Widget(DOMNode): tuple[bool, bool]: A tuple of (, ) """ - if self.layout is None: + if not self.is_container: return False, False enabled = self.show_vertical_scrollbar, self.show_horizontal_scrollbar return enabled + @property + def scrollbar_dimensions(self) -> tuple[int, int]: + """Get the size of any scrollbars on the widget""" + return (int(self.show_horizontal_scrollbar), int(self.show_vertical_scrollbar)) + + @property + def scrollbar_width(self) -> int: + """Get the width used by the *vertical* scrollbar.""" + return int(self.show_vertical_scrollbar) + + @property + def scrollbar_height(self) -> int: + """Get the height used by the *horizontal* scrollbar.""" + return int(self.show_horizontal_scrollbar) + def set_dirty(self) -> None: """Set the Widget as 'dirty' (requiring re-render).""" self._dirty_regions.clear() @@ -366,7 +396,6 @@ class Widget(DOMNode): bool: True if the scroll position changed, otherwise False. """ scrolled_x = scrolled_y = False - if animate: # TODO: configure animation speed if x is not None: @@ -915,8 +944,6 @@ class Widget(DOMNode): def on_descendant_focus(self, event: events.DescendantFocus) -> None: self.descendant_has_focus = True - if self.is_container and isinstance(event.sender, Widget): - self.scroll_to_widget(event.sender, animate=True) def on_descendant_blur(self, event: events.DescendantBlur) -> None: self.descendant_has_focus = False From 0fc4607176cc093d857662122670761704d4d4e9 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sun, 22 May 2022 09:05:29 +0100 Subject: [PATCH 09/10] tests and comments --- src/textual/widget.py | 2 ++ tests/test_box_model.py | 30 +++++++++++++++--------------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/textual/widget.py b/src/textual/widget.py index 5684fb048..2a9bd8d84 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -262,6 +262,7 @@ class Widget(DOMNode): @property def max_scroll_x(self) -> float: + """The maximum value of `scroll_x`.""" return max( 0, self.virtual_size.width - self.container_size.width + self.scrollbar_width, @@ -269,6 +270,7 @@ class Widget(DOMNode): @property def max_scroll_y(self) -> float: + """The maximum value of `scroll_y`.""" return max( 0, self.virtual_size.height diff --git a/tests/test_box_model.py b/tests/test_box_model.py index 7b85822ea..6b0e5a593 100644 --- a/tests/test_box_model.py +++ b/tests/test_box_model.py @@ -23,7 +23,7 @@ def test_content_box(): assert False, "must not be called" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) # Size should be inclusive of padding / border assert box_model == BoxModel(Size(10, 8), Spacing(0, 0, 0, 0)) @@ -32,7 +32,7 @@ def test_content_box(): styles.box_sizing = "content-box" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) # width and height have added padding / border to accommodate content assert box_model == BoxModel(Size(14, 12), Spacing(0, 0, 0, 0)) @@ -49,7 +49,7 @@ def test_width(): return 10 box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(60, 20), Spacing(0, 0, 0, 0)) @@ -57,7 +57,7 @@ def test_width(): styles.margin = (1, 2, 3, 4) box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(54, 16), Spacing(1, 2, 3, 4)) @@ -65,7 +65,7 @@ def test_width(): styles.width = "auto" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) # Setting width to auto should call get_auto_width assert box_model == BoxModel(Size(10, 16), Spacing(1, 2, 3, 4)) @@ -74,7 +74,7 @@ def test_width(): styles.width = "100vw" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(80, 16), Spacing(1, 2, 3, 4)) @@ -82,7 +82,7 @@ def test_width(): styles.width = "100%" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(60, 16), Spacing(1, 2, 3, 4)) @@ -90,7 +90,7 @@ def test_width(): styles.max_width = "50%" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(30, 16), Spacing(1, 2, 3, 4)) @@ -106,7 +106,7 @@ def test_height(): return 10 box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(60, 20), Spacing(0, 0, 0, 0)) @@ -114,7 +114,7 @@ def test_height(): styles.margin = (1, 2, 3, 4) box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(54, 16), Spacing(1, 2, 3, 4)) @@ -122,7 +122,7 @@ def test_height(): styles.height = "100vh" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(54, 24), Spacing(1, 2, 3, 4)) @@ -130,7 +130,7 @@ def test_height(): styles.height = "100%" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(54, 20), Spacing(1, 2, 3, 4)) @@ -138,7 +138,7 @@ def test_height(): styles.max_height = "50%" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(54, 10), Spacing(1, 2, 3, 4)) @@ -158,7 +158,7 @@ def test_max(): assert False, "must not be called" box_model = get_box_model( - styles, Size(40, 30), Size(80, 24), get_auto_width, get_auto_height + styles, Size(40, 30), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(40, 30), Spacing(0, 0, 0, 0)) @@ -178,6 +178,6 @@ def test_min(): assert False, "must not be called" box_model = get_box_model( - styles, Size(40, 30), Size(80, 24), get_auto_width, get_auto_height + styles, Size(40, 30), Size(80, 24), (0, 0), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(40, 30), Spacing(0, 0, 0, 0)) From 018ab4d92df6fbf304775fba43735535b6f7034f Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sun, 22 May 2022 09:13:32 +0100 Subject: [PATCH 10/10] simplify --- src/textual/box_model.py | 1 - src/textual/widget.py | 1 - tests/test_box_model.py | 30 +++++++++++++++--------------- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/textual/box_model.py b/src/textual/box_model.py index bdd795a5a..c501ff680 100644 --- a/src/textual/box_model.py +++ b/src/textual/box_model.py @@ -17,7 +17,6 @@ def get_box_model( styles: StylesBase, container: Size, viewport: Size, - scrollbars: tuple[int, int], get_content_width: Callable[[Size, Size], int], get_content_height: Callable[[Size, Size, int], int], ) -> BoxModel: diff --git a/src/textual/widget.py b/src/textual/widget.py index 2a9bd8d84..11d17666c 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -168,7 +168,6 @@ class Widget(DOMNode): self.styles, container, viewport, - self.scrollbar_dimensions, self.get_content_width, self.get_content_height, ) diff --git a/tests/test_box_model.py b/tests/test_box_model.py index 6b0e5a593..7b85822ea 100644 --- a/tests/test_box_model.py +++ b/tests/test_box_model.py @@ -23,7 +23,7 @@ def test_content_box(): assert False, "must not be called" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height ) # Size should be inclusive of padding / border assert box_model == BoxModel(Size(10, 8), Spacing(0, 0, 0, 0)) @@ -32,7 +32,7 @@ def test_content_box(): styles.box_sizing = "content-box" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height ) # width and height have added padding / border to accommodate content assert box_model == BoxModel(Size(14, 12), Spacing(0, 0, 0, 0)) @@ -49,7 +49,7 @@ def test_width(): return 10 box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(60, 20), Spacing(0, 0, 0, 0)) @@ -57,7 +57,7 @@ def test_width(): styles.margin = (1, 2, 3, 4) box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(54, 16), Spacing(1, 2, 3, 4)) @@ -65,7 +65,7 @@ def test_width(): styles.width = "auto" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height ) # Setting width to auto should call get_auto_width assert box_model == BoxModel(Size(10, 16), Spacing(1, 2, 3, 4)) @@ -74,7 +74,7 @@ def test_width(): styles.width = "100vw" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(80, 16), Spacing(1, 2, 3, 4)) @@ -82,7 +82,7 @@ def test_width(): styles.width = "100%" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(60, 16), Spacing(1, 2, 3, 4)) @@ -90,7 +90,7 @@ def test_width(): styles.max_width = "50%" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(30, 16), Spacing(1, 2, 3, 4)) @@ -106,7 +106,7 @@ def test_height(): return 10 box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(60, 20), Spacing(0, 0, 0, 0)) @@ -114,7 +114,7 @@ def test_height(): styles.margin = (1, 2, 3, 4) box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(54, 16), Spacing(1, 2, 3, 4)) @@ -122,7 +122,7 @@ def test_height(): styles.height = "100vh" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(54, 24), Spacing(1, 2, 3, 4)) @@ -130,7 +130,7 @@ def test_height(): styles.height = "100%" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(54, 20), Spacing(1, 2, 3, 4)) @@ -138,7 +138,7 @@ def test_height(): styles.max_height = "50%" box_model = get_box_model( - styles, Size(60, 20), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(60, 20), Size(80, 24), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(54, 10), Spacing(1, 2, 3, 4)) @@ -158,7 +158,7 @@ def test_max(): assert False, "must not be called" box_model = get_box_model( - styles, Size(40, 30), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(40, 30), Size(80, 24), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(40, 30), Spacing(0, 0, 0, 0)) @@ -178,6 +178,6 @@ def test_min(): assert False, "must not be called" box_model = get_box_model( - styles, Size(40, 30), Size(80, 24), (0, 0), get_auto_width, get_auto_height + styles, Size(40, 30), Size(80, 24), get_auto_width, get_auto_height ) assert box_model == BoxModel(Size(40, 30), Spacing(0, 0, 0, 0))