From f5f09445b88bf35e25970db41467c115dead910f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 20 Jan 2023 15:11:19 +0100 Subject: [PATCH 1/4] gtestutils: Define TAP version clearly as defined value As we do in tests --- glib/gtestutils.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/glib/gtestutils.c b/glib/gtestutils.c index a03ff559d..22449141f 100644 --- a/glib/gtestutils.c +++ b/glib/gtestutils.c @@ -58,6 +58,7 @@ #include "glib-private.h" #include "gutilsprivate.h" +#define TAP_VERSION G_STRINGIFY (13) /* FIXME: Remove '#' prefix when we'll depend on a meson version supporting TAP 14 * See https://gitlab.gnome.org/GNOME/glib/-/issues/2885 */ #define TAP_SUBTEST_PREFIX "# " /* a 4-space indented line */ @@ -1100,7 +1101,7 @@ g_test_log (GTestLogType lbit, { if (!is_subtest ()) { - g_test_tap_print (0, FALSE, "TAP version 13\n"); + g_test_tap_print (0, FALSE, "TAP version " TAP_VERSION "\n"); } else { From d8e4c1a8f8826bdb696438b6ac92773b4f5d3a27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 20 Jan 2023 15:27:27 +0100 Subject: [PATCH 2/4] gtestutils: Do not make a subtest to Bail out! the root one This is something that a base test should decide, so if a subtest fails it's up to the caller to decide whether to bail out the whole suite or just the subtest. --- glib/gtestutils.c | 11 +---------- glib/tests/testing.c | 6 ++---- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/glib/gtestutils.c b/glib/gtestutils.c index 22449141f..597629820 100644 --- a/glib/gtestutils.c +++ b/glib/gtestutils.c @@ -1219,16 +1219,7 @@ g_test_log (GTestLogType lbit, while ((line = strchr (line, '\n'))) *(line++) = ' '; - if (is_subtest ()) - { - g_test_tap_print (subtest_level, FALSE, "Bail out! %s\n", message); - g_test_tap_print (0, FALSE, "Bail out!\n"); - } - else - { - g_test_tap_print (0, FALSE, "Bail out! %s\n", message); - } - + g_test_tap_print (subtest_level, FALSE, "Bail out! %s\n", message); g_free (message); } else if (g_test_verbose ()) diff --git a/glib/tests/testing.c b/glib/tests/testing.c index e0653857d..3f335b4ac 100644 --- a/glib/tests/testing.c +++ b/glib/tests/testing.c @@ -2722,8 +2722,7 @@ test_tap_subtest_error (void) g_assert_cmpstr (interesting_lines, ==, TAP_SUBTEST_PREFIX "Bail out! GLib-FATAL-ERROR: This should error out " - "Because it's just wrong!\n" - "Bail out!\n"); + "Because it's just wrong!\n"); g_free (output); g_ptr_array_unref (argv); @@ -2815,8 +2814,7 @@ test_tap_subtest_error_and_pass (void) g_assert_cmpstr (interesting_lines, ==, TAP_SUBTEST_PREFIX "Bail out! GLib-FATAL-ERROR: This should error out " - "Because it's just wrong!\n" - "Bail out!\n"); + "Because it's just wrong!\n"); g_free (output); g_ptr_array_unref (argv); From a707cebcd9182386af206678a6000fffe8d169bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 20 Jan 2023 15:16:28 +0100 Subject: [PATCH 3/4] gtestutils: Improve TAP reporting on tests failures When we've a failure our TAP reporting just bails out without that is clear what is the test that has failed. So in this case, expose a "not ok" message if the test name is known, and use it to report the error message too if available. Otherwise just use the same Bail out! strategy. --- glib/gtestutils.c | 21 ++++++++++++++++++++- glib/tests/testing.c | 20 ++++++++++++-------- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/glib/gtestutils.c b/glib/gtestutils.c index 597629820..4945027dc 100644 --- a/glib/gtestutils.c +++ b/glib/gtestutils.c @@ -1219,7 +1219,26 @@ g_test_log (GTestLogType lbit, while ((line = strchr (line, '\n'))) *(line++) = ' '; - g_test_tap_print (subtest_level, FALSE, "Bail out! %s\n", message); + if (message) + message = g_strstrip (message); + + if (test_run_name && *test_run_name != '\0') + { + if (message && *message != '\0') + g_test_tap_print (subtest_level, FALSE, "not ok %s - %s\n", + test_run_name, message); + else + g_test_tap_print (subtest_level, FALSE, "not ok %s\n", + test_run_name); + + g_clear_pointer (&message, g_free); + } + + if (message && *message != '\0') + g_test_tap_print (subtest_level, FALSE, "Bail out! %s\n", message); + else + g_test_tap_print (subtest_level, FALSE, "Bail out!\n"); + g_free (message); } else if (g_test_verbose ()) diff --git a/glib/tests/testing.c b/glib/tests/testing.c index 3f335b4ac..e7baf7ca2 100644 --- a/glib/tests/testing.c +++ b/glib/tests/testing.c @@ -2676,8 +2676,9 @@ test_tap_error (void) g_assert_nonnull (interesting_lines); interesting_lines += strlen (expected_tap_header); - g_assert_cmpstr (interesting_lines, ==, "Bail out! GLib-FATAL-ERROR: This should error out " - "Because it's just wrong!\n"); + g_assert_cmpstr (interesting_lines, ==, "not ok /error - GLib-FATAL-ERROR: This should error out " + "Because it's just wrong!\n" + "Bail out!\n"); g_free (output); g_strfreev (envp); @@ -2721,8 +2722,9 @@ test_tap_subtest_error (void) interesting_lines += strlen (expected_tap_header); g_assert_cmpstr (interesting_lines, ==, - TAP_SUBTEST_PREFIX "Bail out! GLib-FATAL-ERROR: This should error out " - "Because it's just wrong!\n"); + TAP_SUBTEST_PREFIX "not ok /error - GLib-FATAL-ERROR: This should error out " + "Because it's just wrong!\n" + TAP_SUBTEST_PREFIX "Bail out!\n"); g_free (output); g_ptr_array_unref (argv); @@ -2768,8 +2770,9 @@ test_tap_error_and_pass (void) g_assert_nonnull (interesting_lines); interesting_lines += strlen (expected_tap_header); - g_assert_cmpstr (interesting_lines, ==, "Bail out! GLib-FATAL-ERROR: This should error out " - "Because it's just wrong!\n"); + g_assert_cmpstr (interesting_lines, ==, "not ok /error - GLib-FATAL-ERROR: This should error out " + "Because it's just wrong!\n" + "Bail out!\n"); g_free (output); g_strfreev (envp); @@ -2813,8 +2816,9 @@ test_tap_subtest_error_and_pass (void) interesting_lines += strlen (expected_tap_header); g_assert_cmpstr (interesting_lines, ==, - TAP_SUBTEST_PREFIX "Bail out! GLib-FATAL-ERROR: This should error out " - "Because it's just wrong!\n"); + TAP_SUBTEST_PREFIX "not ok /error - GLib-FATAL-ERROR: This should error out " + "Because it's just wrong!\n" + TAP_SUBTEST_PREFIX "Bail out!\n"); g_free (output); g_ptr_array_unref (argv); From a8f56c5be1d36bc5f0aa74e54e45d3e778cdc05e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 20 Jan 2023 15:41:17 +0100 Subject: [PATCH 4/4] ci: Use verbose output in meson by default Now that we're using TAP parsing, this will show subtest failures in details but without showing any logging error, that we'd still need to parse from actual logs. --- .gitlab-ci/run-tests.sh | 4 ++-- .gitlab-ci/test-msvc.bat | 4 ++-- .gitlab-ci/test-msys2.sh | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.gitlab-ci/run-tests.sh b/.gitlab-ci/run-tests.sh index 461f31f6e..1189493ba 100755 --- a/.gitlab-ci/run-tests.sh +++ b/.gitlab-ci/run-tests.sh @@ -4,13 +4,13 @@ set -ex ./.gitlab-ci/check-missing-install-tag.py _build -meson test \ +meson test -v \ -C _build \ --timeout-multiplier "${MESON_TEST_TIMEOUT_MULTIPLIER}" \ "$@" # Run only the flaky tests, so we can log the failures but without hard failing -meson test \ +meson test -v \ -C _build \ --timeout-multiplier "${MESON_TEST_TIMEOUT_MULTIPLIER}" \ "$@" --setup=unstable_tests --suite=failing --suite=flaky || true diff --git a/.gitlab-ci/test-msvc.bat b/.gitlab-ci/test-msvc.bat index 48b5668d5..60a62ff76 100644 --- a/.gitlab-ci/test-msvc.bat +++ b/.gitlab-ci/test-msvc.bat @@ -17,8 +17,8 @@ meson %args% _build || goto :error python .gitlab-ci/check-missing-install-tag.py _build || goto :error ninja -C _build || goto :error -meson test -C _build --timeout-multiplier %MESON_TEST_TIMEOUT_MULTIPLIER% || goto :error -meson test -C _build --timeout-multiplier %MESON_TEST_TIMEOUT_MULTIPLIER% --setup=unstable_tests --suite=failing --suite=flaky +meson test -v -C _build --timeout-multiplier %MESON_TEST_TIMEOUT_MULTIPLIER% || goto :error +meson test -v -C _build --timeout-multiplier %MESON_TEST_TIMEOUT_MULTIPLIER% --setup=unstable_tests --suite=failing --suite=flaky :: Workaround meson issue https://github.com/mesonbuild/meson/issues/9894 python -c "n = '_build/meson-logs/testlog.junit.xml'; c = open(n, 'rb').read().replace(b'\x1b', b''); open(n, 'wb').write(c)" || goto :error diff --git a/.gitlab-ci/test-msys2.sh b/.gitlab-ci/test-msys2.sh index 8b24c243d..d253007dd 100755 --- a/.gitlab-ci/test-msys2.sh +++ b/.gitlab-ci/test-msys2.sh @@ -54,8 +54,8 @@ if [[ "$CFLAGS" == *"-coverage"* ]]; then --output-file "${DIR}/_coverage/${CI_JOB_NAME}-baseline.lcov" fi -meson test --timeout-multiplier "${MESON_TEST_TIMEOUT_MULTIPLIER}" -meson test --timeout-multiplier "${MESON_TEST_TIMEOUT_MULTIPLIER}" \ +meson test -v --timeout-multiplier "${MESON_TEST_TIMEOUT_MULTIPLIER}" +meson test -v --timeout-multiplier "${MESON_TEST_TIMEOUT_MULTIPLIER}" \ --setup=unstable_tests --suite=failing --suite=flaky || true if [[ "$CFLAGS" == *"-coverage"* ]]; then