| Message ID | 20180718093815.8104-3-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [for,3.0,1/4] tests: call qcrypto_init instead of gnutls_global_init | expand |
Hi Daniel, On 07/18/2018 06:38 AM, Daniel P. Berrangé wrote: > The test-vmstate test is a bit chatty because it triggers various > expected failure scenarios and the code in question uses error_report > instead of accepting 'Error **errp' parameters. To silence this test the > stubs for error_vprintf() were changed to send errors via > g_test_message() instead of stderr: > > commit 28017e010ddf6849cfa830e898da3e44e6610952 > Author: Paolo Bonzini <pbonzini@redhat.com> > Date: Mon Oct 24 18:31:03 2016 +0200 > > tests: send error_report to test log > > Implement error_vprintf to send the output of error_report to > the test log. This silences test-vmstate. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > Message-Id: <1477326663-67817-3-git-send-email-pbonzini@redhat.com> > > Unfortunately this change has global impact across the entire test suite > and means that when tests fail for unexpected reasons, the message is > not displayed on stderr. eg when using &error_abort in a call the test > merely prints > > Unexpected error in qcrypto_tls_session_check_certificate() at crypto/tlssession.c:280: > > and the actual error message is hidden, making it impossible to diagnose > the failure. This is especially problematic in CI or build systems where > it isn't possible to easily pass the --debug-log flag to tests and > re-run with the test log visible. > > This change makes the previous big hammer much more nuanced, providing a > flag in the stub error_vprintf() that can used on a per-test basis to > silence the errors. Only the test-vmstate silences errors initially. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > stubs/error-printf.c | 5 ++++- > tests/test-vmstate.c | 3 +++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/stubs/error-printf.c b/stubs/error-printf.c > index ac6b92aa69..2199d79d28 100644 > --- a/stubs/error-printf.c > +++ b/stubs/error-printf.c > @@ -2,9 +2,12 @@ > #include "qemu-common.h" > #include "qemu/error-report.h" > > +bool silence_test_errors; This is not used. > + > void error_vprintf(const char *fmt, va_list ap) > { > - if (g_test_initialized() && !g_test_subprocess()) { > + if (g_test_initialized() && !g_test_subprocess() && > + getenv("QTEST_SILENT_ERRORS")) { > char *msg = g_strdup_vprintf(fmt, ap); > g_test_message("%s", msg); > g_free(msg); > diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c > index 087844b6c8..42923bb1df 100644 > --- a/tests/test-vmstate.c > +++ b/tests/test-vmstate.c > @@ -32,6 +32,7 @@ > #include "../migration/qemu-file-channel.h" > #include "../migration/savevm.h" > #include "qemu/coroutine.h" > +#include "qemu/error-report.h" Why? This doesn't seem necessary, neither related to this patch. > #include "io/channel-file.h" > > static char temp_file[] = "/tmp/vmst.test.XXXXXX"; > @@ -859,6 +860,8 @@ int main(int argc, char **argv) > > module_call_init(MODULE_INIT_QOM); > > + setenv("QTEST_SILENT_ERRORS", "1", 1); > + > g_test_init(&argc, &argv, NULL); > g_test_add_func("/vmstate/simple/primitive", test_simple_primitive); > g_test_add_func("/vmstate/versioned/load/v1", test_load_v1); > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
On Wed, Jul 18, 2018 at 10:44:11AM -0300, Philippe Mathieu-Daudé wrote: > Hi Daniel, > > On 07/18/2018 06:38 AM, Daniel P. Berrangé wrote: > > The test-vmstate test is a bit chatty because it triggers various > > expected failure scenarios and the code in question uses error_report > > instead of accepting 'Error **errp' parameters. To silence this test the > > stubs for error_vprintf() were changed to send errors via > > g_test_message() instead of stderr: > > > > commit 28017e010ddf6849cfa830e898da3e44e6610952 > > Author: Paolo Bonzini <pbonzini@redhat.com> > > Date: Mon Oct 24 18:31:03 2016 +0200 > > > > tests: send error_report to test log > > > > Implement error_vprintf to send the output of error_report to > > the test log. This silences test-vmstate. > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > Message-Id: <1477326663-67817-3-git-send-email-pbonzini@redhat.com> > > > > Unfortunately this change has global impact across the entire test suite > > and means that when tests fail for unexpected reasons, the message is > > not displayed on stderr. eg when using &error_abort in a call the test > > merely prints > > > > Unexpected error in qcrypto_tls_session_check_certificate() at crypto/tlssession.c:280: > > > > and the actual error message is hidden, making it impossible to diagnose > > the failure. This is especially problematic in CI or build systems where > > it isn't possible to easily pass the --debug-log flag to tests and > > re-run with the test log visible. > > > > This change makes the previous big hammer much more nuanced, providing a > > flag in the stub error_vprintf() that can used on a per-test basis to > > silence the errors. Only the test-vmstate silences errors initially. > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > --- > > stubs/error-printf.c | 5 ++++- > > tests/test-vmstate.c | 3 +++ > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/stubs/error-printf.c b/stubs/error-printf.c > > index ac6b92aa69..2199d79d28 100644 > > --- a/stubs/error-printf.c > > +++ b/stubs/error-printf.c > > @@ -2,9 +2,12 @@ > > #include "qemu-common.h" > > #include "qemu/error-report.h" > > > > +bool silence_test_errors; > > This is not used. Yes, accidentally left over from earlier version of patch > > > + > > void error_vprintf(const char *fmt, va_list ap) > > { > > - if (g_test_initialized() && !g_test_subprocess()) { > > + if (g_test_initialized() && !g_test_subprocess() && > > + getenv("QTEST_SILENT_ERRORS")) { > > char *msg = g_strdup_vprintf(fmt, ap); > > g_test_message("%s", msg); > > g_free(msg); > > diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c > > index 087844b6c8..42923bb1df 100644 > > --- a/tests/test-vmstate.c > > +++ b/tests/test-vmstate.c > > @@ -32,6 +32,7 @@ > > #include "../migration/qemu-file-channel.h" > > #include "../migration/savevm.h" > > #include "qemu/coroutine.h" > > +#include "qemu/error-report.h" > > Why? This doesn't seem necessary, neither related to this patch. Left over from an earlier version of the patch, I'll cull it. > > > #include "io/channel-file.h" > > > > static char temp_file[] = "/tmp/vmst.test.XXXXXX"; > > @@ -859,6 +860,8 @@ int main(int argc, char **argv) > > > > module_call_init(MODULE_INIT_QOM); > > > > + setenv("QTEST_SILENT_ERRORS", "1", 1); > > + > > g_test_init(&argc, &argv, NULL); > > g_test_add_func("/vmstate/simple/primitive", test_simple_primitive); > > g_test_add_func("/vmstate/versioned/load/v1", test_load_v1); > > > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Regards, Daniel
diff --git a/stubs/error-printf.c b/stubs/error-printf.c index ac6b92aa69..2199d79d28 100644 --- a/stubs/error-printf.c +++ b/stubs/error-printf.c @@ -2,9 +2,12 @@ #include "qemu-common.h" #include "qemu/error-report.h" +bool silence_test_errors; + void error_vprintf(const char *fmt, va_list ap) { - if (g_test_initialized() && !g_test_subprocess()) { + if (g_test_initialized() && !g_test_subprocess() && + getenv("QTEST_SILENT_ERRORS")) { char *msg = g_strdup_vprintf(fmt, ap); g_test_message("%s", msg); g_free(msg); diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c index 087844b6c8..42923bb1df 100644 --- a/tests/test-vmstate.c +++ b/tests/test-vmstate.c @@ -32,6 +32,7 @@ #include "../migration/qemu-file-channel.h" #include "../migration/savevm.h" #include "qemu/coroutine.h" +#include "qemu/error-report.h" #include "io/channel-file.h" static char temp_file[] = "/tmp/vmst.test.XXXXXX"; @@ -859,6 +860,8 @@ int main(int argc, char **argv) module_call_init(MODULE_INIT_QOM); + setenv("QTEST_SILENT_ERRORS", "1", 1); + g_test_init(&argc, &argv, NULL); g_test_add_func("/vmstate/simple/primitive", test_simple_primitive); g_test_add_func("/vmstate/versioned/load/v1", test_load_v1);
The test-vmstate test is a bit chatty because it triggers various expected failure scenarios and the code in question uses error_report instead of accepting 'Error **errp' parameters. To silence this test the stubs for error_vprintf() were changed to send errors via g_test_message() instead of stderr: commit 28017e010ddf6849cfa830e898da3e44e6610952 Author: Paolo Bonzini <pbonzini@redhat.com> Date: Mon Oct 24 18:31:03 2016 +0200 tests: send error_report to test log Implement error_vprintf to send the output of error_report to the test log. This silences test-vmstate. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <1477326663-67817-3-git-send-email-pbonzini@redhat.com> Unfortunately this change has global impact across the entire test suite and means that when tests fail for unexpected reasons, the message is not displayed on stderr. eg when using &error_abort in a call the test merely prints Unexpected error in qcrypto_tls_session_check_certificate() at crypto/tlssession.c:280: and the actual error message is hidden, making it impossible to diagnose the failure. This is especially problematic in CI or build systems where it isn't possible to easily pass the --debug-log flag to tests and re-run with the test log visible. This change makes the previous big hammer much more nuanced, providing a flag in the stub error_vprintf() that can used on a per-test basis to silence the errors. Only the test-vmstate silences errors initially. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- stubs/error-printf.c | 5 ++++- tests/test-vmstate.c | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-)