[Bug 38206] Tp::StreamTubeChannel (and subclasses) don't have unit tests

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=38206

--- Comment #6 from Andre Moreira Magalhaes <andrunko at gmail.com> 2011-06-28 06:53:57 PDT ---
(In reply to comment #3) 
> > > 
> > > You should also check, not only for UNIX sockets, but for TCP sockets too, that
> > > the PendingStreamTubeConnection and the tube channel both return a valid
> > > address which one can connect to.
> > In progress.
> > 
> > The updated branch now also enables tests for Port access control.
> 
> +      guint16 port, remote_port;
> 
> -      addr = g_socket_connection_get_remote_address (connection, &error);
> +      dbus_g_type_struct_get (self->priv->access_control_param,
> +              0, &host,
> +              1, &port,
> +              G_MAXUINT);
> 
> I think this might not work when this lands in tp-glib, or if it does, only 
> with little-endian architectures, because AFAIK tp-glib will always just send a
> 32-bit uint in that variant (incorrectly?). At least Gabble extracts the port
> to a guint (a 32-bit uint), not a guint16.
> 
Will fix it

> > 
> > The downside is that I was unable to test Tcp+Port using Qt sockets directly. I
> > used G*Socket* instead, see the test for more details.
> 
> +                // QAbstractSocket::setSocketDescriptor does not work either,
> so using glib sockets
> 
> Really? A quite basic function documented since Qt 4.1 doesn't work?
> 
> As for letting Qt create the socket but binding it, did you try the suggested
> workaround from QTBUG-121, which I guess in code could be something as simple
> as
> 
> sockaddr_in sin;
> socklen_t sinlen = sizeof(sin);
> std::memset(&sin, 0, sinlen);
> 
> sin.sin_family = AF_INET;
> sin.sin_addr.s_addr = INADDR_LOOPBACK;
> sin.sin_port = 0; // 0 is random free port, or change to htons(some desired
> one) if you rather want that
> 
> bind(sock->getSocketDescriptor(), reinterpret_cast<sockaddr *>(&sin), sinlen);
> 
> getsockname(sock->getSocketDescriptor(), reinterpret_cast<sockaddr *>(&sin),
> &sinlen);
> 
> At that point, sin should contain your socket's local address and you can
> proceed with giving it to the CM and connecting. You can translate the address
> to a C-string with inet_ntop and the port to a native integer value with ntohs.
> 
> Of course, it would be nicer if Qt had the API for this - as this needs
> different includes on different platforms etc., but we'd need to wait for at
> least Qt 4.8 for that.
> 
> In any case, I'd very much prefer if our GLib could be restricted to the test
> services as much as possible. If it can't be done, it can't be done, though.
I tried the other way around, tried creating a socket using libc and glib. bind
it and pass it to QTcpSocket using setSocketDescriptor. After that calling
connectToServer fails horribly, but I will try what you suggested to see if it
works.

> Some other observations:
> 
>     QVERIFY(connect(chan->acceptTubeAsUnixSocket(),
>                 SIGNAL(finished(Tp::PendingOperation *)),
>                 SLOT(expectSuccessfulCall(Tp::PendingOperation *))));
>     QCOMPARE(mLoop->exec(), 1);
> 
> You should use expectFailure in these cases, so you don't get a warning.
> 
> Oh, one thing you should test for:
> 
> 1) begin accepting a tube
> 2) immediately, somehow invalidate the channel (either through client-side
> backdoor API or some service backdoor API)
> 3) verify that the accept operation FAILED instead of hanging forever
> 
> And as a variation, make it adjustable when the service goes to StateOpen, and
> only do that a few mainloop iterations after returning from the Accept call.
> Then in between invalidate the channel. I believe that would hang the current
> PendingStreamTubeConnection implementation, which needs to start checking and
> waiting for channel invalidation when waiting for the state change to Open as
> well. The currently WIP dbus tube branch has the same issue actually.
I will add this too.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.