Re: [PATCH 6/6] tests/qtest: migration-test: Add tests for file-based migration

[Peter Xu]

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