Skip to content

Commit

Permalink
Fix CSS watcher crashing when file becomes unavailable... (#4079)
Browse files Browse the repository at this point in the history
* Managing exceptions when watched CSS files are unavailable

* Handling scenario where FileMonitor crashes when file temporarily becomes unavailable.

* Update CHANGELOG

* Update log level to warning
  • Loading branch information
darrenburns authored Jan 31, 2024
1 parent f017604 commit 60e0d8d
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Ensuring `TextArea.SelectionChanged` message only sends when the updated selection is different https://github.com/Textualize/textual/pull/3933
- Fixed declaration after nested rule set causing a parse error https://github.com/Textualize/textual/pull/4012
- ID and class validation was too lenient https://github.com/Textualize/textual/issues/3954
- Fixed CSS watcher crash if file becomes unreadable (even temporarily) https://github.com/Textualize/textual/pull/4079
- Fixed display of keys when used in conjunction with other keys https://github.com/Textualize/textual/pull/3050

## [0.47.1] - 2023-01-05
Expand Down
11 changes: 10 additions & 1 deletion src/textual/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
from .await_remove import AwaitRemove
from .binding import Binding, BindingType, _Bindings
from .command import CommandPalette, Provider
from .css.errors import StylesheetError
from .css.query import NoMatches
from .css.stylesheet import RulesMap, Stylesheet
from .design import ColorSystem
Expand Down Expand Up @@ -1469,7 +1470,15 @@ async def _on_css_change(self) -> None:
try:
time = perf_counter()
stylesheet = self.stylesheet.copy()
stylesheet.read_all(css_paths)
try:
stylesheet.read_all(css_paths)
except StylesheetError as error:
# If one of the CSS paths is no longer available (or perhaps temporarily unavailable),
# we'll end up with partial CSS, which is probably confusing more than anything. We opt to do
# nothing here, knowing that we'll retry again very soon, on the next file monitor invocation.
# Related issue: https://github.com/Textualize/textual/issues/3996
self.log.warning(str(error))
return
stylesheet.parse()
elapsed = (perf_counter() - time) * 1000
if self._css_has_errors:
Expand Down
9 changes: 8 additions & 1 deletion src/textual/file_monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@ def __rich_repr__(self) -> rich.repr.Result:

def _get_last_modified_time(self) -> float:
"""Get the most recent modified time out of all files being watched."""
return max((os.stat(path).st_mtime for path in self._paths), default=0)
modified_times = []
for path in self._paths:
try:
modified_time = os.stat(path).st_mtime
except FileNotFoundError:
modified_time = 0
modified_times.append(modified_time)
return max(modified_times, default=0)

def check(self) -> bool:
"""Check the monitored files. Return True if any were changed since the last modification time."""
Expand Down
29 changes: 24 additions & 5 deletions tests/css/test_css_reloading.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
"""
Regression test for https://github.com/Textualize/textual/issues/3931
"""

import os
from pathlib import Path

from textual.app import App, ComposeResult
Expand Down Expand Up @@ -34,7 +31,7 @@ def on_mount(self) -> None:


async def test_css_reloading_applies_to_non_top_screen(monkeypatch) -> None: # type: ignore
"""Regression test for https://github.com/Textualize/textual/issues/2063."""
"""Regression test for https://github.com/Textualize/textual/issues/3931"""

monkeypatch.setenv(
"TEXTUAL", "debug"
Expand Down Expand Up @@ -65,3 +62,25 @@ async def test_css_reloading_applies_to_non_top_screen(monkeypatch) -> None: #
# Height should fall back to 1.
assert first_label.styles.height is not None
assert first_label.styles.height.value == 1


async def test_css_reloading_file_not_found(monkeypatch, tmp_path):
"""Regression test for https://github.com/Textualize/textual/issues/3996
Files can become temporarily unavailable during saving on certain environments.
"""
monkeypatch.setenv("TEXTUAL", "debug")

css_path = tmp_path / "test_css_reloading_file_not_found.tcss"
with open(css_path, "w") as css_file:
css_file.write("#a {color: red;}")

class TextualApp(App):
CSS_PATH = css_path

app = TextualApp()
async with app.run_test() as pilot:
await pilot.app._on_css_change()
os.remove(css_path)
assert not css_path.exists()
await pilot.app._on_css_change()
50 changes: 50 additions & 0 deletions tests/test_file_monitor.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
from pathlib import Path

from textual.file_monitor import FileMonitor
Expand All @@ -6,3 +7,52 @@
def test_repr() -> None:
file_monitor = FileMonitor([Path(".")], lambda: None)
assert "FileMonitor" in repr(file_monitor)


def test_file_never_found():
path = "doesnt_exist.tcss"
file_monitor = FileMonitor([Path(path)], lambda: None)
file_monitor.check() # Ensuring no exceptions are raised.


async def test_file_deletion(tmp_path):
"""In some environments, a file can become temporarily unavailable during saving.
This test ensures the FileMonitor is robust enough to handle a file temporarily being
unavailable, and that it recovers when the file becomes available again.
Regression test for: https://github.com/Textualize/textual/issues/3996
"""

def make_file():
path = tmp_path / "will_be_deleted.tcss"
path.write_text("#foo { color: dodgerblue; }")
return path

callback_counter = 0

def callback():
nonlocal callback_counter
callback_counter += 1

path = make_file()
file_monitor = FileMonitor([path], callback)

# Ensure the file monitor survives after the file is gone
await file_monitor()
os.remove(path)

# The file is now unavailable, but we don't crash here
await file_monitor()
await file_monitor()

# Despite multiple checks, the file was only available for the first check,
# and we only fire the callback while the file is available.
assert callback_counter == 1

# The file is made available again.
make_file()

# Since the file is available, the callback should fire when the FileMonitor is called
await file_monitor()
assert callback_counter == 2

0 comments on commit 60e0d8d

Please sign in to comment.