[PATCH 0/3] tests: Return EXIT_* from main instead of 0/-1
See 3/3 for rationale. Michal Prívozník (3): testutils: Drop libtool binary name handling tests: Return EXIT_FAILURE/EXIT_SUCCESS instead of -1/0 testutils: Document and enforce @func callback retvals for virTestMain() tests/fchosttest.c | 2 +- tests/qemusecuritytest.c | 2 +- tests/scsihosttest.c | 2 +- tests/seclabeltest.c | 2 +- tests/storagepoolcapstest.c | 2 +- tests/testutils.c | 20 +++++++++++++++----- tests/testutils.h | 4 ++++ tests/virbitmaptest.c | 2 +- tests/vircaps2xmltest.c | 2 +- tests/vircapstest.c | 2 +- tests/virconftest.c | 2 +- tests/virendiantest.c | 2 +- tests/virlogtest.c | 2 +- tests/virresctrltest.c | 2 +- tests/virscsitest.c | 2 +- 15 files changed, 32 insertions(+), 18 deletions(-) -- 2.26.3
Back in the old days, we used to use libtool to run compiled libraries. That meant we had to deal with "lt-" prefix for our binaries. With meson that's no longer the case. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/testutils.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/testutils.c b/tests/testutils.c index d6b7c2a580..870a3b081a 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -750,8 +750,7 @@ int virTestMain(int argc, size_t noutputs = 0; virLogOutput *output = NULL; virLogOutput **outputs = NULL; - g_autofree char *baseprogname = NULL; - const char *progname; + g_autofree char *progname = NULL; g_autofree const char **preloads = NULL; size_t npreloads = 0; g_autofree char *mock = NULL; @@ -784,9 +783,7 @@ int virTestMain(int argc, VIR_TEST_PRELOAD(mock); } - progname = baseprogname = g_path_get_basename(argv[0]); - if (STRPREFIX(progname, "lt-")) - progname += 3; + progname = g_path_get_basename(argv[0]); g_setenv("VIR_TEST_MOCK_PROGNAME", progname, TRUE); -- 2.26.3
When using VIR_TEST_MAIN() or VIR_TEST_MAIN_PRELOAD() macros, the retval of mymain() will become retval of main(). Hence, mymain() should use EXIT_FAILURE and EXIT_SUCCESS return values for greater portability. Another reason is that otherwise our summary printing of failed tests doesn't work (see following commit for more info). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/fchosttest.c | 2 +- tests/qemusecuritytest.c | 2 +- tests/scsihosttest.c | 2 +- tests/seclabeltest.c | 2 +- tests/storagepoolcapstest.c | 2 +- tests/virbitmaptest.c | 2 +- tests/vircaps2xmltest.c | 2 +- tests/vircapstest.c | 2 +- tests/virconftest.c | 2 +- tests/virendiantest.c | 2 +- tests/virlogtest.c | 2 +- tests/virresctrltest.c | 2 +- tests/virscsitest.c | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/fchosttest.c b/tests/fchosttest.c index 44e7f11599..53d02241ca 100644 --- a/tests/fchosttest.c +++ b/tests/fchosttest.c @@ -374,7 +374,7 @@ mymain(void) ret = -1; VIR_FREE(fchost_prefix); - return ret; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virrandom")) diff --git a/tests/qemusecuritytest.c b/tests/qemusecuritytest.c index 184ffca15f..f7186700c4 100644 --- a/tests/qemusecuritytest.c +++ b/tests/qemusecuritytest.c @@ -261,7 +261,7 @@ mymain(void) #endif virObjectUnref(dac); virObjectUnref(stack); - return ret; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } VIR_TEST_MAIN_PRELOAD(mymain, diff --git a/tests/scsihosttest.c b/tests/scsihosttest.c index 17825bba35..7508ac37a3 100644 --- a/tests/scsihosttest.c +++ b/tests/scsihosttest.c @@ -282,7 +282,7 @@ mymain(void) VIR_FREE(fakerootdir); VIR_FREE(fakesysfsdir); VIR_FREE(scsihost_class_path); - return ret; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } VIR_TEST_MAIN(mymain) diff --git a/tests/seclabeltest.c b/tests/seclabeltest.c index 39a96d0fc0..b2a11b278c 100644 --- a/tests/seclabeltest.c +++ b/tests/seclabeltest.c @@ -33,7 +33,7 @@ mymain(void) virObjectUnref(mgr); - return 0; + return EXIT_SUCCESS; } VIR_TEST_MAIN(mymain) diff --git a/tests/storagepoolcapstest.c b/tests/storagepoolcapstest.c index f937670aa7..526c9ad045 100644 --- a/tests/storagepoolcapstest.c +++ b/tests/storagepoolcapstest.c @@ -101,7 +101,7 @@ mymain(void) DO_TEST("full", fullCaps); DO_TEST("fs", fsCaps); - return ret; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } VIR_TEST_MAIN(mymain) diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index a376a613db..02241c4c20 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -784,7 +784,7 @@ mymain(void) if (virTestRun("test16", test16, NULL) < 0) ret = -1; - return ret; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } VIR_TEST_MAIN(mymain) diff --git a/tests/vircaps2xmltest.c b/tests/vircaps2xmltest.c index ead3fb88ff..5b1b60124a 100644 --- a/tests/vircaps2xmltest.c +++ b/tests/vircaps2xmltest.c @@ -113,7 +113,7 @@ mymain(void) DO_TEST_FULL("resctrl-skx-twocaches", VIR_ARCH_X86_64, true, true); DO_TEST_FULL("resctrl-fake-feature", VIR_ARCH_X86_64, true, true); - return ret; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virnuma")) diff --git a/tests/vircapstest.c b/tests/vircapstest.c index 9299b42bf3..8cb6fafd1d 100644 --- a/tests/vircapstest.c +++ b/tests/vircapstest.c @@ -249,7 +249,7 @@ mymain(void) ret = -1; #endif /* WITH_LXC */ - return ret; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } VIR_TEST_MAIN(mymain) diff --git a/tests/virconftest.c b/tests/virconftest.c index 52f68287d6..f1a58b01cf 100644 --- a/tests/virconftest.c +++ b/tests/virconftest.c @@ -465,7 +465,7 @@ mymain(void) if (virTestRun("string-list", testConfParseStringList, NULL) < 0) ret = -1; - return ret; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/virendiantest.c b/tests/virendiantest.c index 38adef9353..8a6d2f5e2d 100644 --- a/tests/virendiantest.c +++ b/tests/virendiantest.c @@ -108,7 +108,7 @@ mymain(void) if (virTestRun("test2", test2, NULL) < 0) ret = -1; - return ret; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } VIR_TEST_MAIN(mymain) diff --git a/tests/virlogtest.c b/tests/virlogtest.c index 44d71015d5..582ac5c5b8 100644 --- a/tests/virlogtest.c +++ b/tests/virlogtest.c @@ -151,7 +151,7 @@ mymain(void) TEST_PARSE_FILTERS_FAIL(":foo", 1); TEST_PARSE_FILTERS_FAIL("1:+", 1); - return ret; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } VIR_TEST_MAIN(mymain) diff --git a/tests/virresctrltest.c b/tests/virresctrltest.c index 00f9f552e5..26fbde3668 100644 --- a/tests/virresctrltest.c +++ b/tests/virresctrltest.c @@ -93,7 +93,7 @@ mymain(void) DO_TEST_UNUSED("resctrl-skx"); DO_TEST_UNUSED("resctrl-skx-twocaches"); - return ret; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } VIR_TEST_MAIN(mymain) diff --git a/tests/virscsitest.c b/tests/virscsitest.c index 84b6f15ec6..0d7c35a261 100644 --- a/tests/virscsitest.c +++ b/tests/virscsitest.c @@ -229,7 +229,7 @@ mymain(void) if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(tmpdir); VIR_FREE(virscsi_prefix); - return ret; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } VIR_TEST_MAIN(mymain) -- 2.26.3
Sometimes a test has a wrapper over main() (e.g. because it's preloading some mock libraries). In such case, the main() is renamed to something else (usually mymain()), and main() is generated by calling one of VIR_TEST_MAIN() or VIR_TEST_MAIN_PRELOAD() macros. This has a neat side effect - if mymain() returns an error a short summary is printed, e.g.: Some tests failed. Run them using: VIR_TEST_DEBUG=1 VIR_TEST_RANGE=5-6 ./virtest However, this detection only works if EXIT_FAILURE is returned by mymain(). Document and enforce this limitation. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/testutils.c | 13 +++++++++++++ tests/testutils.h | 4 ++++ 2 files changed, 17 insertions(+) diff --git a/tests/testutils.c b/tests/testutils.c index 870a3b081a..b57b44fab5 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -838,6 +838,19 @@ int virTestMain(int argc, fprintf(stderr, "%*s", 40 - (int)(testCounter % 40), ""); fprintf(stderr, " %-3zu %s\n", testCounter, ret == 0 ? "OK" : "FAIL"); } + + switch (ret) { + case EXIT_FAILURE: + case EXIT_SUCCESS: + case EXIT_AM_SKIP: + case EXIT_AM_HARDFAIL: + break; + default: + fprintf(stderr, "Test callback did returned invalid value: %d\n", ret); + ret = EXIT_AM_HARDFAIL; + break; + } + if (ret == EXIT_FAILURE && !virBitmapIsAllClear(failedTests)) { g_autofree char *failed = virBitmapFormat(failedTests); fprintf(stderr, "Some tests failed. Run them using:\n"); diff --git a/tests/testutils.h b/tests/testutils.h index e268a95612..6848323586 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -98,6 +98,10 @@ void virTestQuiesceLibvirtErrors(bool always); void virTestCounterReset(const char *prefix); const char *virTestCounterNext(void); +/** + * The @func shall return EXIT_FAILURE or EXIT_SUCCESS or + * EXIT_AM_SKIP or EXIT_AM_HARDFAIL. + */ int virTestMain(int argc, char **argv, int (*func)(void), -- 2.26.3
On Mon, May 17, 2021 at 08:49:10 +0200, Michal Privoznik wrote:
Sometimes a test has a wrapper over main() (e.g. because it's preloading some mock libraries). In such case, the main() is renamed to something else (usually mymain()), and main() is generated by calling one of VIR_TEST_MAIN() or VIR_TEST_MAIN_PRELOAD() macros.
AFAIK it's not just sometimes but always in our testsuite.
This has a neat side effect - if mymain() returns an error a short summary is printed, e.g.:
Some tests failed. Run them using: VIR_TEST_DEBUG=1 VIR_TEST_RANGE=5-6 ./virtest
However, this detection only works if EXIT_FAILURE is returned by mymain(). Document and enforce this limitation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/testutils.c | 13 +++++++++++++ tests/testutils.h | 4 ++++ 2 files changed, 17 insertions(+)
diff --git a/tests/testutils.c b/tests/testutils.c index 870a3b081a..b57b44fab5 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -838,6 +838,19 @@ int virTestMain(int argc, fprintf(stderr, "%*s", 40 - (int)(testCounter % 40), ""); fprintf(stderr, " %-3zu %s\n", testCounter, ret == 0 ? "OK" : "FAIL"); } + + switch (ret) { + case EXIT_FAILURE: + case EXIT_SUCCESS: + case EXIT_AM_SKIP: + case EXIT_AM_HARDFAIL: + break; + default: + fprintf(stderr, "Test callback did returned invalid value: %d\n", ret);
s/did//
+ ret = EXIT_AM_HARDFAIL; + break; + }
On Mon, May 17, 2021 at 08:49:07 +0200, Michal Privoznik wrote:
See 3/3 for rationale.
Michal Prívozník (3): testutils: Drop libtool binary name handling tests: Return EXIT_FAILURE/EXIT_SUCCESS instead of -1/0 testutils: Document and enforce @func callback retvals for virTestMain()
Series: Reviewed-by: Peter Krempa <pkrempa@redhat.com> I've actually fixed a few instances before where we were returning -1 which was actually confusing the test suite and printing that the test got some weird signal.
participants (2)
-
Michal Privoznik -
Peter Krempa