[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

@@ -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

changelog.rst

@@ -10,6 +10,7 @@ Changelog
 
 - Eliminate timeout in waiting on event queue. (`#861 <https://github.com/gorakhargosh/watchdog/pull/861>`_)
 - [inotify] Fix ``not`` equality implementation for ``InotifyEvent``. (`#848 <https://github.com/gorakhargosh/watchdog/pull/848>`_)
+- [watchmedo] Fix calling commands from within a Python script. (`#879 <https://github.com/gorakhargosh/watchdog/pull/879>`_)
 - [watchmedo] ``PyYAML`` is loaded only when strictly necessary. Simple usages of ``watchmedo`` are possible without the module being installed. (`#847 <https://github.com/gorakhargosh/watchdog/pull/847>`_)
 - Thanks to our beloved contributors: @sattlerc, @JanzenLiu, @BoboTiG
 

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
 
 

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))

tests/test_watchmedo.py → 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

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():

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,21 +29,17 @@ def observer():
     obs = BaseObserver(EventEmitter)
     yield obs
     obs.stop()
-    try:
+    with contextlib.suppress(RuntimeError):
         obs.join()
-    except RuntimeError:
-        pass
 
 
 @pytest.fixture
 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
 

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):

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]