GNOME Bugzilla – Bug 767609
Test suite problems
Last modified: 2018-05-24 18:56:42 UTC
Created attachment 329702 [details] [review] [PATCH 1/6] tests: Use built modules instead of installed ones I was upgrading glib from v2.44.1 to v2.48.1 and having some problems with the test suite. Please consider the attached 6 patches. After having applied them locally, the test suite stopped from failing and ended error-free.
Created attachment 329703 [details] [review] [PATCH 2/6] tests: Grant timer some time to run
Created attachment 329704 [details] [review] [PATCH 3/6] tests: Avoid rounding errors with double precision variables
Created attachment 329705 [details] [review] [PATCH 4/6] tests: Continue iochannel test if iconv isn't supporting EUC-JP
Created attachment 329706 [details] [review] [PATCH 5/6] tests: Fix network-address test
Created attachment 329707 [details] [review] [PATCH 6/6] tests: Don't run tests requiring DBUS unconditionally
Anyone? These shouldn't be too hard to review.
Review of attachment 329703 [details] [review]: There's no explanation for why this would be necessary; we should not sprinkle random sleep statements in the hope that things get better.
Review of attachment 329704 [details] [review]: ::: glib/tests/timer.c @@ +57,3 @@ elapsed2 = g_timer_elapsed (timer, NULL); + g_assert_cmpfloat ((float)elapsed, ==, (float)elapsed2); I don't think this cast does anything... the first thing g_assert_cmpfloat does is assigning both arguments to long double variables
Review of attachment 329705 [details] [review]: Have you actually seen this fail with G_CONVERT_ERROR_FAILED ? Looking at the code, I don't think that case will ever happen.
Review of attachment 329702 [details] [review]: ::: gio/tests/Makefile.am @@ +20,3 @@ # Test programs buildable on all platforms +TESTS_ENVIRONMENT += GIO_MODULE_DIR="$(abs_builddir)$(subdir)/.libs" The idea makes sense, but the patch doesn't quite look right to me. Looking in gio/tests/Makefile, I see: abs_builddir = /home/mclasen/Sources/glib/gio/tests subdir = gio/tests So you'll end up with .../glib/gio/tests/gio/tests/.libs instead of .../glib/gio/.libs, which was probably what you intended.
Review of attachment 329707 [details] [review]: ok
(In reply to Matthias Clasen from comment #7) > Review of attachment 329703 [details] [review] [review]: > > There's no explanation for why this would be necessary; we should not > sprinkle random sleep statements in the hope that things get better. There are already some g_usleep() statements in the test. Adding +++ glib/tests/timer.c @@ -37,0 +38 @@ + g_message("micros: %lu, elapsed: %llu\n", micros, ((guint64)(elapsed * 1e6)) % 1000000); reveals (timer.log): GLib-Message: micros: 1, elapsed: 0 ** GLib:ERROR:timer.c:39:test_timer_basic: assertion failed (micros == ((guint64)(elapsed * 1e6)) % 1000000): (1 == 0) ../../tap-test: line 5: 1968 Aborted $1 -k --tap # random seed: R02S69c0094b99ee30717e12fbf84f35580b 1..7 # Start of timer tests # GLib-MESSAGE: micros: 1, elapsed: 0 # GLib:ERROR:timer.c:39:test_timer_basic: assertion failed (micros == ((guint64)(elapsed * 1e6)) % 1000000): (1 == 0) ERROR: timer - too few tests run (expected 7, got 0) ERROR: timer - exited with status 134 (terminated by signal 6?)
(In reply to Matthias Clasen from comment #8) > Review of attachment 329704 [details] [review] [review]: > > ::: glib/tests/timer.c > @@ +57,3 @@ > elapsed2 = g_timer_elapsed (timer, NULL); > > + g_assert_cmpfloat ((float)elapsed, ==, (float)elapsed2); > > I don't think this cast does anything... the first thing g_assert_cmpfloat > does is assigning both arguments to long double variables Without the cast I get: GLib:ERROR:timer.c:57:test_timer_stop: assertion failed (elapsed == elapsed2): (1e-06 == 1e-06)
(In reply to Matthias Clasen from comment #9) > Review of attachment 329705 [details] [review] [review]: > > Have you actually seen this fail with G_CONVERT_ERROR_FAILED ? Looking at > the code, I don't think that case will ever happen. I was surprised looking at the code as well, but I actually have: ** (/tmp/glib-2.48.1/tests/.libs/lt-iochannel-test:7831): WARNING **: Could not open converter from 'EUC-JP' to 'UTF-8': No such file or directory: 2 FAIL: iochannel-test after changing: +++ tests/iochannel-test.c @@ -91 +91 @@ - g_warning ("%s", gerr->message); + g_warning ("%s: %d", gerr->message, gerr->code);
Created attachment 332547 [details] [review] [PATCH v2 1/6] tests: Use built modules instead of installed ones
(In reply to Matthias Clasen from comment #10) > Review of attachment 329702 [details] [review] [review]: > > ::: gio/tests/Makefile.am > @@ +20,3 @@ > # Test programs buildable on all platforms > > +TESTS_ENVIRONMENT += GIO_MODULE_DIR="$(abs_builddir)$(subdir)/.libs" > > The idea makes sense, but the patch doesn't quite look right to me. Looking > in gio/tests/Makefile, I see: > > abs_builddir = /home/mclasen/Sources/glib/gio/tests > subdir = gio/tests > > So you'll end up with .../glib/gio/tests/gio/tests/.libs instead of > .../glib/gio/.libs, which was probably what you intended. See v2 of the patch.
Comment on attachment 329707 [details] [review] [PATCH 6/6] tests: Don't run tests requiring DBUS unconditionally Pushed as commit 731e7fea1739b60ffdff21c38a79155a6a2f66d6.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/1173.