Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-36475

Improve mockability of DBus interfaces

    XMLWordPrintable

Details

    • Bug
    • Resolution: Unresolved
    • P3: Somewhat important
    • None
    • 5.2.0
    • D-Bus
    • None

    Description

      I use to write fake DBus interfaces to be used in unit tests; these have always been working fine until Qt 5.2. Now I'm getting errors such as

      task-0: QFATAL : SignOnUiTest::testRequestWithIndicator() ASSERT: "reply.d_ptr->reply || reply.d_ptr->localMessage" in file qdbusmessage.cpp, line 457
      task-0: FAIL!  : SignOnUiTest::testRequestWithIndicator() Received a fatal error.
      

      After an inspection of the Qt source code, however, it appears that nothing relevant has been changed in QtDBus itself; the failures are due to the fact that this Qt version (which is being tested in Ubuntu) is built with Q_ASSERT on, while in the previous versions it was a no-op.

      So, it turns out that I've been using unsupported code paths all the time, but these code paths are actually working (if it weren't for the Q_ASSERTs) and extremely useful for mocking interfaces. See for example:

      QDBusPendingReply<> doSomething(int param)
      {
          QDBusMessage message; // invalid message, needed just to create the reply
          QDBusPendingCall reply =
              QDBusPendingCall::fromCompletedCall(message.createReply());
          return static_cast<QDBusPendingReply<> >(reply);
      }
      

      This code mocks an asynchronous DBus method which returns an empty reply; this works fine if Q_ASSERTs are disabled, but if they aren't the call to message.createReply() will fail with the errors above.

      I would propose either to remove the relevant Q_ASSERTs from QtDBus or to add more APIs to explicitly handle these cases (for instance, a static QDBusMessage::createReply()). The latter could seem cleaner, but IMHO such APIs would be of very limited use (probably only for unit testing) and just bring confusion when someone is studying the documentation. So my preference would go for removing those Q_ASSERTs which are harmless, and add unit tests to cover those code paths.

      Please let me know what's your preferred strategy; I can probably contribute the implementation of the bug fix, then.

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            thiago Thiago Macieira
            mardy Alberto Mardegan
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:

              Gerrit Reviews

                There are no open Gerrit changes