Skip to content

Windows: add guards for some methods that don't work/exist on windows #295

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.

Sign up for GitHub

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

Open
Tom-Willemsen wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Windows: add guards for some methods that don't work/exist on windows #295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/fastcs/control_system.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import asyncio
import os
import signal
from collections.abc import Coroutine, Sequence
from functools import partial
Expand Down Expand Up @@ -45,8 +46,9 @@ def __init__(
def run(self, interactive: bool = True):
serve = asyncio.ensure_future(self.serve(interactive=interactive))

self._loop.add_signal_handler(signal.SIGINT, serve.cancel)
self._loop.add_signal_handler(signal.SIGTERM, serve.cancel)
if os.name != "nt":
self._loop.add_signal_handler(signal.SIGINT, serve.cancel)
self._loop.add_signal_handler(signal.SIGTERM, serve.cancel)
self._loop.run_until_complete(serve)

async def _run_initial_coros(self):
Expand Down
4 changes: 3 additions & 1 deletion src/fastcs/logging/_logging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import getpass
import os
import sys
from enum import StrEnum
from logging import LogRecord
from typing import TYPE_CHECKING, Any
Expand Down Expand Up @@ -30,8 +31,9 @@ def _configure_logger(
graylog_env_fields: GraylogEnvFields | None = None,
):
logger.remove()
is_pytest_on_windows = "PYTEST_VERSION" in os.environ and os.name == "nt"
logger.add(
sink=StdoutProxy(raw=True), # type: ignore
sink=sys.stdout if is_pytest_on_windows else StdoutProxy(raw=True), # type: ignore
Copy link
Contributor

@GDYendell GDYendell Dec 19, 2025

Choose a reason for hiding this comment

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

Could this be moved to a fixture in the tests to keep the production code unchanged? I think something like this would do it

@pytest.fixture(scope="session", autouse=True)
def patch_stdoutproxy_on_windows(mocker: MockerFixture):
    if os.name == "nt":
        import fastcs.logging._logging as logging_mod

        mocker.patch.object(
            logging_mod,
            "StdoutProxy",
            return_value=sys.stdout,
        )

Copy link
Contributor Author

@Tom-Willemsen Tom-Willemsen Dec 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think the issues with this are:

  • Currently this code path is hit when tests are being collected (i.e. before any fixtures get to run) as it's done at import time. An adapted version does work in a pytest_sessionstart hook, but that seems a bit messy to me.
  • This would be needed in every downstream repo that tests on windows - I guess we could export a function that does this from FastCS for use by downstream tests in their pytest_sessionstart hook?

Let me know what you prefer?

colorize=True,
format=format_record,
level=level or "INFO",
Expand Down