From 4329743afb637c4a32d19c0289a9de9092889002 Mon Sep 17 00:00:00 2001
From: Carl Meyer <carl@oddbird.net>
Date: Mon, 17 Oct 2022 14:15:57 -0600
Subject: [PATCH] Review comments

---
 Lib/test/test_capi.py | 26 +++++++++++++++++++++++---
 Objects/typeobject.c  |  4 ++--
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
index 64b92b0c52fa0c..364c607b3c18b3 100644
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -2,7 +2,7 @@
 # these are all functions _testcapi exports whose name begins with 'test_'.
 
 from collections import OrderedDict
-from contextlib import contextmanager
+from contextlib import contextmanager, ExitStack
 import _thread
 import importlib.machinery
 import importlib.util
@@ -1612,6 +1612,9 @@ class TestTypeWatchers(unittest.TestCase):
     ERROR = 1    # unconditionally sets and signals a RuntimeException
     WRAP = 2     # appends modified type wrapped in list to global event list
 
+    # duplicating the C constant
+    TYPE_MAX_WATCHERS = 8
+
     def add_watcher(self, kind=TYPES):
         return _testcapi.add_type_watcher(kind)
 
@@ -1727,7 +1730,7 @@ class C: pass
         with self.assertRaisesRegex(ValueError, r"Invalid type watcher ID -1"):
             self.watch(-1, C)
         with self.assertRaisesRegex(ValueError, r"Invalid type watcher ID 8"):
-            self.watch(8, C)  # TYPE_MAX_WATCHERS = 8
+            self.watch(self.TYPE_MAX_WATCHERS, C)
 
     def test_watch_unassigned_watcher_id(self):
         class C: pass
@@ -1744,13 +1747,30 @@ class C: pass
         with self.assertRaisesRegex(ValueError, r"Invalid type watcher ID -1"):
             self.unwatch(-1, C)
         with self.assertRaisesRegex(ValueError, r"Invalid type watcher ID 8"):
-            self.unwatch(8, C)  # TYPE_MAX_WATCHERS = 8
+            self.unwatch(self.TYPE_MAX_WATCHERS, C)
 
     def test_unwatch_unassigned_watcher_id(self):
         class C: pass
         with self.assertRaisesRegex(ValueError, r"No type watcher set for ID 1"):
             self.unwatch(1, C)
 
+    def test_clear_out_of_range_watcher_id(self):
+        with self.assertRaisesRegex(ValueError, r"Invalid type watcher ID -1"):
+            self.clear_watcher(-1)
+        with self.assertRaisesRegex(ValueError, r"Invalid type watcher ID 8"):
+            self.clear_watcher(self.TYPE_MAX_WATCHERS)
+
+    def test_clear_unassigned_watcher_id(self):
+        with self.assertRaisesRegex(ValueError, r"No type watcher set for ID 1"):
+            self.clear_watcher(1)
+
+    def test_no_more_ids_available(self):
+        contexts = [self.watcher() for i in range(self.TYPE_MAX_WATCHERS)]
+        with ExitStack() as stack:
+            for ctx in contexts:
+                stack.enter_context(ctx)
+            with self.assertRaisesRegex(RuntimeError, r"no more type watcher IDs"):
+                self.add_watcher()
 
 
 if __name__ == "__main__":
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 1b0c70dea79970..b105d405f25417 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -406,7 +406,7 @@ int
 PyType_ClearWatcher(int watcher_id)
 {
     PyInterpreterState *interp = _PyInterpreterState_GET();
-    if (validate_watcher_id(interp, watcher_id)) {
+    if (validate_watcher_id(interp, watcher_id) < 0) {
         return -1;
     }
     interp->type_watchers[watcher_id] = NULL;
@@ -424,7 +424,7 @@ PyType_Watch(int watcher_id, PyObject* obj)
     }
     PyTypeObject *type = (PyTypeObject *)obj;
     PyInterpreterState *interp = _PyInterpreterState_GET();
-    if (validate_watcher_id(interp, watcher_id)) {
+    if (validate_watcher_id(interp, watcher_id) < 0) {
         return -1;
     }
     // ensure we will get a callback on the next modification