Skip to content

Commit 7506d67

Browse files
committed
Noah feedback
1 parent fecfbc7 commit 7506d67

2 files changed

Lines changed: 18 additions & 130 deletions

File tree

test/asynchronous/test_periodic_executor.py

Lines changed: 9 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -56,62 +56,19 @@ async def asyncTearDown(self):
5656
await self.executor.join(timeout=2)
5757

5858

59-
class TestAsyncPeriodicExecutorRepr(AsyncUnitTest):
59+
class TestAsyncPeriodicExecutor(AsyncPeriodicExecutorTestBase):
6060
async def test_repr_contains_class_and_name(self):
6161
executor = _make_executor(name="exec")
6262
executor_repr = repr(executor)
6363
self.assertIn("AsyncPeriodicExecutor", executor_repr)
6464
self.assertIn("exec", executor_repr)
6565

66-
67-
class TestAsyncPeriodicExecutorBasic(AsyncPeriodicExecutorTestBase):
68-
async def test_wake_sets_event(self):
69-
self.assertFalse(self.executor._event)
70-
self.executor.wake()
71-
self.assertTrue(self.executor._event)
72-
73-
async def test_update_interval(self):
74-
self.executor.update_interval(60)
75-
self.assertEqual(self.executor._interval, 60)
76-
77-
async def test_skip_sleep(self):
78-
self.assertFalse(self.executor._skip_sleep)
79-
self.executor.skip_sleep()
80-
self.assertTrue(self.executor._skip_sleep)
81-
82-
83-
class TestAsyncPeriodicExecutorLifecycle(AsyncPeriodicExecutorTestBase):
84-
async def test_open_starts_worker(self):
85-
self.executor.open()
86-
if _IS_SYNC:
87-
self.assertIsNotNone(self.executor._thread)
88-
self.assertTrue(self.executor._thread.is_alive())
89-
else:
90-
self.assertIsNotNone(self.executor._task)
91-
92-
async def test_close_sets_stopped(self):
93-
self.executor.open()
94-
self.executor.close()
95-
self.assertTrue(self.executor._stopped)
96-
await self.executor.join(timeout=1)
97-
9866
async def test_join_without_open_is_safe(self):
99-
await self.executor.join(timeout=0.01)
100-
101-
async def test_multiple_open_calls_have_no_effect(self):
102-
self.executor.open()
103-
if _IS_SYNC:
104-
worker_id = id(self.executor._thread)
105-
else:
106-
worker_id = id(self.executor._task)
107-
self.executor.open()
108-
if _IS_SYNC:
109-
self.assertEqual(worker_id, id(self.executor._thread))
110-
else:
111-
self.assertEqual(worker_id, id(self.executor._task))
112-
67+
try:
68+
await self.executor.join(timeout=0.01)
69+
except Exception as e:
70+
self.fail(f"join() raised unexpected Exception {e}")
11371

114-
class TestAsyncPeriodicExecutorTarget(AsyncPeriodicExecutorTestBase):
11572
async def test_target_returning_false_stops_executor(self):
11673
if _IS_SYNC:
11774
ran = threading.Event()
@@ -124,12 +81,8 @@ async def target():
12481

12582
self.executor = _make_executor(target=target)
12683
self.executor.open()
127-
if _IS_SYNC:
128-
self.assertTrue(ran.wait(timeout=2), "target never ran")
129-
else:
130-
await asyncio.wait_for(ran.wait(), timeout=2)
13184
await self.executor.join(timeout=2)
132-
self.assertTrue(self.executor._stopped)
85+
self.assertTrue(ran.is_set(), "target never ran")
13386

13487
async def test_target_exception_stops_executor(self):
13588
if _IS_SYNC:
@@ -149,11 +102,10 @@ def target():
149102

150103
self.executor = _make_executor(target=target)
151104
self.executor.open()
152-
self.assertTrue(ran.wait(timeout=2), "target never ran")
153105
self.executor.join(timeout=2)
106+
self.assertTrue(ran.is_set(), "target never ran")
154107
finally:
155108
threading.excepthook = orig_excepthook
156-
self.assertTrue(self.executor._stopped)
157109
self.assertEqual(len(captured_exc), 1)
158110
self.assertIsInstance(captured_exc[0], RuntimeError)
159111
else:
@@ -165,17 +117,16 @@ async def target():
165117

166118
self.executor = _make_executor(target=target)
167119
self.executor.open()
168-
await asyncio.wait_for(ran.wait(), timeout=2)
169120
await self.executor.join(timeout=2)
170-
self.assertTrue(self.executor._stopped)
121+
self.assertTrue(ran.is_set(), "target never ran")
171122
if self.executor._task is not None and self.executor._task.done():
172123
self.executor._task.exception()
173124

174125
async def test_skip_sleep_flag_skips_interval(self):
175126
call_times = []
176127

177128
async def target():
178-
call_times.append(time.monotonic() if _IS_SYNC else asyncio.get_running_loop().time())
129+
call_times.append(time.monotonic())
179130
if len(call_times) >= 2:
180131
return False
181132
return True
@@ -222,14 +173,9 @@ async def target():
222173
self.executor = _make_executor(target=target)
223174
self.executor.open()
224175
await self.executor.join(timeout=2)
225-
self.assertTrue(self.executor._stopped)
226-
if not _IS_SYNC:
227-
first_task = self.executor._task
228176
self.executor.open()
229177
await self.executor.join(timeout=2)
230178
self.assertGreaterEqual(called[0], 2)
231-
if not _IS_SYNC:
232-
self.assertIsNot(self.executor._task, first_task)
233179

234180

235181
class TestShouldStop(AsyncUnitTest):
@@ -278,8 +224,6 @@ def target():
278224
executor.open()
279225
self.assertTrue(ran.wait(timeout=2), "target never ran")
280226
_shutdown_executors()
281-
executor.join(timeout=2)
282-
self.assertTrue(executor._stopped)
283227

284228
def test_shutdown_executors_safe_when_empty(self):
285229
periodic_executor._EXECUTORS.clear()

test/test_periodic_executor.py

Lines changed: 9 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -54,62 +54,19 @@ def tearDown(self):
5454
self.executor.join(timeout=2)
5555

5656

57-
class TestPeriodicExecutorRepr(UnitTest):
57+
class TestPeriodicExecutor(PeriodicExecutorTestBase):
5858
def test_repr_contains_class_and_name(self):
5959
executor = _make_executor(name="exec")
6060
executor_repr = repr(executor)
6161
self.assertIn("PeriodicExecutor", executor_repr)
6262
self.assertIn("exec", executor_repr)
6363

64-
65-
class TestPeriodicExecutorBasic(PeriodicExecutorTestBase):
66-
def test_wake_sets_event(self):
67-
self.assertFalse(self.executor._event)
68-
self.executor.wake()
69-
self.assertTrue(self.executor._event)
70-
71-
def test_update_interval(self):
72-
self.executor.update_interval(60)
73-
self.assertEqual(self.executor._interval, 60)
74-
75-
def test_skip_sleep(self):
76-
self.assertFalse(self.executor._skip_sleep)
77-
self.executor.skip_sleep()
78-
self.assertTrue(self.executor._skip_sleep)
79-
80-
81-
class TestPeriodicExecutorLifecycle(PeriodicExecutorTestBase):
82-
def test_open_starts_worker(self):
83-
self.executor.open()
84-
if _IS_SYNC:
85-
self.assertIsNotNone(self.executor._thread)
86-
self.assertTrue(self.executor._thread.is_alive())
87-
else:
88-
self.assertIsNotNone(self.executor._task)
89-
90-
def test_close_sets_stopped(self):
91-
self.executor.open()
92-
self.executor.close()
93-
self.assertTrue(self.executor._stopped)
94-
self.executor.join(timeout=1)
95-
9664
def test_join_without_open_is_safe(self):
97-
self.executor.join(timeout=0.01)
98-
99-
def test_multiple_open_calls_have_no_effect(self):
100-
self.executor.open()
101-
if _IS_SYNC:
102-
worker_id = id(self.executor._thread)
103-
else:
104-
worker_id = id(self.executor._task)
105-
self.executor.open()
106-
if _IS_SYNC:
107-
self.assertEqual(worker_id, id(self.executor._thread))
108-
else:
109-
self.assertEqual(worker_id, id(self.executor._task))
110-
65+
try:
66+
self.executor.join(timeout=0.01)
67+
except Exception as e:
68+
self.fail(f"join() raised unexpected Exception {e}")
11169

112-
class TestPeriodicExecutorTarget(PeriodicExecutorTestBase):
11370
def test_target_returning_false_stops_executor(self):
11471
if _IS_SYNC:
11572
ran = threading.Event()
@@ -122,12 +79,8 @@ def target():
12279

12380
self.executor = _make_executor(target=target)
12481
self.executor.open()
125-
if _IS_SYNC:
126-
self.assertTrue(ran.wait(timeout=2), "target never ran")
127-
else:
128-
asyncio.wait_for(ran.wait(), timeout=2)
12982
self.executor.join(timeout=2)
130-
self.assertTrue(self.executor._stopped)
83+
self.assertTrue(ran.is_set(), "target never ran")
13184

13285
def test_target_exception_stops_executor(self):
13386
if _IS_SYNC:
@@ -147,11 +100,10 @@ def target():
147100

148101
self.executor = _make_executor(target=target)
149102
self.executor.open()
150-
self.assertTrue(ran.wait(timeout=2), "target never ran")
151103
self.executor.join(timeout=2)
104+
self.assertTrue(ran.is_set(), "target never ran")
152105
finally:
153106
threading.excepthook = orig_excepthook
154-
self.assertTrue(self.executor._stopped)
155107
self.assertEqual(len(captured_exc), 1)
156108
self.assertIsInstance(captured_exc[0], RuntimeError)
157109
else:
@@ -163,17 +115,16 @@ def target():
163115

164116
self.executor = _make_executor(target=target)
165117
self.executor.open()
166-
asyncio.wait_for(ran.wait(), timeout=2)
167118
self.executor.join(timeout=2)
168-
self.assertTrue(self.executor._stopped)
119+
self.assertTrue(ran.is_set(), "target never ran")
169120
if self.executor._task is not None and self.executor._task.done():
170121
self.executor._task.exception()
171122

172123
def test_skip_sleep_flag_skips_interval(self):
173124
call_times = []
174125

175126
def target():
176-
call_times.append(time.monotonic() if _IS_SYNC else asyncio.get_running_loop().time())
127+
call_times.append(time.monotonic())
177128
if len(call_times) >= 2:
178129
return False
179130
return True
@@ -220,14 +171,9 @@ def target():
220171
self.executor = _make_executor(target=target)
221172
self.executor.open()
222173
self.executor.join(timeout=2)
223-
self.assertTrue(self.executor._stopped)
224-
if not _IS_SYNC:
225-
first_task = self.executor._task
226174
self.executor.open()
227175
self.executor.join(timeout=2)
228176
self.assertGreaterEqual(called[0], 2)
229-
if not _IS_SYNC:
230-
self.assertIsNot(self.executor._task, first_task)
231177

232178

233179
class TestShouldStop(UnitTest):
@@ -276,8 +222,6 @@ def target():
276222
executor.open()
277223
self.assertTrue(ran.wait(timeout=2), "target never ran")
278224
_shutdown_executors()
279-
executor.join(timeout=2)
280-
self.assertTrue(executor._stopped)
281225

282226
def test_shutdown_executors_safe_when_empty(self):
283227
periodic_executor._EXECUTORS.clear()

0 commit comments

Comments
 (0)