Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a fixtures that checks if tasks are still running in tests #538

Merged
merged 8 commits into from
Sep 17, 2024

Conversation

evalott100
Copy link
Contributor

Closes #519

@evalott100
Copy link
Contributor Author

With the additions in the above the modified test correcly failed with:

ERROR tests/core/test_status.py::test_async_status_exception_timeout - RuntimeError: Not all tasks [<Task cancelling name='Task-1481' coro=<sleep() running at /dls_sw/apps/python/miniforge/4.10.0-0/envs/python3.11/lib/python3.11/asyncio/tasks.py:639> wait_for=<Future cancelled> cb=[AsyncStatusBase._run_callbacks()]>] closed during test test_async_status_exception_timeout.

Now it's been changed. We still need to track down what's causing the Task destroyed but it is pending - I would assume we need to add the same check to the RunEngine cleanup.

@evalott100
Copy link
Contributor Author

With the changes to the RunEngine we get explicit errors in the tests creating tasks that aren't awaited/cancelled.

There seems to be some nasty behavior in tests when devices connected to outside the RunEngine loop are used within the RunEngine (I've already corrected some of these tests in the same commit). I did some experimenting to see if could creatively change the RunEngine's event loop to avoid this, but no joy... I'll adjust the rest and see if I can find a way to warn the user if they use devices connected in a different event loop within the RunEngine.

def RE(request: FixtureRequest):
loop = asyncio.new_event_loop()
loop.set_debug(True)
RE = RunEngine({}, call_returns_result=True, loop=loop)

=============================================== short test summary info ===============================================
ERROR tests/epics/adsimdetector/test_sim.py::test_detector_with_unnamed_or_disconnected_config_sigs[-config signal must be named before it is passed to the detector] - RuntimeError: Not all tasks {<Task pending name='Task-8496' coro=<StandardDetector.unstage() running at /scratch/twj43146/Programming/ophyd-async/src/ophyd_async/core/_detector.py:227> wait_for=<Future pending cb=[Task.task_wakeup()] created at /dls_sw/apps/python/miniforge/4.10.0-0/envs/python3.11/lib/python3.11/asyncio/base_events.py:427> cb=[AsyncStatusBase._run_callbacks()] created at /dls_sw/apps/python/miniforge/4.10.0-0/envs/python3.11/lib/python3.11/asyncio/tasks.py:374>} closed during test test_detector_with_unnamed_or_disconnected_config_sigs[-config signal must be named before it is passed to the detector].
ERROR tests/epics/adsimdetector/test_sim.py::test_detector_with_unnamed_or_disconnected_config_sigs[some-name-config signal some-name-acquire_time must be connected before it is passed to the detector] - RuntimeError: Not all tasks {<Task pending name='Task-8596' coro=<StandardDetector.unstage() running at /scratch/twj43146/Programming/ophyd-async/src/ophyd_async/core/_detector.py:227> wait_for=<Future pending cb=[Task.task_wakeup()] created at /dls_sw/apps/python/miniforge/4.10.0-0/envs/python3.11/lib/python3.11/asyncio/base_events.py:427> cb=[AsyncStatusBase._run_callbacks()] created at /dls_sw/apps/python/miniforge/4.10.0-0/envs/python3.11/lib/python3.11/asyncio/tasks.py:374>} closed during test test_detector_with_unnamed_or_disconnected_config_sigs[some-name-config signal some-name-acquire_time must be connected before it is passed to the detector].
ERROR tests/sim/test_sim_detector.py::test_writes_pattern_to_file - RuntimeError: Not all tasks {<Task pending name='Task-17379' coro=<PatternDetectorController._coroutine_for_image_writing() running at /scratch/twj43146/Programming/ophyd-async/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py:55> wait_for=<Future pending cb=[Task.task_wakeup()] created at /dls_sw/apps/python/miniforge/4.10.0-0/envs/python3.11/lib/python3.11/asyncio/base_events.py:427> cb=[AsyncStatusBase._run_callbacks()] created at /dls_sw/apps/python/miniforge/4.10.0-0/envs/python3.11/lib/python3.11/asyncio/tasks.py:374>} closed during test test_writes_pattern_to_file.
ERROR tests/sim/test_streaming_plan.py::test_streaming_plan - RuntimeError: Not all tasks {<Task pending name='Task-17410' coro=<PatternDetectorController._coroutine_for_image_writing() running at /scratch/twj43146/Programming/ophyd-async/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py:55> wait_for=<Future pending cb=[Task.task_wakeup()] created at /dls_sw/apps/python/miniforge/4.10.0-0/envs/python3.11/lib/python3.11/asyncio/base_events.py:427> cb=[AsyncStatusBase._run_callbacks()] created at /dls_sw/apps/python/miniforge/4.10.0-0/envs/python3.11/lib/python3.11/asyncio/tasks.py:374>} closed during test test_streaming_plan.
ERROR tests/sim/test_streaming_plan.py::test_plan - RuntimeError: Not all tasks {<Task pending name='Task-17435' coro=<PatternDetectorController._coroutine_for_image_writing() running at /scratch/twj43146/Programming/ophyd-async/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py:55> wait_for=<Future pending cb=[Task.task_wakeup()] created at /dls_sw/apps/python/miniforge/4.10.0-0/envs/python3.11/lib/python3.11/asyncio/base_events.py:427> cb=[AsyncStatusBase._run_callbacks()] created at /dls_sw/apps/python/miniforge/4.10.0-0/envs/python3.11/lib/python3.11/asyncio/tasks.py:374>} closed during test test_plan.
====================================== 423 passed, 1 skipped, 5 errors in 26.88s ======================================

@evalott100 evalott100 force-pushed the 519-correctly-handle-pending-tasks-in-tests branch from 24c7ccf to ff55783 Compare August 22, 2024 14:22
@evalott100 evalott100 marked this pull request as draft August 23, 2024 08:19
@evalott100 evalott100 marked this pull request as ready for review August 27, 2024 07:58
@evalott100
Copy link
Contributor Author

evalott100 commented Aug 27, 2024

Currently, detector controller is only disarmed once:

@AsyncStatus.wrap
async def stage(self) -> None:
# Disarm the detector, stop filewriting.
await self._check_config_sigs()
await asyncio.gather(self.writer.close(), self.controller.disarm())
self._trigger_info = None

However it is armed in both trigger and prepare:

# Arm the detector and wait for it to finish.
indices_written = await self.writer.get_indices_written()
written_status = await self.controller.arm(
num=self._trigger_info.number,
trigger=self._trigger_info.trigger,

self._arm_status = await self.controller.arm(
num=self._trigger_info.number,
trigger=self._trigger_info.trigger,
exposure=self._trigger_info.livetime,
)

This is the cause of all the test failure's here. We should make a seperate issue for it.

tests/conftest.py Outdated Show resolved Hide resolved
tests/epics/adcore/test_single_trigger.py Show resolved Hide resolved
@evalott100
Copy link
Contributor Author

evalott100 commented Sep 9, 2024

Interestingly, the standard detector unstage error is only present if debug mode is set to true in the RE event loop, though it will still raise an error from the new clean up logic.

@evalott100 evalott100 force-pushed the 519-correctly-handle-pending-tasks-in-tests branch from 4959d1d to f01f90b Compare September 9, 2024 14:59
Copy link
Collaborator

@coretl coretl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we wait for #542 to be fixed before merging this?

tests/epics/adcore/test_single_trigger.py Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
@evalott100 evalott100 force-pushed the 519-correctly-handle-pending-tasks-in-tests branch from f298fb6 to e17bcd0 Compare September 10, 2024 09:43
@evalott100 evalott100 force-pushed the 519-correctly-handle-pending-tasks-in-tests branch from e17bcd0 to 0748db8 Compare September 17, 2024 07:37
@evalott100 evalott100 force-pushed the 519-correctly-handle-pending-tasks-in-tests branch from 0748db8 to 8f5c4c7 Compare September 17, 2024 07:40
@evalott100 evalott100 merged commit 989beeb into main Sep 17, 2024
18 checks passed
@evalott100 evalott100 deleted the 519-correctly-handle-pending-tasks-in-tests branch September 17, 2024 08:01
tomtrafford pushed a commit that referenced this pull request Oct 8, 2024
added autorun fixtures that check if tasks are still running and fail on the test level

also corrected some tests with incorrect event-loop usage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Correctly handle pending tasks in our tests
2 participants