Skip to content

Commit bcf54c7

Browse files
committed
Noah feedback
1 parent 0c0ab1a commit bcf54c7

2 files changed

Lines changed: 18 additions & 120 deletions

File tree

test/asynchronous/test_periodic_executor.py

Lines changed: 9 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -56,57 +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_update_interval(self):
69-
self.executor.update_interval(60)
70-
self.assertEqual(self.executor._interval, 60)
71-
72-
async def test_skip_sleep(self):
73-
self.assertFalse(self.executor._skip_sleep)
74-
self.executor.skip_sleep()
75-
self.assertTrue(self.executor._skip_sleep)
76-
77-
78-
class TestAsyncPeriodicExecutorLifecycle(AsyncPeriodicExecutorTestBase):
79-
async def test_open_starts_worker(self):
80-
self.executor.open()
81-
if _IS_SYNC:
82-
self.assertIsNotNone(self.executor._thread)
83-
self.assertTrue(self.executor._thread.is_alive())
84-
else:
85-
self.assertIsNotNone(self.executor._task)
86-
87-
async def test_close_sets_stopped(self):
88-
self.executor.open()
89-
self.executor.close()
90-
self.assertTrue(self.executor._stopped)
91-
await self.executor.join(timeout=1)
92-
9366
async def test_join_without_open_is_safe(self):
94-
await self.executor.join(timeout=0.01)
67+
try:
68+
await self.executor.join(timeout=0.01)
69+
except Exception as e:
70+
self.fail(f"join() raised unexpected Exception {e}")
9571

96-
async def test_multiple_open_calls_have_no_effect(self):
97-
self.executor.open()
98-
if _IS_SYNC:
99-
worker_id = id(self.executor._thread)
100-
else:
101-
worker_id = id(self.executor._task)
102-
self.executor.open()
103-
if _IS_SYNC:
104-
self.assertEqual(worker_id, id(self.executor._thread))
105-
else:
106-
self.assertEqual(worker_id, id(self.executor._task))
107-
108-
109-
class TestAsyncPeriodicExecutorTarget(AsyncPeriodicExecutorTestBase):
11072
async def test_target_returning_false_stops_executor(self):
11173
if _IS_SYNC:
11274
ran = threading.Event()
@@ -119,12 +81,8 @@ async def target():
11981

12082
self.executor = _make_executor(target=target)
12183
self.executor.open()
122-
if _IS_SYNC:
123-
self.assertTrue(ran.wait(timeout=2), "target never ran")
124-
else:
125-
await asyncio.wait_for(ran.wait(), timeout=2)
12684
await self.executor.join(timeout=2)
127-
self.assertTrue(self.executor._stopped)
85+
self.assertTrue(ran.is_set(), "target never ran")
12886

12987
async def test_target_exception_stops_executor(self):
13088
if _IS_SYNC:
@@ -144,11 +102,10 @@ def target():
144102

145103
self.executor = _make_executor(target=target)
146104
self.executor.open()
147-
self.assertTrue(ran.wait(timeout=2), "target never ran")
148105
self.executor.join(timeout=2)
106+
self.assertTrue(ran.is_set(), "target never ran")
149107
finally:
150108
threading.excepthook = orig_excepthook
151-
self.assertTrue(self.executor._stopped)
152109
self.assertEqual(len(captured_exc), 1)
153110
self.assertIsInstance(captured_exc[0], RuntimeError)
154111
else:
@@ -160,17 +117,16 @@ async def target():
160117

161118
self.executor = _make_executor(target=target)
162119
self.executor.open()
163-
await asyncio.wait_for(ran.wait(), timeout=2)
164120
await self.executor.join(timeout=2)
165-
self.assertTrue(self.executor._stopped)
121+
self.assertTrue(ran.is_set(), "target never ran")
166122
if self.executor._task is not None and self.executor._task.done():
167123
self.executor._task.exception()
168124

169125
async def test_skip_sleep_flag_skips_interval(self):
170126
call_times = []
171127

172128
async def target():
173-
call_times.append(time.monotonic() if _IS_SYNC else asyncio.get_running_loop().time())
129+
call_times.append(time.monotonic())
174130
if len(call_times) >= 2:
175131
return False
176132
return True
@@ -217,14 +173,9 @@ async def target():
217173
self.executor = _make_executor(target=target)
218174
self.executor.open()
219175
await self.executor.join(timeout=2)
220-
self.assertTrue(self.executor._stopped)
221-
if not _IS_SYNC:
222-
first_task = self.executor._task
223176
self.executor.open()
224177
await self.executor.join(timeout=2)
225178
self.assertGreaterEqual(called[0], 2)
226-
if not _IS_SYNC:
227-
self.assertIsNot(self.executor._task, first_task)
228179

229180

230181
class TestShouldStop(AsyncUnitTest):
@@ -273,8 +224,6 @@ def target():
273224
executor.open()
274225
self.assertTrue(ran.wait(timeout=2), "target never ran")
275226
_shutdown_executors()
276-
executor.join(timeout=2)
277-
self.assertTrue(executor._stopped)
278227

279228
def test_shutdown_executors_safe_when_empty(self):
280229
periodic_executor._EXECUTORS.clear()

test/test_periodic_executor.py

Lines changed: 9 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -54,57 +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_update_interval(self):
67-
self.executor.update_interval(60)
68-
self.assertEqual(self.executor._interval, 60)
69-
70-
def test_skip_sleep(self):
71-
self.assertFalse(self.executor._skip_sleep)
72-
self.executor.skip_sleep()
73-
self.assertTrue(self.executor._skip_sleep)
74-
75-
76-
class TestPeriodicExecutorLifecycle(PeriodicExecutorTestBase):
77-
def test_open_starts_worker(self):
78-
self.executor.open()
79-
if _IS_SYNC:
80-
self.assertIsNotNone(self.executor._thread)
81-
self.assertTrue(self.executor._thread.is_alive())
82-
else:
83-
self.assertIsNotNone(self.executor._task)
84-
85-
def test_close_sets_stopped(self):
86-
self.executor.open()
87-
self.executor.close()
88-
self.assertTrue(self.executor._stopped)
89-
self.executor.join(timeout=1)
90-
9164
def test_join_without_open_is_safe(self):
92-
self.executor.join(timeout=0.01)
65+
try:
66+
self.executor.join(timeout=0.01)
67+
except Exception as e:
68+
self.fail(f"join() raised unexpected Exception {e}")
9369

94-
def test_multiple_open_calls_have_no_effect(self):
95-
self.executor.open()
96-
if _IS_SYNC:
97-
worker_id = id(self.executor._thread)
98-
else:
99-
worker_id = id(self.executor._task)
100-
self.executor.open()
101-
if _IS_SYNC:
102-
self.assertEqual(worker_id, id(self.executor._thread))
103-
else:
104-
self.assertEqual(worker_id, id(self.executor._task))
105-
106-
107-
class TestPeriodicExecutorTarget(PeriodicExecutorTestBase):
10870
def test_target_returning_false_stops_executor(self):
10971
if _IS_SYNC:
11072
ran = threading.Event()
@@ -117,12 +79,8 @@ def target():
11779

11880
self.executor = _make_executor(target=target)
11981
self.executor.open()
120-
if _IS_SYNC:
121-
self.assertTrue(ran.wait(timeout=2), "target never ran")
122-
else:
123-
asyncio.wait_for(ran.wait(), timeout=2)
12482
self.executor.join(timeout=2)
125-
self.assertTrue(self.executor._stopped)
83+
self.assertTrue(ran.is_set(), "target never ran")
12684

12785
def test_target_exception_stops_executor(self):
12886
if _IS_SYNC:
@@ -142,11 +100,10 @@ def target():
142100

143101
self.executor = _make_executor(target=target)
144102
self.executor.open()
145-
self.assertTrue(ran.wait(timeout=2), "target never ran")
146103
self.executor.join(timeout=2)
104+
self.assertTrue(ran.is_set(), "target never ran")
147105
finally:
148106
threading.excepthook = orig_excepthook
149-
self.assertTrue(self.executor._stopped)
150107
self.assertEqual(len(captured_exc), 1)
151108
self.assertIsInstance(captured_exc[0], RuntimeError)
152109
else:
@@ -158,17 +115,16 @@ def target():
158115

159116
self.executor = _make_executor(target=target)
160117
self.executor.open()
161-
asyncio.wait_for(ran.wait(), timeout=2)
162118
self.executor.join(timeout=2)
163-
self.assertTrue(self.executor._stopped)
119+
self.assertTrue(ran.is_set(), "target never ran")
164120
if self.executor._task is not None and self.executor._task.done():
165121
self.executor._task.exception()
166122

167123
def test_skip_sleep_flag_skips_interval(self):
168124
call_times = []
169125

170126
def target():
171-
call_times.append(time.monotonic() if _IS_SYNC else asyncio.get_running_loop().time())
127+
call_times.append(time.monotonic())
172128
if len(call_times) >= 2:
173129
return False
174130
return True
@@ -215,14 +171,9 @@ def target():
215171
self.executor = _make_executor(target=target)
216172
self.executor.open()
217173
self.executor.join(timeout=2)
218-
self.assertTrue(self.executor._stopped)
219-
if not _IS_SYNC:
220-
first_task = self.executor._task
221174
self.executor.open()
222175
self.executor.join(timeout=2)
223176
self.assertGreaterEqual(called[0], 2)
224-
if not _IS_SYNC:
225-
self.assertIsNot(self.executor._task, first_task)
226177

227178

228179
class TestShouldStop(UnitTest):
@@ -271,8 +222,6 @@ def target():
271222
executor.open()
272223
self.assertTrue(ran.wait(timeout=2), "target never ran")
273224
_shutdown_executors()
274-
executor.join(timeout=2)
275-
self.assertTrue(executor._stopped)
276225

277226
def test_shutdown_executors_safe_when_empty(self):
278227
periodic_executor._EXECUTORS.clear()

0 commit comments

Comments
 (0)