Skip to content

Waiting for a signal with certain arguments #118

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.

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

Closed
The-Compiler opened this issue Dec 17, 2015 · 17 comments
Closed

Waiting for a signal with certain arguments #118

The-Compiler opened this issue Dec 17, 2015 · 17 comments
Assignees

Comments

@The-Compiler
Copy link
Member

I have some scenarios where I'd like to do something like:

with qtbot.waitSignal(got_line, args=['foo']):
    ...

And qtbot would wait until the signal is emitted with exactly those args.

I also have another case where I do some more sophisticated checking:

    spy = QSignalSpy(self.new_data)
    elapsed_timer = QElapsedTimer()
    elapsed_timer.start()

    while True:
        got_signal = spy.wait(timeout)
        if not got_signal or elapsed_timer.hasExpired(timeout):
            raise WaitForTimeout("Timed out after {}ms waiting for "
                                 "{!r}.".format(timeout, kwargs))

        for args in spy:
            assert len(args) == 1
            line = args[0]

            matches = []

            for key, expected in kwargs.items():
                value = getattr(line, key)
                matches.append(self._match_data(value, expected))

            if all(matches):
                return line

I'm thinking if it makes sense or not to do something like:

def check_args(*args):
    return args == ['foo', 2342]

with qtbot.waitSignal(got_line, callback=check_args):
    ...

Then qtbot will only stop waiting when the callback returned something true.

Maybe args isn't even necessary then, as you could do:

with qtbot.waitSignal(got_line, callback=lambda *args: args == ['foo']):
    ...

What do you think? Neither? Only args? args and callback? Both?

(edited to rename arg_callback to callback as it could be used to check other stuff as well.

@nicoddemus
Copy link
Member

I would add only callback as it is more generic and would cover most use cases...

@The-Compiler The-Compiler self-assigned this Jun 25, 2016
@MShekow
Copy link

MShekow commented Jun 27, 2016

Hi. I've had a shot at it, because I needed the ability to check for parameters (of a single signal or even multiple signals), and I also needed the ability to check for a correct order of emitted signals, and also the ability to check that the same signal is emitted in a specific order (each emit with a different parameter).

Here are my method signatures:
Single signal:
waitSignal(self, signal=None, timeout=1000, callback=None, raising=None):
As suggested above. The docblock says:
:param Callable callback: Optional callable(*args) that returns True if the parameters are as expected, or False otherwise.

For multiple signals:
def waitSignals(self, signals=None, timeout=1000, force_order = "none", callbacks = None, raising=None):
signals: Each element of the list can be a :class:Signal or a 2-value tuple where the first value the :class:Signal and the 2nd value is an integer that indicates the number of times this signal expected.
force_order:
"none": no order is enforced
"strict": the signals have to be emitted in the provided order (and no intermediate signals may be emitted)
"simple": like "strict", but signals may be emitted inbetween the provided ones, e.g. suppose
signals=[a, b, c] and the tested object emits the signals a, a, b, a, c, the test will pass with "simple"
but not with "strict"
callbacks: optional list of Callable functions that evaluate the parameters of the signal. Each element of the list
can be a callable, None or a 2-value tuple where the first value is the callable and the 2nd value is an
integer that indicates the number of times this callable is going to be called (matching with the signals)
E.g. [some_func, None, (some_other_func, 2)]. Make sure that the number of callables matches the
number of signals.

If you're interested in the implementation, check out https://gist.github.com/MShekow/0481d22224395d3f86656c3079bf2102 and https://gist.github.com/MShekow/44612f31b983727ff8978521a455b256
I used version 1.11.0 (from pip) as base.

Best regards!
Marius

@nicoddemus
Copy link
Member

Thanks @MShekow, @The-Compiler plans to work on this as well for the upcoming 2.0 release.

@The-Compiler
Copy link
Member Author

@MShekow could you maybe add some tests and open a pull request? I already have quite much on my plate currently, so I'd be more than happy to review and merge your code 😄

@MShekow
Copy link

MShekow commented Jul 3, 2016

@The-Compiler Since I've done that at my job, I'll try to get my boss to allow me to work on that (creating tests and a PR) :)
UPDATE: I got the approval, I'm going to send the PR next week.

@MShekow
Copy link

MShekow commented Jul 12, 2016

Quick update: I'm on it, should be ready on Thursday.

@nicoddemus
Copy link
Member

OK, thanks!

On Tue, Jul 12, 2016 at 12:31 PM MShekow [email protected] wrote:

Quick update: I'm on it, should be ready on Thursday.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#118 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABCO_NT8oxJy4wVCbJxoNlzU05iMkoSuks5qU7MxgaJpZM4G3HqF
.

@MShekow
Copy link

MShekow commented Jul 12, 2016

By the way, there is one problem that I'm facing and I'm not sure how to proceed:
When the user does something like this (because he wants to test that signaller emits the signal1 twice):
with qtbot.waitSignals([signaller.signal1, signaller.signal1], ...): ....

then this is identical to:

x = signaller.signal1
y = signaller.signal1
with qtbot.waitSignals([x, y], ...): ....

In such a case, the code within waitSignals is unable to detect that x and y are actually the same signal (because x==y doesn't hold, at least not when I tested it with PyQt5 signals).

This poses a problem, because the code in MultiSignalBlocker will connect each signal in signals to its slot (self._signal_emitted), effectively connecting the same signal multiple times without even realizing it. This breaks the evaluation of the signals (and their order). One idea for a solution, so far, was to ask the user to provide the signals with a counter, such as:
x = signaller.signal1
y = signaller.signal2
with qtbot.waitSignals([(x, 3), y], ...)

which indicates to self.wait() to expect signaller.signal1 3 times in a row, followed by signaller.signal2. I.e. I'm asking the user to provide only one instance of each signal in the signals parameter, and use other parameters (like the counter integer) to indicate the positions when to expect this signal.

However, suppose the user wants to test that signaller emits the following sequence:
signal1, signal2, signal2, signal1, then my solution no longer works. I'm generally wondering how the user should be able to specify the order of signals when there are discontinuities. One proposal could be:
x = signaller.signal1
y = signaller.signal2
signals = {x: [1, 4], y: [2,3]} # map from signal to the positions where to expect it
with qtbot.waitSignals(signals)

Is this solution something you would accept? Do you have other suggestions?
Cheers!
Marius

@The-Compiler
Copy link
Member Author

The-Compiler commented Jul 12, 2016

Hmm, that's weird (PyQt5):

>>> from PyQt5.QtCore import QObject, pyqtSignal
>>> class Signaller(QObject):
...     sig = pyqtSignal()
... 
>>> s = Signaller()
>>> s.sig
<bound PYQT_SIGNAL sig of Signaller object at 0x7fbde9c9e798>
>>> s.sig
<bound PYQT_SIGNAL sig of Signaller object at 0x7fbde9c9e798>
>>> s.sig is s.sig
False
>>> s.sig == s.sig
False

I'm not sure how it's possible for the is check to be False here - but comparing their id() seems to work:

>>> id(s.sig) == id(s.sig)
True

@MShekow
Copy link

MShekow commented Jul 13, 2016

Hi. Great find using id(). Unfortunately, this won't work when assigning a signal to a variable, i.e.

>>> s = Signaller()
>>> x = s.sig
>>> y = s.sig
>>> x is y
False
>>> x == y
False
>>> id(x) == id(y)
False

However:
str(x) == str(y)
yields True, but I suppose that's kind of a hacky solution.

I see two possibilities:

  1. use id() and hope that the user doesn't assign signals to variables (should be rather uncommon anyway) - in this case I still have to check that this works with PyQt4 and PySide (and if it doesn't, call type(signal) to determine the signal's identity depending on the Qt framework).
  2. use str(signal) or find yet another possibility to discern the signals. I dislike this solution a bit, as the framework guys might decide to overwrite signal.__str__() at any time.

@nicoddemus
Copy link
Member

nicoddemus commented Jul 13, 2016

>>> s.sig
<bound PYQT_SIGNAL sig of Signaller object at 0x7fbde9c9e798>

I'm not sure how it's possible for the is check to be False here

I suspect s.sig is always returning a new object; the address we see above actually belongs to Signaller (the s on s.sig).

>>> id(s.sig) == id(s.sig)
True

Not sure how to explain this, if my above statement was true.

@MShekow
Copy link

MShekow commented Jul 13, 2016

I've contacted the PyQt mailing list. Maybe they (Phil Thompson) can shed some light on this issue.

@MShekow
Copy link

MShekow commented Jul 13, 2016

I got some feedback:
Bound signals (like bound methods) are created on the fly. In the example above two different objects are created which have different values of id().

The fact that the id(s.sig) == id(s.sig) yields True is explained as follows: "...that one of the bound signals is being garbage collected as soon as id() returns. The second bound signal is using the same piece of memory and therefore has the same id()". There you have your explanation, @nicoddemus :)

Phil suggested that the solution that uses str(s.sig) for comparison will work best at the moment. He suggested that a better solution would be if bound signals implemented __eq__ (but he didn't indicate whether he would implement it, and it doesn't matter, as almost all versions out there right now don't have __eq__ implemented anyway).

So, unless you have a better suggestion, I'll go with the str() approach, but I'll check that this actually works across PyQt4/5 and PySide first.

@nicoddemus
Copy link
Member

Sounds like str(s.sig) is the way to go then.

Thanks for all the research and interest on this! 😁

@The-Compiler
Copy link
Member Author

Though we still need to check how things look on PySide and PyQt4 😉

@MShekow
Copy link

MShekow commented Jul 14, 2016

I did check it, str() works fine with all Qt frameworks. I also included a test for that.

Interestingly, tests fail on Py27, because list.clear() doesn't exist apparantly. But then I wonder how the tests could have worked before, because in MultiSignalBlocker._cleanup(self) there was already a self._slots.clear() statement (and test should have failed at that point in the past). Should I replace it with del list[:] ?

@nicoddemus
Copy link
Member

there was already a self._slots.clear() statement

I believe self._slots is a dict.

Should I replace it with del list[:] ?

I think so.

Thanks again for working on this! 😁

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

No branches or pull requests

3 participants