Skip to content

Commit d18fb9c

Browse files
committed
Allow and test all kwargs on all platforms.
The mss_impl fixture would add an implicit display= argument, regardless of platform. The code at that time would ignore it, but we should be (and in the previous commit, were) more strict. Change mss_impl to only use display= if appropriate, so we can be more strict in the future. In 10.1, these were allowed at all times, and ignored if the platform didn't use them. Emulate this behavior in mss.MSS (and mss.mss), with DeprecationWarnings, and test.
1 parent 0d84b93 commit d18fb9c

File tree

3 files changed

+47
-14
lines changed

3 files changed

+47
-14
lines changed

src/mss/base.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from __future__ import annotations
55

66
import platform
7+
import warnings
78
from abc import ABC, abstractmethod
89
from datetime import datetime
910
from threading import Lock
@@ -173,6 +174,22 @@ def __init__(
173174
compression_level: int = 6,
174175
**kwargs: Any,
175176
) -> None:
177+
# TODO(jholveck): #493 Accept platform-specific kwargs on all platforms for migration ease. Foreign kwargs
178+
# are silently stripped with a warning.
179+
platform_only: dict[str, str] = {
180+
"display": "linux",
181+
"max_displays": "darwin",
182+
}
183+
os_ = platform.system().lower()
184+
for kwarg_name, target_platform in platform_only.items():
185+
if kwarg_name in kwargs and os_ != target_platform:
186+
kwargs.pop(kwarg_name)
187+
warnings.warn(
188+
f"{kwarg_name} is only used on {target_platform}. This will be an error in the future.",
189+
DeprecationWarning,
190+
stacklevel=2,
191+
)
192+
176193
self._impl: MSSImplementation = _choose_impl(
177194
backend=backend,
178195
**kwargs,

src/tests/conftest.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from hashlib import sha256
88
from pathlib import Path
99
from platform import system
10+
from typing import Any
1011
from zipfile import ZipFile
1112

1213
import pytest
@@ -78,7 +79,14 @@ def backend(request: pytest.FixtureRequest) -> str:
7879
def mss_impl(backend: str) -> Callable[..., MSS]:
7980
# We can't just use partial here, since it will read $DISPLAY at the wrong time. This can cause problems,
8081
# depending on just how the fixtures get run.
81-
return lambda *args, **kwargs: MSS(*args, display=os.getenv("DISPLAY"), backend=backend, **kwargs)
82+
def impl(*args: Any, **kwargs: Any) -> MSS:
83+
# I'm not really sure if adding an explicit display is needed anymore. It was in a lot of existing code that
84+
# mss_impl replaced, but it should now be the default at this point. I'll have to investigate.
85+
if system() == "Linux":
86+
kwargs = {"display": os.getenv("DISPLAY")} | kwargs
87+
return MSS(*args, backend=backend, **kwargs)
88+
89+
return impl
8290

8391

8492
@pytest.fixture(autouse=True, scope="session")

src/tests/test_compat_10_1.py

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -144,21 +144,29 @@ def test_factory_and_platform_constructor_are_compatible_types() -> None:
144144

145145

146146
def test_deprecated_factory_accepts_documented_kwargs() -> None:
147-
os_ = platform.system().lower()
148-
kwargs: dict[str, object] = {"compression_level": 1, "with_cursor": True}
147+
"""Verify that kwargs are accepted, even if not relevant.
148+
149+
All 10.1-documented kwargs were accepted on every platform, even
150+
if only meaningful on one. Verify that still works via the
151+
deprecated factory.
152+
"""
153+
kwargs = {
154+
"compression_level": 1,
155+
"with_cursor": True,
156+
"max_displays": 1,
157+
"display": getenv("DISPLAY"), # None on non-Linux
158+
}
159+
160+
with pytest.warns(DeprecationWarning) as caught: # noqa: PT030 (we test the contents below)
161+
context = mss.mss(**kwargs)
149162

150-
if os_ == "linux":
151-
display = getenv("DISPLAY")
152-
assert display
153-
kwargs["display"] = display
154-
elif os_ == "darwin":
155-
kwargs["max_displays"] = 1
156-
elif os_ != "windows":
157-
msg = f"Unsupported platform for compatibility test: {os_!r}"
158-
raise AssertionError(msg)
163+
expected_messages = {"mss.mss is deprecated", "is only used on"}
159164

160-
with pytest.warns(DeprecationWarning, match=r"^mss\.mss is deprecated"):
161-
context = mss.mss(**kwargs)
165+
assert any("mss.mss is deprecated" in str(w.message) for w in caught)
166+
assert any("is only used on" in str(w.message) for w in caught)
167+
assert all(any(expected in str(w.message) for expected in expected_messages) for w in caught), (
168+
f"Unexpected warnings: {[str(w.message) for w in caught]}"
169+
)
162170

163171
with context as sct:
164172
assert isinstance(sct, MSSBase)

0 commit comments

Comments
 (0)