Use _dbus_test_fatal to include more detail in test failure diagnostics

Unlike _dbus_assert_not_reached(), this new function takes a printf-style
format string, so we don't need to use a _dbus_warn() to explain why
the failure occurred (unless the failure message is multi-line).

Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103601

6 files changed, 41 insertions, 124 deletions

diff --git a/bus/dispatch.c b/bus/dispatch.c
index 19228bed..9a14849a 100644
--- a/bus/dispatch.c
+++ b/bus/dispatch.c
@@ -35,6 +35,7 @@
 #include "test.h"
 #include <dbus/dbus-internals.h>
 #include <dbus/dbus-misc.h>
+#include <dbus/dbus-test-tap.h>
 #include <string.h>
 
 #ifdef HAVE_UNIX_FD_PASSING
@@ -4783,10 +4784,7 @@ check2_try_iterations (BusContext     *context,
 
   if (!_dbus_test_oom_handling (description, check_oom_check2_func,
                                 &d))
-    {
-      _dbus_warn ("%s failed during oom", description);
-      _dbus_assert_not_reached ("test failed");
-    }
+    _dbus_test_fatal ("%s failed during oom", description);
 }
 
 static dbus_bool_t
@@ -4914,10 +4912,7 @@ bus_dispatch_test_conf (const DBusString *test_data_dir,
     _dbus_assert_not_reached ("ListActivatableNames message failed");
 
   if (!check_no_leftovers (context))
-    {
-      _dbus_warn ("Messages were left over after setting up initial connections");
-      _dbus_assert_not_reached ("initial connection setup failed");
-    }
+    _dbus_test_fatal ("Messages were left over after setting up initial connections");
 
   check1_try_iterations (context, "create_and_hello",
                          check_hello_connection);
@@ -5090,10 +5085,7 @@ bus_dispatch_sha1_test (const DBusString *test_data_dir)
     _dbus_assert_not_reached ("addmatch message failed");
 
   if (!check_no_leftovers (context))
-    {
-      _dbus_warn ("Messages were left over after setting up initial SHA-1 connection");
-      _dbus_assert_not_reached ("initial connection setup failed");
-    }
+    _dbus_test_fatal ("Messages were left over after setting up initial SHA-1 connection");
 
   check1_try_iterations (context, "create_and_hello_sha1",
                          check_hello_connection);
diff --git a/dbus/dbus-marshal-byteswap-util.c b/dbus/dbus-marshal-byteswap-util.c
index 4ee6f4ab..85d46d1d 100644
--- a/dbus/dbus-marshal-byteswap-util.c
+++ b/dbus/dbus-marshal-byteswap-util.c
@@ -76,8 +76,7 @@ do_byteswap_test (int byte_order)
           _dbus_verbose_bytes_of_string (&copy, 0,
                                          _dbus_string_get_length (&copy));
 
-          _dbus_warn ("Byte-swapped data did not have same values as original data");
-          _dbus_assert_not_reached ("test failed");
+          _dbus_test_fatal ("Byte-swapped data did not have same values as original data");
         }
       
       _dbus_string_free (&copy);
diff --git a/dbus/dbus-marshal-recursive-util.c b/dbus/dbus-marshal-recursive-util.c
index b30f5883..5ebd6363 100644
--- a/dbus/dbus-marshal-recursive-util.c
+++ b/dbus/dbus-marshal-recursive-util.c
@@ -29,6 +29,7 @@
 #include "dbus-marshal-basic.h"
 #include "dbus-signature.h"
 #include "dbus-internals.h"
+#include <dbus/dbus-test-tap.h>
 #include <string.h>
 
 #if !defined(PRIx64) && defined(DBUS_WIN)
@@ -307,12 +308,10 @@ real_check_expected_type (DBusTypeReader *reader,
 
   if (t != expected)
     {
-      _dbus_warn ("Read type %s while expecting %s at %s line %d",
+      _dbus_test_fatal ("Read wrong type: read type %s while expecting %s at %s line %d",
                   _dbus_type_to_string (t),
                   _dbus_type_to_string (expected),
                   funcname, line);
-
-      _dbus_assert_not_reached ("read wrong type");
     }
 }
 
@@ -320,17 +319,15 @@ real_check_expected_type (DBusTypeReader *reader,
 
 #define NEXT_EXPECTING_TRUE(reader)  do { if (!_dbus_type_reader_next (reader))         \
  {                                                                                      \
-    _dbus_warn ("_dbus_type_reader_next() should have returned TRUE at %s %d",          \
+    _dbus_test_fatal ("_dbus_type_reader_next() should have returned TRUE at %s %d",    \
                               _DBUS_FUNCTION_NAME, __LINE__);                           \
-    _dbus_assert_not_reached ("test failed");                                           \
  }                                                                                      \
 } while (0)
 
 #define NEXT_EXPECTING_FALSE(reader) do { if (_dbus_type_reader_next (reader))          \
  {                                                                                      \
-    _dbus_warn ("_dbus_type_reader_next() should have returned FALSE at %s %d",         \
+    _dbus_test_fatal ("_dbus_type_reader_next() should have returned FALSE at %s %d",   \
                               _DBUS_FUNCTION_NAME, __LINE__);                           \
-    _dbus_assert_not_reached ("test failed");                                           \
  }                                                                                      \
  check_expected_type (reader, DBUS_TYPE_INVALID);                                       \
 } while (0)
@@ -1398,11 +1395,10 @@ run_test_nodes_iteration (void *data)
   if (!_dbus_string_equal_substring (nid->signature, 0, _dbus_string_get_length (nid->signature),
                                      &nid->block->signature, nid->type_offset))
     {
-      _dbus_warn ("Expected signature '%s' and got '%s' with initial offset %d",
+      _dbus_test_fatal ("Expected signature '%s' and got '%s' with initial offset %d",
                   _dbus_string_get_const_data (nid->signature),
                   _dbus_string_get_const_data_len (&nid->block->signature, nid->type_offset, 0),
                   nid->type_offset);
-      _dbus_assert_not_reached ("wrong signature");
     }
 
   i = 0;
@@ -2456,11 +2452,7 @@ string_read_value (TestTypeNode   *node,
                     seed);
 
   if (strcmp (buf, v) != 0)
-    {
-      _dbus_warn ("read string '%s' expected '%s'",
-                  v, buf);
-      _dbus_assert_not_reached ("test failed");
-    }
+    _dbus_test_fatal ("read string '%s' expected '%s'", v, buf);
 
   return TRUE;
 }
@@ -2627,13 +2619,10 @@ double_read_value (TestTypeNode   *node,
   expected = double_from_seed (seed);
 
   if (!_DBUS_DOUBLES_BITWISE_EQUAL (v, expected))
-    {
-      _dbus_warn ("Expected double %g got %g\n bits = 0x%" PRIx64 " vs.\n bits = 0x%" PRIx64,
-                  expected, v,
-                  *(dbus_uint64_t*)(char*)&expected,
-                  *(dbus_uint64_t*)(char*)&v);
-      _dbus_assert_not_reached ("test failed");
-    }
+    _dbus_test_fatal ("Expected double %g got %g\n bits = 0x%" PRIx64 " vs.\n bits = 0x%" PRIx64,
+                      expected, v,
+                      *(dbus_uint64_t*)(char*)&expected,
+                      *(dbus_uint64_t*)(char*)&v);
 
   return TRUE;
 }
@@ -2724,11 +2713,7 @@ object_path_read_value (TestTypeNode   *node,
   object_path_from_seed (buf, seed);
 
   if (strcmp (buf, v) != 0)
-    {
-      _dbus_warn ("read object path '%s' expected '%s'",
-                  v, buf);
-      _dbus_assert_not_reached ("test failed");
-    }
+    _dbus_test_fatal ("read object path '%s' expected '%s'", v, buf);
 
   return TRUE;
 }
@@ -2799,11 +2784,7 @@ signature_read_value (TestTypeNode   *node,
   signature_from_seed (buf, seed);
 
   if (strcmp (buf, v) != 0)
-    {
-      _dbus_warn ("read signature value '%s' expected '%s'",
-                  v, buf);
-      _dbus_assert_not_reached ("test failed");
-    }
+    _dbus_test_fatal ("read signature value '%s' expected '%s'", v, buf);
 
   return TRUE;
 }
diff --git a/dbus/dbus-marshal-validate-util.c b/dbus/dbus-marshal-validate-util.c
index 9e75a7e7..86b08a1a 100644
--- a/dbus/dbus-marshal-validate-util.c
+++ b/dbus/dbus-marshal-validate-util.c
@@ -29,6 +29,7 @@
 #include "dbus-internals.h"
 #include "dbus-marshal-validate.h"
 #include "dbus-marshal-recursive.h"
+#include <dbus/dbus-test-tap.h>
 
 #include "dbus-test.h"
 #include <stdio.h>
@@ -56,11 +57,7 @@ run_validity_tests (const ValidityTest *tests,
       v = (*func) (&str, 0, _dbus_string_get_length (&str));
 
       if (v != tests[i].expected)
-        {
-          _dbus_warn ("Improper validation result %d for '%s'",
-                      v, tests[i].data);
-          _dbus_assert_not_reached ("test failed");
-        }
+        _dbus_test_fatal ("Improper validation result %d for '%s'", v, tests[i].data);
     }
 }
 
@@ -234,10 +231,7 @@ _dbus_marshal_validate_test (void)
 
       if (!_dbus_validate_path (&str, 0,
                                 _dbus_string_get_length (&str)))
-        {
-          _dbus_warn ("Path \"%s\" should have been valid", valid_paths[i]);
-          _dbus_assert_not_reached ("invalid path");
-        }
+        _dbus_test_fatal ("Path \"%s\" should have been valid", valid_paths[i]);
 
       ++i;
     }
@@ -249,10 +243,7 @@ _dbus_marshal_validate_test (void)
 
       if (_dbus_validate_path (&str, 0,
                                _dbus_string_get_length (&str)))
-        {
-          _dbus_warn ("Path \"%s\" should have been invalid", invalid_paths[i]);
-          _dbus_assert_not_reached ("valid path");
-        }
+        _dbus_test_fatal ("Path \"%s\" should have been invalid", invalid_paths[i]);
 
       ++i;
     }
@@ -265,10 +256,7 @@ _dbus_marshal_validate_test (void)
 
       if (!_dbus_validate_interface (&str, 0,
                                      _dbus_string_get_length (&str)))
-        {
-          _dbus_warn ("Interface \"%s\" should have been valid", valid_interfaces[i]);
-          _dbus_assert_not_reached ("invalid interface");
-        }
+        _dbus_test_fatal ("Interface \"%s\" should have been valid", valid_interfaces[i]);
 
       ++i;
     }
@@ -280,10 +268,7 @@ _dbus_marshal_validate_test (void)
 
       if (_dbus_validate_interface (&str, 0,
                                     _dbus_string_get_length (&str)))
-        {
-          _dbus_warn ("Interface \"%s\" should have been invalid", invalid_interfaces[i]);
-          _dbus_assert_not_reached ("valid interface");
-        }
+        _dbus_test_fatal ("Interface \"%s\" should have been invalid", invalid_interfaces[i]);
 
       ++i;
     }
@@ -298,10 +283,7 @@ _dbus_marshal_validate_test (void)
 
       if (!_dbus_validate_bus_name (&str, 0,
                                    _dbus_string_get_length (&str)))
-        {
-          _dbus_warn ("Bus name \"%s\" should have been valid", valid_interfaces[i]);
-          _dbus_assert_not_reached ("invalid bus name");
-        }
+        _dbus_test_fatal ("Bus name \"%s\" should have been valid", valid_interfaces[i]);
 
       ++i;
     }
@@ -315,10 +297,7 @@ _dbus_marshal_validate_test (void)
 
           if (_dbus_validate_bus_name (&str, 0,
                                        _dbus_string_get_length (&str)))
-            {
-              _dbus_warn ("Bus name \"%s\" should have been invalid", invalid_interfaces[i]);
-              _dbus_assert_not_reached ("valid bus name");
-            }
+            _dbus_test_fatal ("Bus name \"%s\" should have been invalid", invalid_interfaces[i]);
         }
 
       ++i;
@@ -332,10 +311,7 @@ _dbus_marshal_validate_test (void)
 
       if (!_dbus_validate_bus_name (&str, 0,
                                     _dbus_string_get_length (&str)))
-        {
-          _dbus_warn ("Bus name \"%s\" should have been valid", valid_unique_names[i]);
-          _dbus_assert_not_reached ("invalid unique name");
-        }
+        _dbus_test_fatal ("Bus name \"%s\" should have been valid", valid_unique_names[i]);
 
       ++i;
     }
@@ -347,10 +323,7 @@ _dbus_marshal_validate_test (void)
 
       if (_dbus_validate_bus_name (&str, 0,
                                    _dbus_string_get_length (&str)))
-        {
-          _dbus_warn ("Bus name \"%s\" should have been invalid", invalid_unique_names[i]);
-          _dbus_assert_not_reached ("valid unique name");
-        }
+        _dbus_test_fatal ("Bus name \"%s\" should have been invalid", invalid_unique_names[i]);
 
       ++i;
     }
@@ -365,10 +338,7 @@ _dbus_marshal_validate_test (void)
 
       if (!_dbus_validate_error_name (&str, 0,
                                       _dbus_string_get_length (&str)))
-        {
-          _dbus_warn ("Error name \"%s\" should have been valid", valid_interfaces[i]);
-          _dbus_assert_not_reached ("invalid error name");
-        }
+        _dbus_test_fatal ("Error name \"%s\" should have been valid", valid_interfaces[i]);
 
       ++i;
     }
@@ -382,10 +352,7 @@ _dbus_marshal_validate_test (void)
 
           if (_dbus_validate_error_name (&str, 0,
                                          _dbus_string_get_length (&str)))
-            {
-              _dbus_warn ("Error name \"%s\" should have been invalid", invalid_interfaces[i]);
-              _dbus_assert_not_reached ("valid error name");
-            }
+            _dbus_test_fatal ("Error name \"%s\" should have been invalid", invalid_interfaces[i]);
         }
 
       ++i;
@@ -399,10 +366,7 @@ _dbus_marshal_validate_test (void)
 
       if (!_dbus_validate_member (&str, 0,
                                   _dbus_string_get_length (&str)))
-        {
-          _dbus_warn ("Member \"%s\" should have been valid", valid_members[i]);
-          _dbus_assert_not_reached ("invalid member");
-        }
+        _dbus_test_fatal ("Member \"%s\" should have been valid", valid_members[i]);
 
       ++i;
     }
@@ -414,10 +378,7 @@ _dbus_marshal_validate_test (void)
 
       if (_dbus_validate_member (&str, 0,
                                  _dbus_string_get_length (&str)))
-        {
-          _dbus_warn ("Member \"%s\" should have been invalid", invalid_members[i]);
-          _dbus_assert_not_reached ("valid member");
-        }
+        _dbus_test_fatal ("Member \"%s\" should have been invalid", invalid_members[i]);
 
       ++i;
     }
diff --git a/dbus/dbus-message-util.c b/dbus/dbus-message-util.c
index a3b97833..629eb504 100644
--- a/dbus/dbus-message-util.c
+++ b/dbus/dbus-message-util.c
@@ -149,9 +149,8 @@ check_memleaks (void)
 
   if (_dbus_get_malloc_blocks_outstanding () != 0)
     {
-      _dbus_warn ("%d dbus_malloc blocks were not freed in %s",
+      _dbus_test_fatal ("%d dbus_malloc blocks were not freed in %s",
                   _dbus_get_malloc_blocks_outstanding (), __FILE__);
-      _dbus_assert_not_reached ("memleaks");
     }
 }
 
@@ -261,8 +260,7 @@ _dbus_check_fdleaks_leave (DBusInitialFDs *fds)
           if (FD_ISSET (fd, &fds->set))
             continue;
 
-          _dbus_warn ("file descriptor %i leaked in %s.", fd, __FILE__);
-          _dbus_assert_not_reached ("fdleaks");
+          _dbus_test_fatal ("file descriptor %i leaked in %s.", fd, __FILE__);
         }
 
       closedir (d);
@@ -1176,9 +1174,8 @@ verify_test_message_memleak (DBusMessage *message)
 #endif
                                    DBUS_TYPE_INVALID))
     {
-      _dbus_warn ("error: %s - %s", error.name,
+      _dbus_test_fatal ("Could not get arguments: %s - %s", error.name,
                   (error.message != NULL) ? error.message : "no message");
-      _dbus_assert_not_reached ("Could not get arguments");
     }
   else
     {
@@ -1793,9 +1790,8 @@ _dbus_message_test (const char *test_data_dir)
         if (!dbus_internal_do_not_use_try_message_data (&mdata.data,
                                                         mdata.expected_validity))
           {
-            _dbus_warn ("expected validity %d and did not get it",
+            _dbus_test_fatal ("expected validity %d and did not get it",
                         mdata.expected_validity);
-            _dbus_assert_not_reached ("message data failed");
           }
 
         _dbus_message_data_free (&mdata);
diff --git a/dbus/dbus-test.c b/dbus/dbus-test.c
index c6e9c19f..8ae91d65 100644
--- a/dbus/dbus-test.c
+++ b/dbus/dbus-test.c
@@ -30,15 +30,6 @@
 #include <stdlib.h>
 
 #ifdef DBUS_ENABLE_EMBEDDED_TESTS
-static void die (const char *failure) _DBUS_GNUC_NORETURN;
-
-static void
-die (const char *failure)
-{
-  fprintf (stderr, "Unit test failed: %s\n", failure);
-  exit (1);
-}
-
 static void
 check_memleaks (void)
 {
@@ -46,11 +37,8 @@ check_memleaks (void)
 
   _dbus_test_diag ("%s: checking for memleaks", "test-dbus");
   if (_dbus_get_malloc_blocks_outstanding () != 0)
-    {
-      _dbus_warn ("%d dbus_malloc blocks were not freed",
-                  _dbus_get_malloc_blocks_outstanding ());
-      die ("memleaks");
-    }
+    _dbus_test_fatal ("%d dbus_malloc blocks were not freed",
+                      _dbus_get_malloc_blocks_outstanding ());
 }
 
 typedef dbus_bool_t (*TestFunc)(void);
@@ -65,7 +53,7 @@ run_test (const char             *test_name,
     {
       _dbus_test_diag ("%s: running %s tests", "test-dbus", test_name);
       if (!test ())
-	die (test_name);
+        _dbus_test_fatal ("%s test failed", test_name);
 
       check_memleaks ();
     }
@@ -81,7 +69,7 @@ run_data_test (const char             *test_name,
     {
       _dbus_test_diag ("%s: running %s tests", "test-dbus", test_name);
       if (!test (test_data_dir))
-	die (test_name);
+        _dbus_test_fatal ("%s test failed", test_name);
 
       check_memleaks ();
     }
@@ -101,8 +89,8 @@ void
 dbus_internal_do_not_use_run_tests (const char *test_data_dir, const char *specific_test)
 {
   if (!_dbus_threads_init_debug ())
-    die ("debug threads init");
-  
+    _dbus_test_fatal ("debug threads init failed");
+
   if (test_data_dir == NULL)
     test_data_dir = _dbus_getenv ("DBUS_TEST_DATA");