Skip to content

Various small MultiSignalBlocker cleanups #598

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

Merged
merged 4 commits into from
May 8, 2025
Merged

Various small MultiSignalBlocker cleanups #598

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bc12803
wait_signal: Simplify utils
The-Compiler Mar 23, 2025
9be4e0f
Use zip()
The-Compiler Mar 24, 2025
730eed3
Clean up some comments
The-Compiler Mar 25, 2025
2b9b92a
Fix SignalsAndArgs.args type
The-Compiler Mar 25, 2025
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
46 changes: 16 additions & 30 deletions src/pytestqt/wait_signal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import functools
import dataclasses
from typing import Any

from pytestqt.exceptions import TimeoutError
from pytestqt.qt_compat import qt_api
Expand Down Expand Up @@ -257,12 +259,12 @@ def _get_timeout_error_message(self):
)


@dataclasses.dataclass
class SignalAndArgs:
def __init__(self, signal_name, args):
self.signal_name = signal_name
self.args = args
signal_name: str
args: list[Any]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
args: list[Any]
args: list[object]

In this case, we don't care about the type of the object, for this reason object is safer: it will accept anything (just like Any), but contrary to Any, the type checker will flag if we accidentally try to perform some operation on it (say call a method).

Copy link
Member Author

@The-Compiler The-Compiler Mar 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

While we don't, this is exposed to users of the plugin, and we don't know what they want to do with those arguments. If we are going to make those types public in the future (and that's my goal), then users would have to cast those to do anything on it in that case, no?

Copy link
Member

@nicoddemus nicoddemus Mar 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Ahh this is exposed? Then you are right, better to leave it as args: list[Any].

Not suggestion to this now, but I think a way to handle this correctly regarding types would would be to make this class Generic so users could type args according to their needs.

Btw should in this case args be tuple[Any, ...] then? Usually this is how *args is typed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, see https://pytest-qt.readthedocs.io/en/latest/signals.html#getting-emitted-signals-and-arguments:

To determine which of the expected signals were emitted during a wait() you can use blocker.all_signals_and_args which contains a list of wait_signal.SignalAndArgs objects, indicating the signals (and their arguments) in the order they were received.

I don't think this can be made generic, as the arguments will depend on the signal being recorded, and we don't know which signals (and how many in total) there will be in there.

Good catch regarding tuple[Any, ...], will adjust accordingly.


def _get_readable_signal_with_optional_args(self):
def __str__(self) -> str:
args = repr(self.args) if self.args else ""

# remove signal parameter signature, e.g. turn "some_signal(str,int)" to "some_signal", because we're adding
Expand All @@ -272,18 +274,9 @@ def _get_readable_signal_with_optional_args(self):

return signal_name + args

def __str__(self):
return self._get_readable_signal_with_optional_args()

def __eq__(self, other):
if isinstance(other, self.__class__):
return self.__dict__ == other.__dict__
else:
return False


# Returns e.g. "3rd" for 3, or "21st" for 21
def get_ordinal_str(n):
def get_ordinal_str(n: int) -> str:
"""Return e.g. "3rd" for 3, or "21st" for 21."""
return "%d%s" % (n, {1: "st", 2: "nd", 3: "rd"}.get(n if n < 20 else n % 10, "th"))


Expand All @@ -308,22 +301,17 @@ def __init__(self, timeout=5000, raising=True, check_params_cbs=None, order="non
super().__init__(timeout, raising=raising)
self._order = order
self._check_params_callbacks = check_params_cbs
self._signals_emitted = (
[]
) # list of booleans, indicates whether the signal was already emitted
self._signals_map = (
{}
) # maps from a unique Signal to a list of indices where to expect signal instance emits
self._signals = (
[]
) # list of all Signals (for compatibility with _AbstractSignalBlocker)
self._signals_emitted: list[bool] = [] # whether the signal was already emitted
# maps from a unique Signal to a list of indices where to expect signal instance emits
self._signals_map = {}
# list of all Signals (for compatibility with _AbstractSignalBlocker)
self._signals = []
self._slots = [] # list of slot functions
self._signal_expected_index = 0 # only used when forcing order
self._strict_order_violated = False
self._actual_signal_and_args_at_violation = None
self._signal_names = (
{}
) # maps from the unique Signal to the name of the signal (as string)
# maps from the unique Signal to the name of the signal (as string)
self._signal_names = {}
self.all_signals_and_args = [] # list of SignalAndArgs instances

def add_signals(self, signals):
Expand Down Expand Up @@ -570,9 +558,7 @@ def _get_signal_for_index(self, index):

def _cleanup(self):
super()._cleanup()
for i in range(len(self._signals)):
signal = self._signals[i]
slot = self._slots[i]
for signal, slot in zip(self._signals, self._slots):
_silent_disconnect(signal, slot)
del self._signals_emitted[:]
self._signals_map.clear()
Expand Down