Re: [PATCH 11/22] tests/qtest/migration: Move common test code

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Nov 06, 2024 at 09:26:24AM -0300, Fabiano Rosas wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Tue, Nov 05, 2024 at 03:08:26PM -0300, Fabiano Rosas wrote:
> >> Put the common test code in a separate file. Leave only individual
> >> test functions and their static helpers in migration-test.c.
> >> 
> >> This moves the shared:
> >> 
> >>  test_postcopy_common
> >>  test_postcopy_recovery_common
> >>  test_precopy_common
> >>  test_file_common
> >>  migrate_precopy_tcp_multifd_start_common
> >>  migrate_start
> >>  migrate_end
> >>  migration_get_env
> >>  migration_env_clean
> >> 
> >> and some of their static helpers to migrate-common.c.
> >> 
> >> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> >> ---
> >>  tests/qtest/meson.build                  |    1 +
> >>  tests/qtest/migration-test.c             | 1124 +---------------------
> >>  tests/qtest/migration/bootfile.c         |    2 +-
> >>  tests/qtest/migration/bootfile.h         |    2 +-
> >>  tests/qtest/migration/migration-common.c |  963 ++++++++++++++++++
> >>  tests/qtest/migration/migration-common.h |  214 ++++
> >>  tests/qtest/migration/migration-util.h   |   13 -
> >
> > Looking at these I guess I'd ask the question of what is the rule for
> > deciding whether to put something in -common vs in -util ?
> >
> > Both names sounds like being a general "bag of bits", so it isn't
> > obvious to me what their distinct roles are.
> >
> 
> The idea was to make -util the general bag of bits just like
> migration-helpers is currently.
> 
> -common should be the more "important" migration-specific functions,
> code that we spend more time reading and that are actually responsible
> for the results of the tests. Consider:
> 
> static void test_multifd_file_mapped_ram_fdset_dio(void)
> {
>     ...
>     if (!probe_o_direct_support(tmpfs)) {          <--- just a helper
>         g_test_skip("Filesystem does not support O_DIRECT");
>         return;
>     }
> 
>     test_file_common(&args, true);  <--- actually doing the heavy-lifting
> }
> 
> We could to some renaming and restructuring, but I think the distinction
> is important. We already have it today, what is -common is all stuffed
> into migration-test.c and the rest is in migration-helpers.c.

Ah ok, perhaps "migration/test-framework.{c,h}" is a more descriptive
name for this ?

> 
> >>  7 files changed, 1193 insertions(+), 1126 deletions(-)
> >>  create mode 100644 tests/qtest/migration/migration-common.c
> >>  create mode 100644 tests/qtest/migration/migration-common.h
> >> 
> >
> > With regards,
> > Daniel
> 

With 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 :|