assertSignal

[The-Compiler]

I'd like to add a qtbot.assertSignal to pytest-qt

/cc @acogneau

Motivation

Something I need to do regularily is to make sure a given code snippet calls a signal, or doesn't - but I don't want to wait for the signal.

The current way to do that works, but is unpythonic:

spy = QSignalSpy(foo.signal)
foo.do_stuff()
assert len(spy) == 1
assert spy[1] == "signal_arg"

spy = QSignalSpy(foo.error)
foo.do_stuff()
assert not spy

API

After some thinking, I think a context manager with this signature would be best:

qtbot.assertSignal(signal, count=None) -> QSignalSpy

• signal: The signal it should listen to
• count: Either None (in which case it expects the signal to be emitted >= 1 times), or an int. By setting count to 0, it can be ensured the signal is not emitted.
• return value: A QSignalSpy which can be used to check the signal arguments.

Examples

with qtbot.assertSignal(foo.signal, count=1) as spy:
    foo.do_stuff()
assert spy[1] == "signal_arg"

with qtbot.assertSignal(foo.error, count=0):
    foo.do_stuff()

Alternatives

I came up with some other ideas while thinking about this, but I think all of them are worse:

• qtbot.assertSignal(signal, emitted=True): Doesn't give you the possibility to check a signal was emitted exactly once, and isn't better in any way.
• qtbot.assertSignal(signal, calls=[("signal arg 1", "signal arg 2")]): Too complicated, and can be done easier by checking the QSignalSpy
• qtbot.assertNotEmitted(signal) - less functionality, and I think passing count=0 is okay if you want that (that was the usecase I had in mind originally)
• Shoehorning this function into waitSignal - it's complex enough already, and qtbot.waitSignal(signal, do_not_actually_wait=True) just sounds wrong 😉
• Adding a multi-signal variant (AND): Simply use with qtbot.assertSignal(foo), qtbot.assertSignal(bar): instead - that isn't an issue because it doesn't wait for the signal.
• Adding a multi-signal variant (OR): I don't really see the usecase for it.

[ghost]

For the API, wouldn't it be better to return a list of lists? The outer list containing count inner lists with each inner list containing the arguments of the emitted signal.

[The-Compiler]

That's basically how QSignalSpy already behaves (with PyQt5 at least) since it inherits QList<QList<QVariant> > - but yeah, for a more natural API (especially with PySide), something like this would probably be a good idea:

emissions = []
for elem in spy:
    args = [extract_from_variant(arg) for arg in elem]
    emissions.append(args)

(where extract_from_variant is taken from pytestqt/qt_compat.py in #63)

[nicoddemus]

Hi guys!

but I don't want to wait for the signal.

Hmm since signals are asynchronous by nature, not waiting for the signal seems to be error prone. For example, on linux widget events are asynchronous due to the client/server nature of the X window design. Triggering a signal sometimes doesn't mean it will be processed right away, depending on the signal or the system.

If I understand correctly, this is very similar to what is possible already with waitSignal, except that currently waitSignal doesn't provide any means to check which parameters or how many signals were emitted. Perhaps if we tweak waitSignal a bit we can accommodate the extra features that you propose?

with qtbot.waitSignal(signal, count=2) as blocker:
    foo.do_stuff()
assert blocker.args[0] == ("first signal args",)
assert blocker.args[1] == ("second signal args",)

[The-Compiler]

That might indeed be better for the "make sure a signal was received" usecase - that's basically #64 plus the count addition, then.

---

The more important thing, IMHO, (and my original usecase) is a way to make sure a signal is not emitted. Consider something like this (from the qutebrowser testsuite):

def test_get_version_success(qtbot):
    http_stub = HTTPGetStub(success=True)
    client = autoupdate.PyPIVersionClient(client=http_stub)

    # Use a spy to inspect the signal
    error_spy = QSignalSpy(client.error)

    with qtbot.waitSignal(client.success, raising=True):
        client.get_version('test')

    assert len(error_spy) == 0
    assert http_stub.url == QUrl('https://pypi.python.org/pypi/test/json')

or this:

def test_set_cookies_never_accept(config_stub):
    """Test setCookiesFromUrl when cookies are not accepted."""
    config_stub.data = CONFIG_NEVER_COOKIES
    ram_jar = cookies.RAMCookieJar()
    changed_signal_spy = QSignalSpy(ram_jar.changed)

    assert not ram_jar.setCookiesFromUrl('test', 'test')
    assert not changed_signal_spy

here I know I'd get the signal during setCookiesFromUrl if the config was read wrong, and I want to make sure I didn't get it.

So maybe it should be something like this instead, like my original thought:

with qtbot.assertNotEmitted(ram_jar.changed):
    assert not ram_jar.setCookiesFromUrl('test', 'test')

[nicoddemus]

Hmm I see, thanks. 😄

How would the assertNotEmitted signature look like?

def assertNotEmitted(self, signal, max_wait=1.0):
    """
    Asserts that the given signal was not emitted within the given ``max_wait`` seconds has
    elapsed.
    """

?

(BTW, thinking about it a little more, I'm not sure max_wait is necessary for most use cases, we might decide to ditch it)

Applying it to your examples:

def test_get_version_success(qtbot):
    http_stub = HTTPGetStub(success=True)
    client = autoupdate.PyPIVersionClient(client=http_stub)
    with qtbot.waitSignal(client.success, raising=True), qtbot.assertNotEmitted(client.error):
        client.get_version('test')
    assert http_stub.url == QUrl('https://pypi.python.org/pypi/test/json')

def test_set_cookies_never_accept(config_stub, qtbot):
    """Test setCookiesFromUrl when cookies are not accepted."""
    config_stub.data = CONFIG_NEVER_COOKIES
    ram_jar = cookies.RAMCookieJar()
    with qtbot.assertNotEmitted(ram_jar.changed):
        assert not ram_jar.setCookiesFromUrl('test', 'test')

The first example seems OK, but to be honest I'm not sure if the second example is much of an improvement.

What do you think?

[The-Compiler]

Sorry for not getting back to this earlier!

I think the second example is an improvement as well, I like to see things indented in context managers 😉

I'll probably work on this soon(tm) too.