Improve waitSignal/waitSignals TimeoutError messages and catch arguments

[MShekow]

Hi. I've started thinking about implementing these 2 features (as they are closely related). Originally @nicoddemus suggested that SignalBlocker could show the following message:

TimeoutError:  `text_received` emitted with ['WAIT_CLIENTS', 'PROCESSING'] 
but did not satisfy the `is_disconnected` callback.

I investigated a bit and it seems that it's not possible to get the actual name of the signal (here: text_received) within SignalBlocker. The reason is that Signal objects don't have a __name__ attribute. PyQt4/5 signals have an internal signal field which contains the name of the signal itself, prefixed with a '2' (don't ask me why, I'll ask in the pyqt mailing list), whereas PySide signals have no attributes at all, their str() representation is just something like <PySide.QtCore.SignalInstance object at 0x05D82880>.

I'm not sure how to continue now. For SignalBlocker it's not much of an issue, as the wait() call of the blocker expects just one signal anyway, and the code line number in the stack trace of a failed test will allow the test engineer to figure out which signal exactly failed.

However, I'd also like to add this feature to MultiSignalBlocker. Here it would be nice to have a human-readable form of all caught signals, including their parameters. From what we currently know, it wouldn't be possible to get the name of the emitted signals.

After a bit more digging, I found that there could be a solution, if we can get _AbstractSignalBlocker to inherit from QObject. Because then we can get the signal by getting the signal's sender and iterating over its attributes, as follows:

# the following code (Python 3) works when placed inside SignalBlocker._quit_loop_by_signal() and
# you manually change that _AbstractSignalBlocker inherits from QObject of some Qt framework
def get_signal_name():
    sender = self.sender()
    possible_signal_names = [possible_signal_name for possible_signal_name in sender.__dict__]
    if not possible_signal_names:
        # e.g. PyQt5 has an empty __dict__ for some reason...
        possible_signal_names = dir(self.sender())
    for possible_signal_name in possible_signal_names:
        possible_signal = getattr(sender, possible_signal_name, None)
        if possible_signal and str(self._signals[0]) == str(possible_signal):
            return possible_signal_name
    return "UNKNOWN SIGNAL"

Is there a way to have _AbstractSignalBlocker inherit from the configuration-specific QObject implementation?

Cheers!
Marius

[The-Compiler]

I've had various issues with segfaults in the past when using QObject.sender(), and it's documented to not work with a Qt.DirectConnection across different threads.

For that reason, I'd really like to avoid using it (or doing other fun magic) in pytest-qt, even if that means worse error messages...

[The-Compiler]

prefixed with a '2' (don't ask me why, I'll ask in the pyqt mailing list)

All I can say is that it's hardcoded in the PyQt source:

// Create a new signal instance.
qpycore_pyqtSignal *qpycore_pyqtSignal_New(const char *signature, bool *fatal)
{
    // Assume any error is fatal.
    if (fatal)
        *fatal = true;

    Chimera::Signature *parsed_signature = Chimera::parse(signature,
                "a signal argument");

    // [...]

    parsed_signature->signature.prepend('2');

[MShekow]

OK, so then, for MultiSignalBlocker there would not be any good way to generate an error message, as we cannot get the name of the signal consistently. This leaves several options:

1. Always have degenerate error messages, such as "the first N signals were emitted as expected, but then some unexpected signal was emitted"
2. Get the name when we can (i.e. for PyQt, where the signal has a signal attribute which does contain the signal's name, remove the "2" at the beginning, if found). A good error message would then be e.g. "emitted signals were signal1, signal2, signal3, expected signals were signal2, signal1, signal3". When no name can be determined, the degenerate error message is shown.
3. Allow the user to specify not a list of signals, but a list of tuples, where the tuple contains the signal and the (manually provided) name of the signal as string. If the signal names are provided this way, we can provide good error messages, otherwise just degenerate error messages.

I like the combination of 2+3 best, where a hint is printed (?) by pytest-qt if the user uses PySide signals without providing the signals' names. The hint could be something like "to get better error messages in TimeoutError, consider providing signal names in the signal parameter".

[The-Compiler]

So with 3), you'd check if the user supplied a list of signals or a list of iterables/tuples, and then adjust accordingly? That could work out nicely.

2. sounds good to me too - I think we can reasonably assume that signal attribute to be there and be some meaningful string with PyQt.

For PySide, maybe once PySide2 is actually something usable, we could do a feature request for something similar upstream.

As for printing, I guess this information should be part of the SignalTimeoutError exception message, so it's all in one place.

[MShekow]

Alright. I started working on that feature. See https://github.com/MShekow/pytest-qt/commit/3f1a8a3388c87f0d733b6bf2747b60a952b377c2
I'll be on yet another vacation and continue working on it mid-end next week.

[nicoddemus]

Fixed by #153