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

Maybe: a fixture which allows for easier handling of persistent tasks #734

Open
evalott100 opened this issue Jan 14, 2025 · 0 comments
Open

Comments

@evalott100
Copy link
Contributor

evalott100 commented Jan 14, 2025

Thus far, we've had the philosophy that if there is a task still running by the end of a test then the test is erroneous. This is true in the literal sense that it will cause python tear down errors in the test, but for some operations, e.g tasks running while other futures in the event loop enter fail states, it could be perfectly acceptable that in production the task would be kicking around for a bit before failure.

Take the following aravis test:

async def test_unsupported_trigger_excepts(test_adaravis: adaravis.AravisDetector):
with patch(
"ophyd_async.epics.adcore._hdf_writer.ADHDFWriter.open", new_callable=AsyncMock
) as mock_open:
with pytest.raises(
ValueError,
# str(EnumClass.value) handling changed in Python 3.11
match=(
"AravisController only supports the following trigger types: .* but"
),
):
await test_adaravis.prepare(
TriggerInfo(
number_of_triggers=0,
trigger=DetectorTrigger.VARIABLE_GATE,
deadtime=1,
livetime=1,
frame_timeout=3,
)
)
mock_open.assert_called_once()

Instead of mocking private members the developer should be able to articulate that there'll be a certain task which will still be running at the end of the test.

Suggestion

A fixture which runs before the finalizer which checks for unfinished tasks at the end of tests:

class AsyncTaskHandler
    def __init__(self):
        self._allowed_pending_tasks = {}

    def expect_remaining_tasks(*task_names: str):
        self._allowed_pending_tasks.update(set(task_names))

    def close_expected_tasks(self):
        """Closes tasks from self._allowed_pending_tasks"""

@pytest.fixture
def pending_tasks_handler()
    handler = AsyncTaskHandler()
    yield handler
    handler.close_expected_tasks()

...

async def test_unsupported_trigger_excepts(test_adaravis: adaravis.AravisDetector, pending_tasks_handler):
    pending_tasks_handler.expect_remaining_tasks("ADHDFWriter.open")
    with pytest.raises(
        ValueError,
        # str(EnumClass.value) handling changed in Python 3.11
        match=(
            "AravisController only supports the following trigger types: .* but"
        ),
    ):
        await test_adaravis.prepare(
            TriggerInfo(
                number_of_triggers=0,
                trigger=DetectorTrigger.VARIABLE_GATE,
                deadtime=1,
                livetime=1,
                frame_timeout=3,
            )
        )

This forces developers to keep in mind what tasks will still be around in error states. It's clear from the code that this will happen while open is still pending. It could also be used in a context which could kill the tasks within a portion of a test, e.g:

with pending_tasks_handler("ADHDFWriter.open", "some_other_task"):
   ...

Originally posted by @evalott100 in #730 (comment)

Considerations

Would this have use on the plan level?

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

No branches or pull requests

1 participant