From be50c422288e721dc864f453ca93ef21113c6977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Schoentgen?= Date: Fri, 25 Mar 2022 12:27:23 +0100 Subject: [PATCH] [watchmedo] Fix calling commands from within a Python script (#880) Fixes #879 * Fix handling tests Mokcing `time.sleep()` does not work if `eventlet.monkey_patch()` has been called before. So I renamed watchmedo the test file to be run before the one using `eventlet`. Also done some cleaning in tests. --- .github/workflows/tests.yml | 2 +- changelog.rst | 1 + src/watchdog/watchmedo.py | 1 + tests/conftest.py | 2 +- ...{test_watchmedo.py => test_0_watchmedo.py} | 27 ++++++++- tests/test_inotify_c.py | 58 +++++++------------ tests/test_observer.py | 19 +++--- tests/test_snapshot_diff.py | 56 +++++++++--------- tox.ini | 2 +- 9 files changed, 86 insertions(+), 82 deletions(-) rename tests/{test_watchmedo.py => test_0_watchmedo.py} (78%) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9f1bf81d..8a4a7513 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -20,7 +20,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, windows-latest, macos-latest] - python: [3.6, 3.7, 3.8, 3.9, 'pypy-3.7'] + python: [3.6, 3.7, 3.8, 3.9, "3.10", "pypy-3.7"] steps: - name: Checkout uses: actions/checkout@v2 diff --git a/changelog.rst b/changelog.rst index cfea69a3..0b12e353 100644 --- a/changelog.rst +++ b/changelog.rst @@ -10,6 +10,7 @@ Changelog - Eliminate timeout in waiting on event queue. (`#861 `_) - [inotify] Fix ``not`` equality implementation for ``InotifyEvent``. (`#848 `_) +- [watchmedo] Fix calling commands from within a Python script. (`#879 `_) - [watchmedo] ``PyYAML`` is loaded only when strictly necessary. Simple usages of ``watchmedo`` are possible without the module being installed. (`#847 `_) - Thanks to our beloved contributors: @sattlerc, @JanzenLiu, @BoboTiG diff --git a/src/watchdog/watchmedo.py b/src/watchdog/watchmedo.py index 668984dc..0841c9a0 100755 --- a/src/watchdog/watchmedo.py +++ b/src/watchdog/watchmedo.py @@ -97,6 +97,7 @@ def decorator(func): for arg in args: parser.add_argument(*arg[0], **arg[1]) parser.set_defaults(func=func) + return func return decorator diff --git a/tests/conftest.py b/tests/conftest.py index 9dd9fabc..2e84a02f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -48,7 +48,7 @@ def no_warnings(recwarn): "Not importing directory" in message or "Using or importing the ABCs" in message or "dns.hash module will be removed in future versions" in message - or ("eventlet" in filename and "eventlet" in filename) + or "eventlet" in filename ): continue warnings.append("{w.filename}:{w.lineno} {w.message}".format(w=warning)) diff --git a/tests/test_watchmedo.py b/tests/test_0_watchmedo.py similarity index 78% rename from tests/test_watchmedo.py rename to tests/test_0_watchmedo.py index c90dec89..312ba5a1 100644 --- a/tests/test_watchmedo.py +++ b/tests/test_0_watchmedo.py @@ -1,7 +1,8 @@ -# coding: utf-8 - +from unittest.mock import patch import pytest +from watchdog.utils import WatchdogShutdown + # Skip if import PyYAML failed. PyYAML missing possible because # watchdog installed without watchmedo. See Installation section # in README.rst @@ -71,3 +72,25 @@ def test_kill_auto_restart(tmpdir, capfd): assert '+++++ 9' not in cap.out # we killed the subprocess before the end # in windows we seem to lose the subprocess stderr # assert 'KeyboardInterrupt' in cap.err + + +@pytest.mark.parametrize("command", ["tricks-from", "tricks"]) +def test_tricks_from_file(command, tmp_path): + tricks_file = tmp_path / "tricks.yaml" + tricks_file.write_text(""" +tricks: +- watchdog.tricks.LoggerTrick: + patterns: ["*.py", "*.js"] +""") + args = watchmedo.cli.parse_args([command, str(tricks_file)]) + + checkpoint = False + + def mocked_sleep(_): + nonlocal checkpoint + checkpoint = True + raise WatchdogShutdown() + + with patch("time.sleep", mocked_sleep): + watchmedo.tricks_from(args) + assert checkpoint diff --git a/tests/test_inotify_c.py b/tests/test_inotify_c.py index 50090ead..b4f49699 100644 --- a/tests/test_inotify_c.py +++ b/tests/test_inotify_c.py @@ -1,4 +1,3 @@ - import pytest from watchdog.utils import platform @@ -13,6 +12,7 @@ import struct from functools import partial from queue import Queue +from unittest.mock import patch from watchdog.events import DirCreatedEvent, DirDeletedEvent, DirModifiedEvent from watchdog.observers.api import ObservedWatch @@ -47,10 +47,8 @@ def watching(path=None, use_full_emitter=False): def teardown_function(function): rm(p(''), recursive=True) - try: + with contextlib.suppress(NameError): assert not emitter.is_alive() - except NameError: - pass def struct_inotify(wd, mask, cookie=0, length=0, name=b""): @@ -66,7 +64,7 @@ def struct_inotify(wd, mask, cookie=0, length=0, name=b""): return struct.pack(struct_format, wd, mask, cookie, length, name) -def test_late_double_deletion(monkeypatch): +def test_late_double_deletion(): inotify_fd = type(str("FD"), (object,), {})() # Empty object inotify_fd.last = 0 inotify_fd.wds = [] @@ -116,13 +114,13 @@ def inotify_rm_watch(fd, wd): # Mocks the API! from watchdog.observers import inotify_c - monkeypatch.setattr(os, "read", fakeread) - monkeypatch.setattr(os, "close", fakeclose) - monkeypatch.setattr(inotify_c, "inotify_init", inotify_init) - monkeypatch.setattr(inotify_c, "inotify_add_watch", inotify_add_watch) - monkeypatch.setattr(inotify_c, "inotify_rm_watch", inotify_rm_watch) + mock1 = patch.object(os, "read", new=fakeread) + mock2 = patch.object(os, "close", new=fakeclose) + mock3 = patch.object(inotify_c, "inotify_init", new=inotify_init) + mock4 = patch.object(inotify_c, "inotify_add_watch", new=inotify_add_watch) + mock5 = patch.object(inotify_c, "inotify_rm_watch", new=inotify_rm_watch) - with watching(p('')): + with mock1, mock2, mock3, mock4, mock5, watching(p('')): # Watchdog Events for evt_cls in [DirCreatedEvent, DirDeletedEvent] * 2: event = event_queue.get(timeout=5)[0] @@ -137,32 +135,18 @@ def inotify_rm_watch(fd, wd): assert inotify_fd.wds == [2, 3] # Only 1 is removed explicitly -def test_raise_error(monkeypatch): - func = Inotify._raise_error - - monkeypatch.setattr(ctypes, "get_errno", lambda: errno.ENOSPC) - with pytest.raises(OSError) as exc: - func() - assert exc.value.errno == errno.ENOSPC - assert "inotify watch limit reached" in str(exc.value) - - monkeypatch.setattr(ctypes, "get_errno", lambda: errno.EMFILE) - with pytest.raises(OSError) as exc: - func() - assert exc.value.errno == errno.EMFILE - assert "inotify instance limit reached" in str(exc.value) - - monkeypatch.setattr(ctypes, "get_errno", lambda: errno.ENOENT) - with pytest.raises(OSError) as exc: - func() - assert exc.value.errno == errno.ENOENT - assert "No such file or directory" in str(exc.value) - - monkeypatch.setattr(ctypes, "get_errno", lambda: -1) - with pytest.raises(OSError) as exc: - func() - assert exc.value.errno == -1 - assert "Unknown error -1" in str(exc.value) +@pytest.mark.parametrize("error, pattern", [ + (errno.ENOSPC, "inotify watch limit reached"), + (errno.EMFILE, "inotify instance limit reached"), + (errno.ENOENT, "No such file or directory"), + (-1, "Unknown error -1"), +]) +def test_raise_error(error, pattern): + with patch.object(ctypes, "get_errno", new=lambda: error): + with pytest.raises(OSError) as exc: + Inotify._raise_error() + assert exc.value.errno == error + assert pattern in str(exc.value) def test_non_ascii_path(): diff --git a/tests/test_observer.py b/tests/test_observer.py index cc4f8748..4ca5c77f 100644 --- a/tests/test_observer.py +++ b/tests/test_observer.py @@ -14,7 +14,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +import contextlib import threading +from unittest.mock import patch import pytest @@ -27,10 +29,8 @@ def observer(): obs = BaseObserver(EventEmitter) yield obs obs.stop() - try: + with contextlib.suppress(RuntimeError): obs.join() - except RuntimeError: - pass @pytest.fixture @@ -38,10 +38,8 @@ def observer2(): obs = BaseObserver(EventEmitter) yield obs obs.stop() - try: + with contextlib.suppress(RuntimeError): obs.join() - except RuntimeError: - pass def test_schedule_should_start_emitter_if_running(observer): @@ -118,7 +116,7 @@ def test_2_observers_on_the_same_path(observer, observer2): assert len(observer2.emitters) == 1 -def test_start_failure_should_not_prevent_further_try(monkeypatch, observer): +def test_start_failure_should_not_prevent_further_try(observer): observer.schedule(None, '') emitters = observer.emitters assert len(emitters) == 1 @@ -129,14 +127,13 @@ def mocked_start(): raise OSError() emitter = next(iter(emitters)) - monkeypatch.setattr(emitter, "start", mocked_start) - with pytest.raises(OSError): - observer.start() + with patch.object(emitter, "start", new=mocked_start): + with pytest.raises(OSError): + observer.start() # The emitter should be removed from the list assert len(observer.emitters) == 0 # Restoring the original behavior should work like there never be emitters - monkeypatch.undo() observer.start() assert len(observer.emitters) == 0 diff --git a/tests/test_snapshot_diff.py b/tests/test_snapshot_diff.py index 1667b2a9..850f3aed 100644 --- a/tests/test_snapshot_diff.py +++ b/tests/test_snapshot_diff.py @@ -18,6 +18,7 @@ import os import pickle import time +from unittest.mock import patch from watchdog.utils.dirsnapshot import DirectorySnapshot from watchdog.utils.dirsnapshot import DirectorySnapshotDiff @@ -107,7 +108,7 @@ def test_dir_modify_on_move(p): wait() mv(p('dir1', 'a'), p('dir2', 'b')) diff = DirectorySnapshotDiff(ref, DirectorySnapshot(p(''))) - assert set(diff.dirs_modified) == set([p('dir1'), p('dir2')]) + assert set(diff.dirs_modified) == {p('dir1'), p('dir2')} def test_detect_modify_for_moved_files(p): @@ -138,11 +139,12 @@ def listdir_fcn(path): DirectorySnapshot(p('root'), listdir=listdir_fcn) -def test_permission_error(monkeypatch, p): +def test_permission_error(p): # Test that unreadable folders are not raising exceptions mkdir(p('a', 'b', 'c'), parents=True) ref = DirectorySnapshot(p('')) + walk_orig = DirectorySnapshot.walk def walk(self, root): """Generate a permission error on folder "a/b".""" @@ -151,16 +153,11 @@ def walk(self, root): raise OSError(errno.EACCES, os.strerror(errno.EACCES)) # Mimic the original method - for entry in walk_orig(self, root): - yield entry - - walk_orig = DirectorySnapshot.walk - monkeypatch.setattr(DirectorySnapshot, "walk", walk) - - # Should NOT raise an OSError (EACCES) - new_snapshot = DirectorySnapshot(p('')) + yield from walk_orig(self, root) - monkeypatch.undo() + with patch.object(DirectorySnapshot, "walk", new=walk): + # Should NOT raise an OSError (EACCES) + new_snapshot = DirectorySnapshot(p('')) diff = DirectorySnapshotDiff(ref, new_snapshot) assert repr(diff) @@ -169,38 +166,39 @@ def walk(self, root): assert diff.dirs_deleted == [(p('a', 'b', 'c'))] -def test_ignore_device(monkeypatch, p): +def test_ignore_device(p): # Create a file and take a snapshot. touch(p('file')) ref = DirectorySnapshot(p('')) wait() + inode_orig = DirectorySnapshot.inode + def inode(self, path): # This function will always return a different device_id, # even for the same file. result = inode_orig(self, path) inode.times += 1 return result[0], result[1] + inode.times + inode.times = 0 # Set the custom inode function. - inode_orig = DirectorySnapshot.inode - monkeypatch.setattr(DirectorySnapshot, 'inode', inode) - - # If we make the diff of the same directory, since by default the - # DirectorySnapshotDiff compares the snapshots using the device_id (and it will - # be different), it thinks that the same file has been deleted and created again. - snapshot = DirectorySnapshot(p('')) - diff_with_device = DirectorySnapshotDiff(ref, snapshot) - assert diff_with_device.files_deleted == [(p('file'))] - assert diff_with_device.files_created == [(p('file'))] - - # Otherwise, if we choose to ignore the device, the file will not be detected as - # deleted and re-created. - snapshot = DirectorySnapshot(p('')) - diff_without_device = DirectorySnapshotDiff(ref, snapshot, ignore_device=True) - assert diff_without_device.files_deleted == [] - assert diff_without_device.files_created == [] + with patch.object(DirectorySnapshot, 'inode', new=inode): + # If we make the diff of the same directory, since by default the + # DirectorySnapshotDiff compares the snapshots using the device_id (and it will + # be different), it thinks that the same file has been deleted and created again. + snapshot = DirectorySnapshot(p('')) + diff_with_device = DirectorySnapshotDiff(ref, snapshot) + assert diff_with_device.files_deleted == [(p('file'))] + assert diff_with_device.files_created == [(p('file'))] + + # Otherwise, if we choose to ignore the device, the file will not be detected as + # deleted and re-created. + snapshot = DirectorySnapshot(p('')) + diff_without_device = DirectorySnapshotDiff(ref, snapshot, ignore_device=True) + assert diff_without_device.files_deleted == [] + assert diff_without_device.files_created == [] def test_empty_snapshot(p): diff --git a/tox.ini b/tox.ini index ccf7e601..9b170f4c 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py{310,39,38,37,36,35,py3} +envlist = py{310,39,38,37,36,py3} skip_missing_interpreters = True [testenv]