On Wed, Nov 06, 2024 at 09:26:24AM -0300, Fabiano Rosas wrote:
> Daniel P. Berrangé <berra...@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 <faro...@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 :|
