|
From: | Thomas Huth |
Subject: | Re: [PATCH] qtest/migration: Add a test for the analyze-migration script |
Date: | Thu, 28 Sep 2023 07:07:16 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 |
On 27/09/2023 23.47, Fabiano Rosas wrote:
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.
Without looking at this too closely, my first thought was: This sounds rather like a good candidate for an avocado test instead. It's using Python, so tests/avocado/ sounds like a better fit. Have you considered adding it as an avocado test already?
>+#define ANALYZE_SCRIPT "tests/qtest/analyze-migration.py" Why can't you use scripts/analyze-migration.py directly? >+ file = g_strdup_printf("%s/migfile", tmpfs);Please, no static file names for temporary files - tests might be running in parallel, and then you get race conditions! Use something like g_file_open_tmp() instead to create a file with a random name.
Thomas
[Prev in Thread] | Current Thread | [Next in Thread] |