API break suggestions for 4.0

[The-Compiler]

As mentioned in #302 (comment) I went through the docs/code and thought about possible API breaks which would make sense for 4.0. Here are a couple of ideas:

• 1. One thing that always irked me a bit is that we have two naming conventions (add_widget vs. addWidget). Is it really a good idea to support both? At least in my project, it only lead to using both inconsistently over the years without noticing. Should we get rid of one of those? On the other hand, that's probably a lot of churn for projects using pytest-qt, so I'm not sure if it's worth it. Maybe as an alternative, we could make it possible to configure the desired API style in pytest.ini so that the other style would raise an exception (that could be done after 4.0)?
• 2. Right now the user needs to decide between waitActive, waitExposed and waitForWindowShown (which might be a hurdle if using the same tests for two different backends). Should we have a wrapper which decides what to use instead?
• 3. Shouldn't waitForWindowShown have a timeout as well? (can be done after 4.0)
• 4. Does anyone really use qtbot.stopForInteraction() instead of qtbot.stop()? A GitHub search reveals 60 results, vs. 122 results for .stop(), though I guess it's mostly used interactively for debugging without being added to the code. Should we remove the long name?
• 5. Should we raise the default timeout for waitSignal() to 5000, similar to QSignalSpy.wait() in Qt and the other default timeouts in pytest-qt?
• 6. waitSignal and waitSignals can take None instead of a signal, in which case they just wait until the timeout occurred. But we already have qtbot.wait() for that, so is there a reason to support it?
• 7. Can we get rid of the SignalTimeoutError compatibility alias for TimeoutError?
• 8. The docs for pytestqt.logging.Record says that context is only available for Qt 5, that can probably be removed now (can be done after 4.0)
• 9. (new, see below) Get rid of qt_wait_signal_raising
• 10. (new, see below) Get rid of aliases in plugin.py

[nicoddemus]

Thanks a lot for working on this list Florian!

1. Indeed, that's something I regret. Today, I would have rather started with PEP-8, but that not being the case, adding an alternative API is also a regret. I agree it is probably a lot of churn. Not sure about the option, it is very weird (never seen an option like this).

2. Good idea, if that is possible. 👍

3. Sounds good.

4. Agreed, once users learn the short form, there's really no reason to use the long one. Also this is a simple find replace on a very uncommon name, so 👍 from me.

5. Definitely!

6. Sounds good too.

7. 👍

8. 👍

Given all the breaking changes we are planning for 4.0, we might write a short "upgrade guide" (for example, replace all .stopForInteraction calls by .stop, check timeouts for waitForWindowShown, etc).

We might decide to put those in the CHANGELOG though: we announce the change, and also what to do to upgrade.

[The-Compiler]

Another one: We should probably remove qt_wait_signal_raising (legacy alias for qt_default_raising).

[The-Compiler]

And yet another one, in plugin.py:

# classes/functions imported here just for backward compatibility before we
# split the implementation of this file in several modules
assert QtBot
assert SignalBlocker
assert MultiSignalBlocker
assert Record
assert capture_exceptions
assert format_captured_exceptions

I suppose those can go away?

[nicoddemus]

Another one: We should probably remove qt_wait_signal_raising (legacy alias for qt_default_raising).

Agree. 👍

I suppose those can go away?

Not sure, this will force whoever is importing this to import from the actual module. Perhaps it makes sense to still have every name being imported from pytestqt.plugin instead of where they are declared?

[The-Compiler]

Not sure, this will force whoever is importing this to import from the actual module. Perhaps it makes sense to still have every name being imported from pytestqt.plugin instead of where they are declared?

But the pytestqt.plugin module seems like a weird place for such convenience imports (if you ignore the historical reason why they're there) - I can see how this would make sense in __init__.py (so that people could import directly from the pytestqt package), but plugin.py for me says "this is the interface to pytest" and wouldn't be the place I'd look for classes to use "manually".

I did a quick GitHub code search (also tried grep.app and Debian Code search, with less results). It looks like this'd affect almost nobody:

• Spyder IDE imports QtBot to create a module-scoped qtbot fixture.
• Same for Spyder and QtBot in another place
• Spyder and QtBot again, unused import
• Phy uses QtBot to have it on the test class instead of using a fixture
• qutebrowser uses SignalBlocker for some custom API needing to wait for signals
• Various places using pytestqt.plugin.QtBot in Sphinx docstrings

Compare that to a search for pytestqt.qtbot which yields 125 results, mostly from people using pytestqt.qtbot.QtBot in type annotations.

As for the split modules:

• Nobody uses pytestqt.wait_signal
• One project uses pytestqt.logging for some monkeypatching involving private APIs
• 15 or so use pytestqt.exceptions.TimeoutError
• One project uses pytestqt.exceptions.capture_exceptions

So it looks like direct usage of the API (outside of qtbot) is incredibly rare, and for those who use it, they already use split modules. The only exception maybe is pytestqt.plugin.QtBot which is used in a couple of places, but I'd still prefer to have one genuine place for it, even if Spyder and Phy will need to adjust.

[nicoddemus]

Thanks for the research! Agreed then, let's ditch those convenience imports. 👍