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

199 test helpers for assering value reading and configuration #226

Merged

Conversation

Relm-Arrowny
Copy link
Contributor

@Relm-Arrowny Relm-Arrowny commented Apr 17, 2024

This is to fix #199 and possibly #206

Acceptance Criteria

  • Functions in ophyd-async:
  • assert_reading
  • assert_configuration
  • assert_value
  • assert_emitted
  • Tests provided
  • Docs provided where appropriate
  • Issues raised in dodal to replace verefy() and any other old functions that can be superseded

For #206 Need to document 2 ways:

  • Exercise the device directly, i.e. call device.read() and inspect the output. Should use helpers made in Test helpers for assering value, reading and configuration #199
  • This is already convered in test_sensor_reading_shows_value so I only changed it to use helpers made here.
  • Run a plan in the RunEngine and examine the docs. Use a pattern like:
  • Added test_sensor_in_plan in demo.py together with changes in the documentation.

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.

Looks like this is on the right track, thanks!

src/ophyd_async/core/signal.py Outdated Show resolved Hide resolved
src/ophyd_async/core/signal.py Outdated Show resolved Hide resolved
src/ophyd_async/core/signal.py Outdated Show resolved Hide resolved
src/ophyd_async/core/signal.py Outdated Show resolved Hide resolved
src/ophyd_async/core/signal.py Outdated Show resolved Hide resolved
src/ophyd_async/core/signal.py Outdated Show resolved Hide resolved
await verify_readable(configurable.read_configuration, configuration)


def assert_emitted(docs: Dict[str, list], **numbers: int):
Copy link
Contributor

Choose a reason for hiding this comment

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

You can import DocumentType from event model

Suggested change
def assert_emitted(docs: Dict[str, list], **numbers: int):
def assert_emitted(docs: DocumentType, **numbers: int):

Copy link
Collaborator

Choose a reason for hiding this comment

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

Although docs is a dict of document name to list of document instances, so I think that should be:

Suggested change
def assert_emitted(docs: Dict[str, list], **numbers: int):
def assert_emitted(docs: Dict[str, list[DocumentType]], **numbers: int):

Except DocumentType is actually the types, which is not what we want, we would quite like a similar Document union in event model...

Copy link
Collaborator

Choose a reason for hiding this comment

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

so in the absence of that it's probably as correct as it could be...

src/ophyd_async/core/signal.py Show resolved Hide resolved
@Relm-Arrowny Relm-Arrowny marked this pull request as ready for review April 18, 2024 15:27
src/ophyd_async/core/signal.py Outdated Show resolved Hide resolved
src/ophyd_async/core/signal.py Outdated Show resolved Hide resolved
@coretl
Copy link
Collaborator

coretl commented Apr 19, 2024

Please could you also add to the "writing tests for devices" doc the information on how to use assert_emitted from #206

@Relm-Arrowny
Copy link
Contributor Author

I will deal with #206 the conflicts and all the changes once the main branch is stable, atm the doc will not build correctly.

@Relm-Arrowny Relm-Arrowny linked an issue Apr 19, 2024 that may be closed by this pull request
@Relm-Arrowny
Copy link
Contributor Author

I think I got everything, but please have a quick check if it all make sense and I have covered everything.

Copy link
Contributor

@callumforrester callumforrester left a comment

Choose a reason for hiding this comment

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

Docs look good, only a couple of formatting points

docs/how-to/write-tests-for-devices.rst Outdated Show resolved Hide resolved
docs/how-to/write-tests-for-devices.rst Outdated Show resolved Hide resolved
Relm-Arrowny and others added 2 commits April 24, 2024 09:26
src/ophyd_async/core/signal.py Outdated Show resolved Hide resolved
src/ophyd_async/core/signal.py Outdated Show resolved Hide resolved
Copy link
Contributor

@callumforrester callumforrester left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

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.

Sorry, one last thing

src/ophyd_async/core/signal.py Outdated Show resolved Hide resolved
@Relm-Arrowny Relm-Arrowny merged commit ec4e61d into main Apr 29, 2024
18 checks passed
@Relm-Arrowny Relm-Arrowny deleted the 199-test-helpers-for-assering-value-reading-and-configuration branch April 29, 2024 09:04
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.

Write docs on how to do testing Test helpers for assering value, reading and configuration
3 participants