Attachment 135466 Details for Bug 103601 – [06/13] Use _dbus_test_fatal to include more detail in test failure diagnostics

[patch] [06/13] Use _dbus_test_fatal to include more detail in test failure diagnostics

0006-Use-_dbus_test_fatal-to-include-more-detail-in-test-.patch (text/plain), 17.82 KB, created by Simon McVittie on 2017-11-14 19:10:07 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2017-11-14 19:10:07 UTC

Size: 17.82 KB

patch

obsolete

>From 3238c20fc8c0ae7a9232c48fdd3446655126650f Mon Sep 17 00:00:00 2001
>From: Simon McVittie <smcv@collabora.com>
>Date: Tue, 14 Nov 2017 14:13:45 +0000
>Subject: [PATCH 06/13] 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).
>
>Signed-off-by: Simon McVittie <smcv@collabora.com>
>---
> bus/dispatch.c                     | 16 +++------
> dbus/dbus-marshal-byteswap-util.c  |  3 +-
> dbus/dbus-marshal-recursive-util.c | 43 +++++++-----------------
> dbus/dbus-marshal-validate-util.c  | 67 ++++++++------------------------------
> dbus/dbus-message-util.c           | 12 +++----
> dbus/dbus-test.c                   | 24 ++++----------
> 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");
> 
>-- 
>2.15.0
>

Actions: View | Splinter Review

Attachments on bug 103601: 135460 | 135462 | 135463 | 135464 | 135465 | 135466 | 135467 | 135468 | 135469 | 135470 | 135471 | 135472 | 135473 | 135485

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.