All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Fabiano Rosas <farosas@suse.de>
Cc: qemu-devel@nongnu.org, "Juan Quintela" <quintela@redhat.com>,
	"Steve Sistare" <steven.sistare@oracle.com>,
	"Daniel P . Berrangé" <berrange@redhat.com>,
	"Leonardo Bras" <leobras@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Laurent Vivier" <lvivier@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>
Subject: Re: [PATCH 6/6] tests/qtest: migration-test: Add tests for file-based migration
Date: Fri, 30 Jun 2023 12:25:39 -0400	[thread overview]
Message-ID: <ZJ8CAw1S1n0hp192@x1n> (raw)
In-Reply-To: <874jmpq9cc.fsf@suse.de>

On Fri, Jun 30, 2023 at 12:05:23PM -0300, Fabiano Rosas wrote:
> >> +static void test_precopy_file_offset_bad(void)
> >> +{
> >> +    /* using a value not supported by qemu_strtosz() */
> >> +    g_autofree char *uri = g_strdup_printf("file:%s/migfile,offset=0x20M",
> >> +                                           tmpfs);
> >> +    MigrateCommon args = {
> >> +        .connect_uri = uri,
> >> +        .listen_uri = "defer",
> >> +        .error_str = g_strdup(
> >> +            "file URI has bad offset 0x20M: Unknown error -22"),
> >
> > "Unknown error" may imply that in Steve's patch the errno is inverted..
> >
> > Shall we not rely on the string in the test?  It might be too strict, I
> > worry, because error strings should be defined for human readers, and we
> > may not want some e.g. grammar / trivial change to break a test.
> >
> 
> Well, you just caught an issue with the errno by looking at the string,
> so maybe testing it is a good thing?
> 
> I'd expect anyone changing the string to run the test and catch the
> mismatch before sending a patch anyway.
> 
> I don't have a strong opinion about it, though. I can remove the
> error_str.

I can give a few other examples outside "grammar error" (which in this case
we can guarantee there's no grammar issue..): we can always try to append
something to an error, when error_setg_errno() got refactored, or even
someone just thinks better to make the 1st letter capitalized ("f" -> "F").

It's just too fragile to me..

Thanks,

-- 
Peter Xu



  parent reply	other threads:[~2023-06-30 16:26 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-28 16:55 [PATCH 0/6] migration: Test the new "file:" migration Fabiano Rosas
2023-06-28 16:55 ` [PATCH 1/6] migration: Set migration status early in incoming side Fabiano Rosas
2023-06-29 19:18   ` Peter Xu
2023-06-30 14:57     ` Fabiano Rosas
2023-06-28 16:55 ` [PATCH 2/6] tests/qtest: migration: Expose migrate_set_capability Fabiano Rosas
2023-06-29  6:46   ` Thomas Huth
2023-06-28 16:55 ` [PATCH 3/6] tests/qtest: migration: Add migrate_incoming_qmp helper Fabiano Rosas
2023-06-29 21:16   ` Peter Xu
2023-06-30 14:57     ` Fabiano Rosas
2023-06-28 16:55 ` [PATCH 4/6] tests/qtest: migration: Use migrate_incoming_qmp where appropriate Fabiano Rosas
2023-06-28 16:55 ` [PATCH 5/6] tests/qtest: migration: Add support for negative testing of qmp_migrate Fabiano Rosas
2023-06-28 16:55 ` [PATCH 6/6] tests/qtest: migration-test: Add tests for file-based migration Fabiano Rosas
2023-06-29 21:36   ` Peter Xu
2023-06-30 15:05     ` Fabiano Rosas
2023-06-30 15:19       ` Steven Sistare
2023-06-30 16:25       ` Peter Xu [this message]
  -- strict thread matches above, loose matches on Subject: below --
2023-06-26 18:22 [PATCH 0/6] migration: Test the new "file:" migration Fabiano Rosas
2023-06-26 18:22 ` [PATCH 6/6] tests/qtest: migration-test: Add tests for file-based migration Fabiano Rosas
2023-06-27  9:07   ` Daniel P. Berrangé
2023-06-27 12:54     ` Fabiano Rosas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZJ8CAw1S1n0hp192@x1n \
    --to=peterx@redhat.com \
    --cc=berrange@redhat.com \
    --cc=farosas@suse.de \
    --cc=leobras@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=steven.sistare@oracle.com \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.