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

Windows: fix unit tests & enable CI #633

Merged
merged 22 commits into from
Nov 1, 2024
Merged

Conversation

Tom-Willemsen
Copy link
Member

@Tom-Willemsen Tom-Willemsen commented Oct 31, 2024

Description

  • Fixes a number of unit tests which were either failing or inconsistent under windows: mostly race conditions, paths, numpy type differences, and asyncio scheduling differences.
  • Enable windows-latest as part of the testing CI matrix.

Motivation and Context

At isis we're running on windows, we'd like to be able to run the unit tests.

How Has This Been Tested?

See the CI runs.

@Tom-Willemsen Tom-Willemsen marked this pull request as draft October 31, 2024 11:54
@Tom-Willemsen Tom-Willemsen marked this pull request as ready for review October 31, 2024 17:28
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.

Thanks for the additions! A couple fo comments...

tests/epics/test_motor.py Show resolved Hide resolved
@@ -149,7 +149,7 @@ def flying_plan():
"uid": ANY,
"data_key": data_key_name,
"mimetype": "application/x-hdf5",
"uri": "file://localhost" + str(tmp_path / "test-panda.h5"),
"uri": "file://localhost/" + str(tmp_path / "test-panda.h5").lstrip("/"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does one of these windows paths actually look like? Not sure what the right file:// semantics are for drive letter etc. What does a web browser show if you point to file://something_on_c_drive on windows?

Copy link
Member Author

@Tom-Willemsen Tom-Willemsen Nov 1, 2024

Choose a reason for hiding this comment

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

Web browsers (at least firefox, chrome and edge) do correctly understand a path like file://C:\Instrument\Var\logs\ioc\ARINST-20241030.log. Forward slashes rather than backslashes would also work - but the drive letter part is what it wants as far as i understand it.

@Tom-Willemsen Tom-Willemsen merged commit ca56f74 into main Nov 1, 2024
20 checks passed
@Tom-Willemsen Tom-Willemsen deleted the windows_fix_unit_tests branch November 1, 2024 15:04
ZohebShaikh pushed a commit that referenced this pull request Nov 14, 2024
* Fix race condition in mock_signal_backend

* Fix numpy datatypes in test_mock_signal_backend

* Fix adaravis URI paths

* Fix path in test_kinetix

* Fix paths in simdetector tests

* Loosen timings against race condition in test_sim_detector

* Correct paths in test_advimba

* Fix paths in test_eiger

* Fix broken tests in test_signals

* Fix tange test_base_device tests on Windows

Missing process=True argument was causing all subsequent tests to fail on windows

* Fix race conditions in tango tests

* Use tango FQTRL to placate windows unit tests

* Correct paths in fastcs tests

* Correct path logic for windows compatibility

* Slacken timings for race condition in test_sim_motor

* Slacken race condition timings in test_motor

* Enable windows CI builds

* Try allowing asyncio time to clean up tasks

* Object IDs on windows are uppercase

* Try wait_for_wakeups
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.

2 participants