Skip to content

Conversation

@Tom-Willemsen
Copy link
Contributor

This PR contains some minimal changes to allow a FastCS driver to start on Windows.

  • loop.add_signal_handler() is not available on windows. On windows, signals are always delivered to the main thread of an application. Ctrl-c does correctly stop a FastCS driver in non-interactive mode on windows without this handler.
  • StdoutProxy didn't work on windows under pytest - I believe it is conflicting with pytest's stdout capturing. Just use sys.stdout for that case.

FastCS' own unit tests don't pass on Windows even with this change - that would require a larger PR.

@Tom-Willemsen Tom-Willemsen changed the title Windows: add guards for some methods that don't work/exist on win32 Windows: add guards for some methods that don't work/exist on windows Dec 19, 2025
@codecov
Copy link

codecov bot commented Dec 19, 2025

Codecov Report

❌ Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.67%. Comparing base (c86f760) to head (470cf55).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
src/fastcs/logging/_logging.py 57.14% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #295      +/-   ##
==========================================
- Coverage   90.82%   90.67%   -0.15%     
==========================================
  Files          70       70              
  Lines        2486     2532      +46     
==========================================
+ Hits         2258     2296      +38     
- Misses        228      236       +8     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Exception was also occuring in other non-console contexts, e.g. sphinx
@Tom-Willemsen Tom-Willemsen force-pushed the guard_methods_that_dont_work_on_windows branch from e4c48ae to ce40c51 Compare January 2, 2026 22:24
Copy link
Contributor

@GDYendell GDYendell left a comment

Choose a reason for hiding this comment

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

This looks good. Just a couple of minor changes.

@GDYendell GDYendell merged commit 301803a into DiamondLightSource:main Jan 12, 2026
9 of 11 checks passed
@GDYendell
Copy link
Contributor

Thanks @Tom-Willemsen !

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