| Message ID | 8d8b242f38caccd81c27125167862f4457e8a22f.1601655308.git.qemu_oss@crudebyte.com |
|---|---|
| State | New |
| Headers | show |
| Series | 9pfs: add tests using local fs driver | expand |
On 02/10/20 18:15, Christian Schoenebeck wrote: > -int main(int argc, char **argv) > +int main(int argc, char **argv, char** envp) > { > g_test_init(&argc, &argv, NULL); > + if (g_test_verbose()) { > + printf("ENVIRONMENT VARIABLES: {\n"); > + for (char **env = envp; *env != 0; env++) { > + printf("\t%s\n", *env); > + } > + printf("}\n"); > + } But doesn't this (and patch 6 as well) break TAP output? Using g_test_message + g_test_verbose would be the best of both worlds. In fact using printf in tests should be forbidden, since glib 2.62 and newer _always_ emit TAP. Paolo
On Donnerstag, 8. Oktober 2020 14:37:00 CEST Paolo Bonzini wrote: > On 02/10/20 18:15, Christian Schoenebeck wrote: > > -int main(int argc, char **argv) > > +int main(int argc, char **argv, char** envp) > > > > { > > > > g_test_init(&argc, &argv, NULL); > > > > + if (g_test_verbose()) { > > + printf("ENVIRONMENT VARIABLES: {\n"); > > + for (char **env = envp; *env != 0; env++) { > > + printf("\t%s\n", *env); > > + } > > + printf("}\n"); > > + } > > But doesn't this (and patch 6 as well) break TAP output? Using > g_test_message + g_test_verbose would be the best of both worlds. If there was TAP output then yes, patches 4, 5, 6 would probably break it. How/when is TAP output enabled? I don't see any TAP output by default. > In fact using printf in tests should be forbidden, since glib 2.62 and > newer _always_ emit TAP. > > Paolo The reason why I used printf() was because g_test_message() clutters the output tremendously, as it always wraps the text of each g_test_message() call into: "(MSG: %s)\n" which is inappropriate for multi-line messages as these proposed patches do. Is that actually a real-life problem? I mean these patches only output anything if --verbose CL switch is used, and I don't see any TAP output enabled by default. And the --verbose CL switch is usually just used by developers for debugging test case issues, isn't it? Best regards, Christian Schoenebeck
On 08/10/20 15:09, Christian Schoenebeck wrote: >> But doesn't this (and patch 6 as well) break TAP output? Using >> g_test_message + g_test_verbose would be the best of both worlds. > > If there was TAP output then yes, patches 4, 5, 6 would probably break it. > > How/when is TAP output enabled? I don't see any TAP output by default. With "--tap", but with glib 2.62 it will be enabled by default. For example on Fedora 32: $ ./test-mul64 # random seed: R02S3efb20d48a41e1897cb761e02393c11b 1..2 # Start of host-utils tests ok 1 /host-utils/mulu64 ok 2 /host-utils/muls64 # End of host-utils tests I'm okay I guess with using g_test_message on 2.62 or newer, and assuming people don't use --tap --verbose on older versions. Paolo > which is inappropriate for multi-line messages as these proposed patches do. > > Is that actually a real-life problem? I mean these patches only output > anything if --verbose CL switch is used, and I don't see any TAP output > enabled by default. And the --verbose CL switch is usually just used by > developers for debugging test case issues, isn't it? > > Best regards, > Christian Schoenebeck > >
On Donnerstag, 8. Oktober 2020 15:21:54 CEST Paolo Bonzini wrote: > On 08/10/20 15:09, Christian Schoenebeck wrote: > >> But doesn't this (and patch 6 as well) break TAP output? Using > >> g_test_message + g_test_verbose would be the best of both worlds. > > > > If there was TAP output then yes, patches 4, 5, 6 would probably break it. > > > > How/when is TAP output enabled? I don't see any TAP output by default. > > With "--tap", but with glib 2.62 it will be enabled by default. For > example on Fedora 32: > > $ ./test-mul64 > # random seed: R02S3efb20d48a41e1897cb761e02393c11b > 1..2 > # Start of host-utils tests > ok 1 /host-utils/mulu64 > ok 2 /host-utils/muls64 > # End of host-utils tests > > I'm okay I guess with using g_test_message on 2.62 or newer, and > assuming people don't use --tap --verbose on older versions. Simpler solution: just appending '#' character in front of each printf() line, that would be both fine for TAP and regular output: http://testanything.org/tap-specification.html#diagnostics Unfortunately 'test_tap_log' is a private variable in glib (gtestutils.c), otherwise I could have made that conditionally. There is no getter function in the glib API for this (TAP on/off) variable. I could check the CL for --verbose somewhere, but I think that's probably overkill. Best regards, Christian Schoenebeck
On 08/10/20 15:42, Christian Schoenebeck wrote: >> >> I'm okay I guess with using g_test_message on 2.62 or newer, and >> assuming people don't use --tap --verbose on older versions. > Simpler solution: just appending '#' character in front of each printf() line, > that would be both fine for TAP and regular output: > http://testanything.org/tap-specification.html#diagnostics I'm not sure how it would be simpler than a #if !GLIB_CHECK_VERSION(2, 62, 0) #define qemu_test_message printf #else #define qemu_test_message g_test_message #endif but you choose. Paolo
On Donnerstag, 8. Oktober 2020 15:52:08 CEST Paolo Bonzini wrote: > On 08/10/20 15:42, Christian Schoenebeck wrote: > >> I'm okay I guess with using g_test_message on 2.62 or newer, and > >> assuming people don't use --tap --verbose on older versions. > > > > Simpler solution: just appending '#' character in front of each printf() > > line, that would be both fine for TAP and regular output: > > http://testanything.org/tap-specification.html#diagnostics > > I'm not sure how it would be simpler than a > > #if !GLIB_CHECK_VERSION(2, 62, 0) > #define qemu_test_message printf > #else > #define qemu_test_message g_test_message > #endif > > but you choose. > > Paolo Simple yes, but it would not fix the cluttered output problem of g_test_message(). So I'll go with prepending '#' for now, and if one day there will be a public glib function to check for TAP mode, it can easily be adjusted. Best regards, Christian Schoenebeck
diff --git a/tests/qtest/qos-test.c b/tests/qtest/qos-test.c index d98ef78613..fe240b32a7 100644 --- a/tests/qtest/qos-test.c +++ b/tests/qtest/qos-test.c @@ -313,9 +313,16 @@ static void walk_path(QOSGraphNode *orig_path, int len) * machine/drivers/test objects * - Cleans up everything */ -int main(int argc, char **argv) +int main(int argc, char **argv, char** envp) { g_test_init(&argc, &argv, NULL); + if (g_test_verbose()) { + printf("ENVIRONMENT VARIABLES: {\n"); + for (char **env = envp; *env != 0; env++) { + printf("\t%s\n", *env); + } + printf("}\n"); + } qos_graph_init(); module_call_init(MODULE_INIT_QOM); module_call_init(MODULE_INIT_LIBQOS);
If qtests are run in verbose mode (i.e. if --verbose CL argument was provided) then print all environment variables to stdout before running the individual tests. Instead of using g_test_message() rather use printf() in combination with g_test_verbose(), to avoid g_test_message() cluttering the output. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- tests/qtest/qos-test.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)