Bug 28334

The test library isn't really a proper library, but if Vala really can't deal with code that isn't at least pretending to be a library, and you need the test pseudo-library for the Vala tests, I suppose we could re-namespace it.

However, this is far too large for a single patch and I'd much prefer it in sensible-sized patches: for instance, one to rename each GObject class and make the tests pass again, and one for each utility function or group of utility functions.

> tp_tests_clear_object

Unless I'm missing something, this macro is useless for bindings, so renaming it is meaningless?

(I'm tempted to add it to the public util.h as tp_clear_object, though; we do this sort of boilerplate often enough that it seems potentially useful. CPython has Py_CLEAR for this purpose.)

> tp_tests_assert_no_error

If you're going to replace this with anything, please replace calls to it with g_assert_no_error() and delete the definition. I didn't do this systematically yet because it would be a relatively intrusive patch (although, less intrusive than this one).

> test_connection_run_until_dbus_queue_processed

If you're going to replace this with anything, please replace it with test_proxy_run_until_dbus_queue_processed and delete the definition. I didn't do this systematically yet because it would be a relatively intrusive patch.

> test_connection_run_until_ready

If you're going to replace this with anything, please replace it with test_proxy_run_until_prepared (., [TP_CONNECTION_FEATURE_CONNECTED]) and delete the definition. I didn't do this systematically yet because it would be a relatively intrusive patch.

> test_connection_manager_run_until_ready

Likewise, but test_proxy_run_until_prepared (., NULL).

> test_connection_manager_run_until_readying_fails

Likewise, but tp_tests_proxy_run_until_prepared_or_failed (cm, NULL, &error) and ensure it's followed by either g_assert_error (error, domain, code) or g_assert (error != NULL).

(In reply to comment #1)
> > tp_tests_clear_object
> 
> Unless I'm missing something, this macro is useless for bindings, so renaming
> it is meaningless?
> 
> (I'm tempted to add it to the public util.h as tp_clear_object, though; we do
> this sort of boilerplate often enough that it seems potentially useful. CPython
> has Py_CLEAR for this purpose.)

Fixed in Bug #28345.

> > tp_tests_assert_no_error
> 
> If you're going to replace this with anything, please replace calls to it with
> g_assert_no_error() and delete the definition. I didn't do this systematically
> yet because it would be a relatively intrusive patch (although, less intrusive
> than this one).

This and the other similar things are fixed in Bug #28347.

(In reply to comment #1)
> The test library isn't really a proper library, but if Vala really can't deal
> with code that isn't at least pretending to be a library, and you need the test
> pseudo-library for the Vala tests, I suppose we could re-namespace it.

Yeah, as far as Vala's concerned, there are something like 15-20 namespaces in this library (including "" in at least once place), and I don't think it supports (> 1 namespace)/library (at least when you're binding it from C).

> However, this is far too large for a single patch and I'd much prefer it in
> sensible-sized patches: for instance, one to rename each GObject class and make
> the tests pass again, and one for each utility function or group of utility
> functions.

Sorry about that. I thought it was a lot smaller, but it was painfully obvious when I split it up.

Please see the new branch. It's rebased against the branch from bug #28347

Thanks, this looks much better. review+ with a couple of changes:

> -   * regression test that's going to be run under a temporary session bus,
> +   * regression tp_tests that's going to be run under a temporary session bus,

Over-zealous sed detected :-)

> -#endif
> +#endif /* #ifndef __TP_TESTS_LIB_UTIL_H__ */
> -#endif
> +#endif /* ifndef __TP_TESTS_CONTACTS_CONN_H__ */

If anything, I'd be inclined to *remove* this comment from the multi-inclusion header guards. It doesn't seem to me as though it adds value (it should be obvious that the last #endif in a header is for the multi-inclusion guard) and it's one more thing to (fail to) change when basing a new header file on an old one.

(In reply to comment #4)
> Thanks, this looks much better. review+ with a couple of changes:
> 
> > -   * regression test that's going to be run under a temporary session bus,
> > +   * regression tp_tests that's going to be run under a temporary session bus,
> 
> Over-zealous sed detected :-)

Because I suspect this branch will cause conflicts/test failures in most other pending telepathy-glib branches, I'd like to fast-track it in, so I've corrected this in branch smcv/tp-tests and am stealing this bug.

> > -#endif
> > +#endif /* #ifndef __TP_TESTS_LIB_UTIL_H__ */
> > -#endif
> > +#endif /* ifndef __TP_TESTS_CONTACTS_CONN_H__ */
> 
> If anything, I'd be inclined to *remove* this comment from the multi-inclusion
> header guards.

Not a merge blocker, just a comment for the future really, so I'm ignoring this.

Actually, I'll need to fix conflicts with some changes from Zdra that are already in master. In progress...

Try this. With my changes, this is r+ from me.

r+ from Guillaume, merged for 0.11.7.