search

bluesky / ophyd-async

Hardware abstraction for bluesky written using asyncio
https://blueskyproject.io/ophyd-async
BSD 3-Clause "New" or "Revised" License
12 stars 26 forks source link

539 put mock should be async #559

Closed DominicOram closed 2 months ago

DominicOram commented 2 months ago

Fixes https://github.com/bluesky/ophyd-async/issues/539

To test:

Note: It's not 100% clear to me why the tests fail without the second commit. Clearly something to do with the patch leaking between tests but I'm not sure why it would be leaking or why this would only appear given the async mock change. However, the tests seem to pass fine without the patches and in general patching less seems good so I'm inclined to just move on.

evalott100 commented 2 months ago

It's not 100% clear to me why the tests fail without the second commit. Clearly something to do with the patch leaking between tests

There's some unsafe status' throughout the tests, I'm working on a PR to fix this and cause failure during tests where these status' are created, with exact info on where the status is, rather than failure in later tests when the weak ref status' happen to be garbage collected.

https://github.com/bluesky/ophyd-async/issues/519

I looked through the call stack on ad.arm/ad.disarm and I couldn't tell which status it might be. I think it's probably a consequence of some other status not being cancelled/awaited by the end of another test, the removal of the fixture didn't actually do anything (I reran tests for that commit and there are no errors now).

That being said, it doesn't seem like we need to mock wait_for_value here anyway so we should keep that commit.

DominicOram commented 2 months ago

I can't reproduce the new error locally and not sure where it's from. Are you able to reproduce it Eva?

coretl commented 2 months ago

I think we are still waiting for #542 before the fix to #519. I suggest we merge this as is, then fix any test failures that remain after those 2 issues are closed.