Skip to content

fix flaky test in test_bart_robot.py#947

Merged
olliesilvester merged 8 commits intoDiamondLightSource:mainfrom
arikaran-13:arikaran-13/issue-#943-fix
Dec 10, 2024
Merged

fix flaky test in test_bart_robot.py#947
olliesilvester merged 8 commits intoDiamondLightSource:mainfrom
arikaran-13:arikaran-13/issue-#943-fix

Conversation

@arikaran-13
Copy link
Contributor

@arikaran-13 arikaran-13 commented Dec 5, 2024

#943 Fix flakey bart robot unit test

Instructions to reviewer on how to test:

  1. Run the test tests/devices/unit_tests/test_bart_robot.py::test_given_program_not_running_and_pin_unmounts_then_mounts_when_load_pin_then_pin_loaded
  2. Confirm that the test passes consistently, even if run multiple times.

Checks for reviewer

  • Would the PR title make sense to a scientist on a set of release notes
  • If a new device has been added does it follow the standards
  • If changing the API for a pre-existing device, ensure that any beamlines using this device have updated their Bluesky plans accordingly
  • Have the connection tests for the relevant beamline(s) been run via dodal connect ${BEAMLINE}

DiamondLightSource#943 Fix flakey bart robot unit test
@arikaran-13 arikaran-13 requested a review from a team as a code owner December 5, 2024 17:11
@arikaran-13 arikaran-13 changed the title fix flaky test_given_program_not_running_and_pin_unmounts_then_mounts_when_load_pin_then_pin_loaded fix flaky test in test_bart_robot.py Dec 5, 2024
Copy link
Contributor

@olliesilvester olliesilvester left a comment

Choose a reason for hiding this comment

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

I don't think these extra statements guarantee that the task will have finished in time - if we change the sleep times then the tests still fail.

A good short term the solution would just be to increase await sleep(0.01) slightly, and to increase device.LOAD_TIMEOUT = 0.03 to account for this.

DiamondLightSource#943 load timeout and sleep time updated
@arikaran-13
Copy link
Contributor Author

I don't think these extra statements guarantee that the task will have finished in time - if we change the sleep times then the tests still fail.

A good short term the solution would just be to increase await sleep(0.01) slightly, and to increase device.LOAD_TIMEOUT = 0.03 to account for this.

I thought that since we are changing the state from PIN_MOUNTED to NO_PIN_MOUNTED and then back to PIN_MOUNTED, if we await the state change, the trigger() method will be called correctly. But if that doesn't fix the test, as a short-term solution, I have updated the timing of the LOAD_TIMEOUT

@codecov
Copy link

codecov bot commented Dec 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.17%. Comparing base (4b224c1) to head (7e10eb7).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #947   +/-   ##
=======================================
  Coverage   97.17%   97.17%           
=======================================
  Files         134      134           
  Lines        5631     5631           
=======================================
  Hits         5472     5472           
  Misses        159      159           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

noemifrisina and others added 5 commits December 7, 2024 11:48
* Add new position to backlight

* Fix path to zoom params file
* added mirror and slits

---------

Co-authored-by: Joseph Ware <53935796+DiamondJoseph@users.noreply.github.com>
format issue fixed
Copy link
Contributor

@olliesilvester olliesilvester left a comment

Choose a reason for hiding this comment

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

Thanks!

@olliesilvester olliesilvester merged commit 665b949 into DiamondLightSource:main Dec 10, 2024
@arikaran-13
Copy link
Contributor Author

Thanks!

Thanks you
This is my first open source contribution :)

huw-dls pushed a commit that referenced this pull request Dec 18, 2024
#943 Fix flakey bart robot unit test
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.

4 participants