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.

Sign up for GitHub

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

Jump to bottom

Add Python 3.12 to CI now that p4p is updated #655

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+43 −24
Open

Add Python 3.12 to CI now that p4p is updated #655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
47a6f45
Add Python 3.12 to CI and pyproject.toml now that p4p is updated
OCopping Nov 19, 2024
d15f3b3
Replace get_event_loop() in conftest.py as deprecated in Python 3.12
OCopping Nov 19, 2024
db1746f
Add the python package and the config and make the contracts kept (#669)
stan-dot Nov 25, 2024
7633bc3
only fail on unclosed tasks with a running event loop
evalott100 Nov 27, 2024
6368080
fixed tests other than `_event_processor` deprecation warning
evalott100 Nov 27, 2024
ad985e7
ignored tango deprecation warning
evalott100 Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
strategy:
matrix:
runs-on: ["ubuntu-latest", "windows-latest"] # can add macos-latest
python-version: ["3.10", "3.11"] # 3.12 should be added when p4p is updated
python-version: ["3.10","3.11","3.12"]
include:
# Include one that runs in the dev environment
- runs-on: "ubuntu-latest"
Expand Down
9 changes: 7 additions & 2 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ classifiers = [
"License :: OSI Approved :: BSD License",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.12",
]
description = "Asynchronous Bluesky hardware abstraction code, compatible with control systems like EPICS and Tango"
dependencies = [
Expand Down Expand Up @@ -98,8 +99,12 @@ addopts = """
--doctest-glob="*.rst" --doctest-glob="*.md"
--ignore=docs/examples --ignore=src/ophyd_async/epics/signal.py
"""
# https://iscinumpy.gitlab.io/post/bound-version-constraints/#watch-for-warnings
filterwarnings = "error"
# "error" for https://iscinumpy.gitlab.io/post/bound-version-constraints/#watch-for-warnings
# "tango" for https://github.com/bluesky/ophyd-async/issues/681
filterwarnings = """
error
ignore::DeprecationWarning:tango.asyncio_executor:
"""
# Doctest python code in docs, python code in src docstrings, test functions in tests
testpaths = "docs src tests"
log_format = "%(asctime)s,%(msecs)03d %(levelname)s (%(threadName)s) %(message)s"
Expand Down
25 changes: 17 additions & 8 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ def _error_and_kill_pending_tasks(
unfinished_tasks = {
task
for task in asyncio.all_tasks(loop)
if task.get_coro().__name__ not in _ALLOWED_PYTEST_TASKS and not task.done()
if (coro := task.get_coro()) is not None
and coro.__name__ not in _ALLOWED_PYTEST_TASKS
and not task.done()
}
for task in unfinished_tasks:
task.cancel()
Expand All @@ -113,15 +115,22 @@ def fail_test_on_unclosed_tasks(request: FixtureRequest):
by the end of the test.
"""

fail_count = request.session.testsfailed
loop = asyncio.get_event_loop()
loop.set_debug(True)
try:
fail_count = request.session.testsfailed
loop = asyncio.get_running_loop()
Copy link
Contributor

Choose a reason for hiding this comment

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

get_event_loop gives a deprecation warning that future python versions will raise an error if there is no event loop, get_running_loop does this already. It doesn't seem possible to check for an event loop without failing/warning if there isn't one, so we just catch the exception for the tests which don't use one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not ready to merge yet. After changing this fixture to be async (which would use the same event loop as the async test about to be started), I had a bunch more errors. Will tackle tomorrow!

Copy link
Contributor

@evalott100 evalott100 Nov 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Making this method on both main and this branch causes unfinished tasks to be properly caught, and (the same) tests to fail.

#683

We'll also be able to remove this try catch since the event-loop will have to be open for the async fixture to be running.


loop.set_debug(True)

request.addfinalizer(
lambda: _error_and_kill_pending_tasks(
loop, request.node.name, request.session.testsfailed == fail_count
request.addfinalizer(
lambda: _error_and_kill_pending_tasks(
loop, request.node.name, request.session.testsfailed == fail_count
)
)
)
# Once https://github.com/bluesky/ophyd-async/issues/683
# is finished we can remove this try, except.
except RuntimeError as error:
if str(error) != "no running event loop":
raise error
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Is it worth having a pass explaining that if str(error) == "no running event loop" then the test either does not require a loop or will fail when attempting to use the loop?

Copy link
Contributor

@evalott100 evalott100 Nov 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is it worth having a pass

What do you mean by this?

except RuntimeError as error:
    if str(error) == "no running event loop":
        # Explain
        pass
    else:
        raise error

?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes exactly that, just a bit more obvious what is intended

Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer the current != comprehension but I'll add a comment explaining!



@pytest.fixture(scope="function")
Expand Down
1 change: 1 addition & 0 deletions tests/core/test_mock_signal_backend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ async def test_blocks_during_put(mock_signals):
with mock_puts_blocked(signal1, signal2):
status1 = signal1.set("second_value", wait=True, timeout=None)
status2 = signal2.set("second_value", wait=True, timeout=None)
await asyncio.sleep(0.1)
assert await signal1.get_value() == "second_value"
assert await signal2.get_value() == "second_value"
assert not status1.done
Expand Down
16 changes: 8 additions & 8 deletions tests/core/test_readable.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ def test_standard_readable_hints():

assert sr.hints == {}

hint1 = MagicMock()
hint1 = MagicMock(spec=HasHints)
evalott100 marked this conversation as resolved.
Show resolved Hide resolved
hint1.hints = {"fields": ["abc"], "dimensions": [(["f1", "f2"], "s1")]}

hint2 = MagicMock()
hint2 = MagicMock(spec=HasHints)
hint2.hints = {"fields": ["def", "ghi"]}

hint3 = MagicMock()
hint3 = MagicMock(spec=HasHints)
hint3.hints = {"fields": ["jkl"], "gridding": "rectilinear_nonsequential"}

sr.add_readables([hint1, hint2, hint3])
Expand All @@ -53,10 +53,10 @@ def test_standard_readable_hints():
def test_standard_readable_hints_raises_when_overriding_string_literal():
sr = StandardReadable()

hint1 = MagicMock()
hint1 = MagicMock(spec=HasHints)
hint1.hints = {"gridding": "rectilinear_nonsequential"}

hint2 = MagicMock()
hint2 = MagicMock(spec=HasHints)
hint2.hints = {"gridding": "a different string"}

sr._has_hints = (
Expand All @@ -71,10 +71,10 @@ def test_standard_readable_hints_raises_when_overriding_string_literal():
def test_standard_readable_hints_raises_when_overriding_sequence():
sr = StandardReadable()

hint1 = MagicMock()
hint1 = MagicMock(spec=HasHints)
hint1.hints = {"fields": ["field1", "field2"]}

hint2 = MagicMock()
hint2 = MagicMock(spec=HasHints)
hint2.hints = {"fields": ["field2"]}

sr._has_hints = (
Expand All @@ -90,7 +90,7 @@ def test_standard_readable_hints_raises_when_overriding_sequence():
def test_standard_readable_hints_invalid_types(invalid_type):
sr = StandardReadable()

hint1 = MagicMock()
hint1 = MagicMock(spec=HasHints)
hint1.hints = {"test": invalid_type}

sr._has_hints = (hint1,)
Expand Down
8 changes: 5 additions & 3 deletions tests/epics/adcore/test_drivers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,12 @@ async def wait_then_fail():
await asyncio.sleep(0)
set_mock_value(driver.detector_state, adcore.DetectorState.DISCONNECTED)

await wait_then_fail()

acquiring = await adcore.start_acquiring_driver_and_ensure_status(
driver, timeout=0.1
)
await wait_then_fail()

with pytest.raises(ValueError):
with pytest.raises(
ValueError, match="Final detector state DetectorState.DISCONNECTED"
):
Comment on lines +98 to +100
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be starting setting up wait_then_fail before we schedule the task which should fail.

await acquiring
6 changes: 4 additions & 2 deletions tests/sim/demo/test_sim_motor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import asyncio
import time

import pytest
from bluesky.plans import spiral_square
from bluesky.run_engine import RunEngine

Expand Down Expand Up @@ -49,6 +50,7 @@ async def test_stop():
new_pos = await m1.user_readback.get_value()
assert new_pos < 10
assert new_pos >= 0.1
# move should not be successful as we stopped it
assert move_status.done

assert not move_status.success
with pytest.raises(RuntimeError, match="Motor was stopped"):
await move_status
evalott100 marked this conversation as resolved.
Show resolved Hide resolved
Loading