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

Assert statements broken in tests #618

Closed
coretl opened this issue Oct 22, 2024 · 5 comments · Fixed by #702
Closed

Assert statements broken in tests #618

coretl opened this issue Oct 22, 2024 · 5 comments · Fixed by #702
Assignees
Labels
bug Something isn't working

Comments

@coretl
Copy link
Collaborator

coretl commented Oct 22, 2024

          Seems like `assert_configuration()` (and presumably also `assert_reading()`) are broken for numpy array types. I guess they probably were already broken before these changes, but one of the tests in dodal uses assert_configuration and I encountered this as I'm currently in the process of going through dodal and mx-bluesky to test out this PR.

I will keep you posted if I find any other issues.

Originally posted by @rtuck99 in #594 (comment)

I suggest that we move the assert functions into a new ophyd_async.testing package, make pytest a dependency of it, then use pytest.approx on the value

@coretl
Copy link
Collaborator Author

coretl commented Nov 22, 2024

The other alternative is to document that you should use pytest.approx in your test code when testing for arrays:

async def test_assert_reading_array(mock_signal_array: SignalRW):
    set_mock_value(mock_signal_array, [1, 2, 4.5])
    dummy_reading = {
        "mock_signal": Reading(
            {"alarm_severity": 0, "timestamp": ANY, "value": pytest.approx([1, 2, 4.5])}
        )
    }
    await assert_reading(mock_signal_array, dummy_reading)

@rtuck99 do you prefer moving ophyd_async.core.asset_* to ophyd_async.testing.assert_* and adding the pytest approx there, or writing docs that tell people to put pytest.approx in their code?

@coretl
Copy link
Collaborator Author

coretl commented Nov 26, 2024

An in between option: do the pytest import directly inside assert_reading, wrapping any numpy array value with pytest.approx and adding "timestamp": pytest.ANY and "alarm_severity": 0 if they don't already exist

@stan-dot stan-dot added the bug Something isn't working label Nov 26, 2024
@stan-dot stan-dot changed the title Seems like assert_configuration() (and presumably also assert_reading()) are broken for numpy array types. I guess they probably were already broken before these changes, but one of the tests in dodal uses assert_configuration and I encountered this as I'm currently in the process of going through dodal and mx-bluesky to test out this PR. Assert statements broken in tests Nov 26, 2024
@coretl
Copy link
Collaborator Author

coretl commented Dec 9, 2024

Decided to move things to ophyd_async.testing in #695, so this ticket is to add pytest.approx to the functions

@coretl
Copy link
Collaborator Author

coretl commented Dec 9, 2024

@Relm-Arrowny are you interested in taking this on?

@Relm-Arrowny
Copy link
Contributor

@Relm-Arrowny are you interested in taking this on?

sure, sound easy enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants