After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769486 - Deprecate g_test_expect_message() with structured logging
Deprecate g_test_expect_message() with structured logging
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on: 769485
Blocks: 682794
 
 
Reported: 2016-08-03 20:59 UTC by Philip Withnall
Modified: 2016-08-07 07:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Revert "Add a test for expected messages with structured logging" (2.86 KB, patch)
2016-08-03 20:59 UTC, Philip Withnall
committed Details | Review
Revert "Make g_test_expect_message work for structured logs" (5.38 KB, patch)
2016-08-03 20:59 UTC, Philip Withnall
committed Details | Review
gmessages: Document g_test_expect_message() doesn’t like structured logs (4.54 KB, patch)
2016-08-03 20:59 UTC, Philip Withnall
none Details | Review
gmessages: Document g_test_expect_message() doesn’t like structured logs (4.60 KB, patch)
2016-08-04 09:44 UTC, Philip Withnall
committed Details | Review

Description Philip Withnall 2016-08-03 20:59:35 UTC
Since GTK+ doesn’t need g_test_expect_message() for its tests any more (see bug #769485), we can decouple g_test_expect_message() from structured logging, so that it’s only used with the old-style logging API now.

Revert the two patches which added structured logging support to it, and add a third which documents this all. See that patch for details of the rationale and alternatives here (tl;dr: don’t test code by looking at its stderr; or, if you really have to, use a custom log writer function instead).
Comment 1 Philip Withnall 2016-08-03 20:59:39 UTC
Created attachment 332681 [details] [review]
Revert "Add a test for expected messages with structured logging"

This reverts commit bc40c7a05cbd0e7c9b97ce86cccb6a4b78c2e56a.

We are going to make g_test_expect_message() only work for the old
logging API.
Comment 2 Philip Withnall 2016-08-03 20:59:43 UTC
Created attachment 332682 [details] [review]
Revert "Make g_test_expect_message work for structured logs"

This reverts commit df02e8f47cac4d2c550491c9de633233218c7415.

We are going to make g_test_expect_message() only work for the old
logging API.
Comment 3 Philip Withnall 2016-08-03 20:59:48 UTC
Created attachment 332683 [details] [review]
gmessages: Document g_test_expect_message() doesn’t like structured logs

It’s effectively deprecated if G_LOG_USE_STRUCTURED is defined, or if
the structured logging API is used directly. See the documentation for
rationale.
Comment 4 Matthias Clasen 2016-08-03 22:23:13 UTC
Review of attachment 332681 [details] [review]:

ok, as discussed
Comment 5 Matthias Clasen 2016-08-03 22:23:45 UTC
Review of attachment 332682 [details] [review]:

ok, as discussed
Comment 6 Matthias Clasen 2016-08-03 22:23:46 UTC
Review of attachment 332682 [details] [review]:

ok, as discussed
Comment 7 Matthias Clasen 2016-08-03 22:23:58 UTC
Review of attachment 332682 [details] [review]:

ok, as discussed
Comment 8 Matthias Clasen 2016-08-03 22:43:10 UTC
Review of attachment 332683 [details] [review]:

Apart from those minor comments; looks fine

::: glib/gmessages.c
@@ +57,3 @@
  * g_log_structured(), so that all log messages end up in same destination.
+ * If `G_LOG_USE_STRUCTURED` is defined, g_test_expect_message() will become a
+ * no-op (see [Testing for Messages][testing-for-messages]).

Thats not entirely accurate; maybe say: If G_LOG_USE_STRUCTURED is defined, g_test_expect_message()
will become ineffective for the wrapper macros g_warning() and friends (see ...)

@@ +121,3 @@
+ * g_test_trap_assert_stderr().
+ *
+ * If it is really necessary to test the structured log messages outputted by a

emitted ?
Comment 9 Philip Withnall 2016-08-04 09:44:53 UTC
Created attachment 332708 [details] [review]
gmessages: Document g_test_expect_message() doesn’t like structured logs

It’s effectively deprecated if G_LOG_USE_STRUCTURED is defined, or if
the structured logging API is used directly. See the documentation for
rationale.
Comment 10 Philip Withnall 2016-08-07 07:09:55 UTC
Attachment 332681 [details] pushed as 2a5d9f8 - Revert "Add a test for expected messages with structured logging"
Attachment 332682 [details] pushed as 4e66036 - Revert "Make g_test_expect_message work for structured logs"
Attachment 332708 [details] pushed as 0e132b8 - gmessages: Document g_test_expect_message() doesn’t like structured logs