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