[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] qtest/migration: Add a test for the analyze-migration script
From: |
Fabiano Rosas |
Subject: |
Re: [PATCH] qtest/migration: Add a test for the analyze-migration script |
Date: |
Wed, 27 Sep 2023 19:06:08 -0300 |
Fabiano Rosas <farosas@suse.de> writes:
> Add a smoke test that migrates to a file and gives it to the
> script. It should catch the most annoying errors such as changes in
> the ram flags.
>
> After code has been merged it becomes way harder to figure out what is
> causing the script to fail, the person making the change is the most
> likely to know right away what the problem is.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> I know this adds a python dependency to qtests and I'm not sure how
> much we care about this script, but on the other hand it would be nice
> to catch these errors early on.
>
> This would also help with future work that touches the migration
> stream (moving multifd out of ram.c and fixed-ram).
>
> Let me know what you think.
> ---
> tests/qtest/meson.build | 6 +++++
> tests/qtest/migration-test.c | 51 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 57 insertions(+)
>
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 1fba07f4ed..d2511b3227 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -301,6 +301,10 @@ if gnutls.found()
> endif
> endif
>
> +configure_file(input: meson.project_source_root() /
> 'scripts/analyze-migration.py',
> + output: 'analyze-migration.py',
> + configuration: configuration_data())
> +
> qtests = {
> 'bios-tables-test': [io, 'boot-sector.c', 'acpi-utils.c', 'tpm-emu.c'],
> 'cdrom-test': files('boot-sector.c'),
> @@ -356,6 +360,8 @@ foreach dir : target_dirs
> test_deps += [qsd]
> endif
>
> + qtest_env.set('PYTHON', python.full_path())
> +
> foreach test : target_qtests
> # Executables are shared across targets, declare them only the first
> time we
> # encounter them
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 1b43df5ca7..122089522f 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -66,6 +66,8 @@ static bool got_dst_resume;
> */
> #define DIRTYLIMIT_TOLERANCE_RANGE 25 /* MB/s */
>
> +#define ANALYZE_SCRIPT "tests/qtest/analyze-migration.py"
> +
> #if defined(__linux__)
> #include <sys/syscall.h>
> #include <sys/vfs.h>
> @@ -1486,6 +1488,52 @@ static void test_baddest(void)
> test_migrate_end(from, to, false);
> }
>
> +#ifndef _WIN32
> +static void test_analyze_script(void)
> +{
> + MigrateStart args = {};
> + QTestState *from, *to;
> + g_autofree char *uri = NULL;
> + g_autofree char *file = NULL;
> + int pid, wstatus;
> + const char *python = g_getenv("PYTHON");
> +
> + if (!python) {
> + g_test_skip("PYTHON variable not set");
> + return;
> + }
> +
> + /* dummy url */
> + if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", &args)) {
> + return;
> + }
> +
> + file = g_strdup_printf("%s/migfile", tmpfs);
> + uri = g_strdup_printf("exec:cat > %s", file);
> +
> + migrate_ensure_converge(from);
> + migrate_qmp(from, uri, "{}");
> + wait_for_migration_complete(from);
> +
> + pid = fork();
> + if (!pid) {
> + close(1);
> + open("/dev/null", O_WRONLY);
> + execl(python, python, ANALYZE_SCRIPT,
> + "-f", file, NULL);
> + g_assert_not_reached();
> + }
> +
> + assert(waitpid(pid, &wstatus, 0) == pid);
> + if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) != 0) {
> + g_test_message("Failed to analyze the migration stream");
> + g_test_fail();
I just noticed that this is really nice because it fails the test
without aborting, so we get a nice line in the output like this:
▶ 44/355 /x86_64/migration/analyze-script FAIL
which means that if we could replace some asserts with g_test_fail in
the migration code we would actually see what failed without having to
look at the "ok" line in the log and guess what the next test was.
> + }
> + test_migrate_end(from, to, false);
> + cleanup("migfile");
> +}
> +#endif
> +
> static void test_precopy_common(MigrateCommon *args)
> {
> QTestState *from, *to;
> @@ -2828,6 +2876,9 @@ int main(int argc, char **argv)
> }
>
> qtest_add_func("/migration/bad_dest", test_baddest);
> +#ifndef _WIN32
> + qtest_add_func("/migration/analyze-script", test_analyze_script);
> +#endif
> qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
> qtest_add_func("/migration/precopy/unix/xbzrle",
> test_precopy_unix_xbzrle);
> /*