Skip to content

Commit 5aac560

Browse files
committed
Fix an intermittent test failure
It seemed to be caused by issues around our observer API, so we make the following changes: * Disable filesystem watching in tests * Open the project in a context manager as intended, although this has no real effect due to the previous change * Add a big lock to the FileWatcher, just to be safe
1 parent 3812ada commit 5aac560

File tree

3 files changed

+56
-28
lines changed

3 files changed

+56
-28
lines changed

snooty/parser.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1596,17 +1596,22 @@ class Project:
15961596
15971597
This class's public methods are thread-safe."""
15981598

1599-
__slots__ = ("_project", "_lock", "_filesystem_watcher")
1600-
16011599
def __init__(
1602-
self, root: Path, backend: ProjectBackend, build_identifiers: BuildIdentifierSet
1600+
self,
1601+
root: Path,
1602+
backend: ProjectBackend,
1603+
build_identifiers: BuildIdentifierSet,
1604+
watch: bool = True,
16031605
) -> None:
16041606
self._filesystem_watcher = util.FileWatcher(self._on_asset_event)
16051607
self._project = _Project(
16061608
root, backend, self._filesystem_watcher, build_identifiers
16071609
)
16081610
self._lock = threading.Lock()
1609-
self._filesystem_watcher.start()
1611+
1612+
self.watch = watch
1613+
if watch:
1614+
self._filesystem_watcher.start()
16101615

16111616
@property
16121617
def config(self) -> ProjectConfig:
@@ -1644,7 +1649,8 @@ def build(
16441649

16451650
def stop_monitoring(self) -> None:
16461651
"""Stop the filesystem monitoring thread associated with this project."""
1647-
self._filesystem_watcher.stop(join=True)
1652+
if self.watch:
1653+
self._filesystem_watcher.stop(join=True)
16481654

16491655
def _on_asset_event(self, ev: watchdog.events.FileSystemEvent) -> None:
16501656
with self._lock:

snooty/util.py

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import re
66
import sys
77
import tempfile
8+
import threading
89
import time
910
import urllib.parse
1011
from collections import defaultdict
@@ -180,7 +181,7 @@ def dispatch(self, event: watchdog.events.FileSystemEvent) -> None:
180181
path = Path(event.src_path)
181182
if (
182183
path.parent in self.directories
183-
and path.name in self.directories[path.parent].filenames
184+
and path.name in self.directories[path.parent]
184185
):
185186
self.on_event(event)
186187

@@ -189,15 +190,31 @@ class AssetWatch:
189190
"""Track files in a directory to watch. This reflects the underlying interface
190191
exposed by watchdog."""
191192

192-
filenames: Counter[str]
193+
_filenames: Counter[str]
193194
watch_handle: watchdog.observers.api.ObservedWatch
194195

195196
def __len__(self) -> int:
196-
return len(self.filenames)
197+
return len(self._filenames)
198+
199+
def increment(self, filename: str) -> None:
200+
self._filenames[filename] += 1
201+
202+
def decrement(self, filename: str) -> None:
203+
self._filenames[filename] -= 1
204+
205+
def __getitem__(self, filename: str) -> int:
206+
return self._filenames[filename]
207+
208+
def __delitem__(self, filename: str) -> None:
209+
del self._filenames[filename]
210+
211+
def __contains__(self, filename: str) -> bool:
212+
return filename in self._filenames
197213

198214
def __init__(
199215
self, on_event: Callable[[watchdog.events.FileSystemEvent], None]
200216
) -> None:
217+
self.lock = threading.Lock()
201218
self.observer = watchdog.observers.Observer()
202219
self.directories: Dict[Path, FileWatcher.AssetWatch] = {}
203220
self.handler = self.AssetChangedHandler(self.directories, on_event)
@@ -206,29 +223,33 @@ def watch_file(self, path: Path) -> None:
206223
"""Start reporting upon changes to a file."""
207224
directory = path.parent
208225
logger.debug("Starting watch: %s", path)
209-
if directory in self.directories:
210-
self.directories[directory].filenames[path.name] += 1
211-
return
212-
213-
watch = self.observer.schedule(self.handler, str(directory))
214-
self.directories[directory] = self.AssetWatch(Counter({path.name: 1}), watch)
226+
with self.lock:
227+
if directory in self.directories:
228+
self.directories[directory].increment(path.name)
229+
return
230+
231+
watch = self.observer.schedule(self.handler, str(directory))
232+
self.directories[directory] = self.AssetWatch(
233+
Counter({path.name: 1}), watch
234+
)
215235

216236
def end_watch(self, path: Path) -> None:
217237
"""Stop watching a file."""
218238
directory = path.parent
219-
if directory not in self.directories:
220-
return
239+
with self.lock:
240+
if directory not in self.directories:
241+
return
221242

222-
watch = self.directories[directory]
223-
watch.filenames[path.name] -= 1
224-
if watch.filenames[path.name] <= 0:
225-
del watch.filenames[path.name]
243+
watch = self.directories[directory]
244+
watch.decrement(path.name)
245+
if watch[path.name] <= 0:
246+
del watch[path.name]
226247

227-
# If there are no files remaining in this watch directory, unwatch it.
228-
if not watch.filenames:
229-
self.observer.unschedule(watch.watch_handle)
230-
logger.info("Stopping watch: %s", path)
231-
del self.directories[directory]
248+
# If there are no files remaining in this watch directory, unwatch it.
249+
if len(watch) == 0:
250+
self.observer.unschedule(watch.watch_handle)
251+
logger.info("Stopping watch: %s", path)
252+
del self.directories[directory]
232253

233254
def start(self) -> None:
234255
"""Start a thread watching for file changes."""
@@ -248,7 +269,8 @@ def __exit__(self, *args: object) -> None:
248269
self.stop()
249270

250271
def __len__(self) -> int:
251-
return sum(len(w) for w in self.directories.values())
272+
with self.lock:
273+
return sum(len(w) for w in self.directories.values())
252274

253275

254276
def option_string(argument: Optional[str]) -> Optional[str]:

snooty/util_test.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,8 @@ def make_test(
225225

226226
backend = BackendTestResults()
227227

228-
project = Project(root, backend, {})
229-
project.build()
228+
with Project(root, backend, {}, watch=False) as project:
229+
project.build()
230230

231231
yield backend
232232

0 commit comments

Comments
 (0)