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
