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 <pbonz...@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 <pbonz...@redhat.com>
> > Message-Id: <1477326663-67817-3-git-send-email-pbonz...@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é <berra...@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é <f4...@amsat.org>
> Tested-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
