[PATCH] tests/qtest: add TIMEOUT_MULTIPLIER

Dmitry Frolov posted 1 patch 1 year, 2 months ago
Failed in applying to current master (apply log)
scripts/mtest2make.py | 2 ++
1 file changed, 2 insertions(+)
[PATCH] tests/qtest: add TIMEOUT_MULTIPLIER
Posted by Dmitry Frolov 1 year, 2 months ago
Some tests need more time when qemu is built with
"--enable-asan --enable-ubsan"

As was discussed here:
https://patchew.org/QEMU/20241112120100.176492-2-frolov@swemel.ru/

TIMEOUT_MULTIPLIER enviroment variable will be
a useful option, allowing non-invasive timeouts
increasing for a specific build.

Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
---
 scripts/mtest2make.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
index eb01a05ddb..ff60b62724 100644
--- a/scripts/mtest2make.py
+++ b/scripts/mtest2make.py
@@ -27,7 +27,9 @@ def names(self, base):
 .speed.slow = $(foreach s,$(sort $(filter-out %-thorough, $1)), --suite $s)
 .speed.thorough = $(foreach s,$(sort $1), --suite $s)
 
+ifndef TIMEOUT_MULTIPLIER
 TIMEOUT_MULTIPLIER = 1
+endif
 .mtestargs = --no-rebuild -t $(TIMEOUT_MULTIPLIER)
 ifneq ($(SPEED), quick)
 .mtestargs += --setup $(SPEED)
-- 
2.43.0
Re: [PATCH] tests/qtest: add TIMEOUT_MULTIPLIER
Posted by Daniel P. Berrangé 1 year, 2 months ago
On Wed, Nov 13, 2024 at 12:43:40PM +0300, Dmitry Frolov wrote:
> Some tests need more time when qemu is built with
> "--enable-asan --enable-ubsan"
> 
> As was discussed here:
> https://patchew.org/QEMU/20241112120100.176492-2-frolov@swemel.r/u
> 
> TIMEOUT_MULTIPLIER enviroment variable will be
> a useful option, allowing non-invasive timeouts
> increasing for a specific build.
> 
> Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
> ---
>  scripts/mtest2make.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
> index eb01a05ddb..ff60b62724 100644
> --- a/scripts/mtest2make.py
> +++ b/scripts/mtest2make.py
> @@ -27,7 +27,9 @@ def names(self, base):
>  .speed.slow = $(foreach s,$(sort $(filter-out %-thorough, $1)), --suite $s)
>  .speed.thorough = $(foreach s,$(sort $1), --suite $s)
>  
> +ifndef TIMEOUT_MULTIPLIER
>  TIMEOUT_MULTIPLIER = 1
> +endif

Can you explain what scenario this is needed for, as unless I'm
missing something this change has no purpose. This assignment is
merely defining the defalt value, which can already be overridden
at runtime without this 'ifndef'

eg

$ make check-unit TIMEOUT_MULTIPLIER=7

In another shell:

$ ps -axuwwf | grep 'meson test'
berrange 1931657  3.9  0.1 330904 99344 pts/1    S+   18:29   0:00                  \_ /var/home/berrange/src/virt/qemu/build/pyvenv/bin/python3 /var/home/berrange/src/virt/qemu/build/pyvenv/bin/meson test --no-rebuild -t 7 --num-processes 1 --print-errorlogs --suite unit

shows TIMEOUT_MULTIPLIER being honoured



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 :|
Re: [PATCH] tests/qtest: add TIMEOUT_MULTIPLIER
Posted by Дмитрий Фролов 1 year, 2 months ago
Hello, Daniel

On 26.11.2024 21:32, Daniel P. Berrangé wrote:
> On Wed, Nov 13, 2024 at 12:43:40PM +0300, Dmitry Frolov wrote:
>> Some tests need more time when qemu is built with
>> "--enable-asan --enable-ubsan"
>>
>> As was discussed here:
>> https://patchew.org/QEMU/20241112120100.176492-2-frolov@swemel.r/u
>>
>> TIMEOUT_MULTIPLIER enviroment variable will be
>> a useful option, allowing non-invasive timeouts
>> increasing for a specific build.
>>
>> Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
>> ---
>>   scripts/mtest2make.py | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
>> index eb01a05ddb..ff60b62724 100644
>> --- a/scripts/mtest2make.py
>> +++ b/scripts/mtest2make.py
>> @@ -27,7 +27,9 @@ def names(self, base):
>>   .speed.slow = $(foreach s,$(sort $(filter-out %-thorough, $1)), --suite $s)
>>   .speed.thorough = $(foreach s,$(sort $1), --suite $s)
>>   
>> +ifndef TIMEOUT_MULTIPLIER
>>   TIMEOUT_MULTIPLIER = 1
>> +endif
> Can you explain what scenario this is needed for, as unless I'm
> missing something this change has no purpose. This assignment is
> merely defining the defalt value, which can already be overridden
> at runtime without this 'ifndef'
>
> eg
>
> $ make check-unit TIMEOUT_MULTIPLIER=7
>
> In another shell:
>
> $ ps -axuwwf | grep 'meson test'
> berrange 1931657  3.9  0.1 330904 99344 pts/1    S+   18:29   0:00                  \_ /var/home/berrange/src/virt/qemu/build/pyvenv/bin/python3 /var/home/berrange/src/virt/qemu/build/pyvenv/bin/meson test --no-rebuild -t 7 --num-processes 1 --print-errorlogs --suite unit
>
> shows TIMEOUT_MULTIPLIER being honoured
Yeah... You are right!
It is possible to set TIMEOUT_MULTIPLIER only to run tests.
It is not necessary to set it for the whole build.

Sorry, and thanks a lot!
>
>
> With regards,
> Daniel
regards,
Dmitry

Re: [PATCH] tests/qtest: add TIMEOUT_MULTIPLIER
Posted by Pierrick Bouvier 1 year, 2 months ago
On 11/26/24 23:40, Дмитрий Фролов wrote:
> Hello, Daniel
> 
> On 26.11.2024 21:32, Daniel P. Berrangé wrote:
>> On Wed, Nov 13, 2024 at 12:43:40PM +0300, Dmitry Frolov wrote:
>>> Some tests need more time when qemu is built with
>>> "--enable-asan --enable-ubsan"
>>>
>>> As was discussed here:
>>> https://patchew.org/QEMU/20241112120100.176492-2-frolov@swemel.r/u
>>>
>>> TIMEOUT_MULTIPLIER enviroment variable will be
>>> a useful option, allowing non-invasive timeouts
>>> increasing for a specific build.
>>>
>>> Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
>>> ---
>>>    scripts/mtest2make.py | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
>>> index eb01a05ddb..ff60b62724 100644
>>> --- a/scripts/mtest2make.py
>>> +++ b/scripts/mtest2make.py
>>> @@ -27,7 +27,9 @@ def names(self, base):
>>>    .speed.slow = $(foreach s,$(sort $(filter-out %-thorough, $1)), --suite $s)
>>>    .speed.thorough = $(foreach s,$(sort $1), --suite $s)
>>>    
>>> +ifndef TIMEOUT_MULTIPLIER
>>>    TIMEOUT_MULTIPLIER = 1
>>> +endif
>> Can you explain what scenario this is needed for, as unless I'm
>> missing something this change has no purpose. This assignment is
>> merely defining the defalt value, which can already be overridden
>> at runtime without this 'ifndef'
>>
>> eg
>>
>> $ make check-unit TIMEOUT_MULTIPLIER=7
>>
>> In another shell:
>>
>> $ ps -axuwwf | grep 'meson test'
>> berrange 1931657  3.9  0.1 330904 99344 pts/1    S+   18:29   0:00                  \_ /var/home/berrange/src/virt/qemu/build/pyvenv/bin/python3 /var/home/berrange/src/virt/qemu/build/pyvenv/bin/meson test --no-rebuild -t 7 --num-processes 1 --print-errorlogs --suite unit
>>
>> shows TIMEOUT_MULTIPLIER being honoured
> Yeah... You are right!
> It is possible to set TIMEOUT_MULTIPLIER only to run tests.
> It is not necessary to set it for the whole build.
> 
> Sorry, and thanks a lot!
>>
>>
>> With regards,
>> Daniel
> regards,
> Dmitry
> 

This patch is still useful if we want to set TIMEOUT_MULTIPLIER as 
global env variable, and not explicitely on each make invocation.
Re: [PATCH] tests/qtest: add TIMEOUT_MULTIPLIER
Posted by Philippe Mathieu-Daudé 1 year, 2 months ago
On 27/11/24 19:44, Pierrick Bouvier wrote:
> On 11/26/24 23:40, Дмитрий Фролов wrote:
>> Hello, Daniel
>>
>> On 26.11.2024 21:32, Daniel P. Berrangé wrote:
>>> On Wed, Nov 13, 2024 at 12:43:40PM +0300, Dmitry Frolov wrote:
>>>> Some tests need more time when qemu is built with
>>>> "--enable-asan --enable-ubsan"
>>>>
>>>> As was discussed here:
>>>> https://patchew.org/QEMU/20241112120100.176492-2-frolov@swemel.r/u
>>>>
>>>> TIMEOUT_MULTIPLIER enviroment variable will be
>>>> a useful option, allowing non-invasive timeouts
>>>> increasing for a specific build.
>>>>
>>>> Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
>>>> ---
>>>>    scripts/mtest2make.py | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
>>>> index eb01a05ddb..ff60b62724 100644
>>>> --- a/scripts/mtest2make.py
>>>> +++ b/scripts/mtest2make.py
>>>> @@ -27,7 +27,9 @@ def names(self, base):
>>>>    .speed.slow = $(foreach s,$(sort $(filter-out %-thorough, $1)), 
>>>> --suite $s)
>>>>    .speed.thorough = $(foreach s,$(sort $1), --suite $s)
>>>> +ifndef TIMEOUT_MULTIPLIER
>>>>    TIMEOUT_MULTIPLIER = 1
>>>> +endif
>>> Can you explain what scenario this is needed for, as unless I'm
>>> missing something this change has no purpose. This assignment is
>>> merely defining the defalt value, which can already be overridden
>>> at runtime without this 'ifndef'
>>>
>>> eg
>>>
>>> $ make check-unit TIMEOUT_MULTIPLIER=7
>>>
>>> In another shell:
>>>
>>> $ ps -axuwwf | grep 'meson test'
>>> berrange 1931657  3.9  0.1 330904 99344 pts/1    S+   18:29   
>>> 0:00                  \_ 
>>> /var/home/berrange/src/virt/qemu/build/pyvenv/bin/python3 
>>> /var/home/berrange/src/virt/qemu/build/pyvenv/bin/meson test 
>>> --no-rebuild -t 7 --num-processes 1 --print-errorlogs --suite unit
>>>
>>> shows TIMEOUT_MULTIPLIER being honoured
>> Yeah... You are right!
>> It is possible to set TIMEOUT_MULTIPLIER only to run tests.
>> It is not necessary to set it for the whole build.
>>
>> Sorry, and thanks a lot!
>>>
>>>
>>> With regards,
>>> Daniel
>> regards,
>> Dmitry
>>
> 
> This patch is still useful if we want to set TIMEOUT_MULTIPLIER as 
> global env variable, and not explicitely on each make invocation.

If so, I'd rather use conditional assignment [*]:

- TIMEOUT_MULTIPLIER = 1
+ TIMEOUT_MULTIPLIER ?= 1

[*] 
https://www.gnu.org/software/make/manual/html_node/Conditional-Assignment.html



Re: [PATCH] tests/qtest: add TIMEOUT_MULTIPLIER
Posted by Fabiano Rosas 1 year, 2 months ago
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 27/11/24 19:44, Pierrick Bouvier wrote:
>> On 11/26/24 23:40, Дмитрий Фролов wrote:
>>> Hello, Daniel
>>>
>>> On 26.11.2024 21:32, Daniel P. Berrangé wrote:
>>>> On Wed, Nov 13, 2024 at 12:43:40PM +0300, Dmitry Frolov wrote:
>>>>> Some tests need more time when qemu is built with
>>>>> "--enable-asan --enable-ubsan"
>>>>>
>>>>> As was discussed here:
>>>>> https://patchew.org/QEMU/20241112120100.176492-2-frolov@swemel.r/u
>>>>>
>>>>> TIMEOUT_MULTIPLIER enviroment variable will be
>>>>> a useful option, allowing non-invasive timeouts
>>>>> increasing for a specific build.
>>>>>
>>>>> Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
>>>>> ---
>>>>>    scripts/mtest2make.py | 2 ++
>>>>>    1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
>>>>> index eb01a05ddb..ff60b62724 100644
>>>>> --- a/scripts/mtest2make.py
>>>>> +++ b/scripts/mtest2make.py
>>>>> @@ -27,7 +27,9 @@ def names(self, base):
>>>>>    .speed.slow = $(foreach s,$(sort $(filter-out %-thorough, $1)), 
>>>>> --suite $s)
>>>>>    .speed.thorough = $(foreach s,$(sort $1), --suite $s)
>>>>> +ifndef TIMEOUT_MULTIPLIER
>>>>>    TIMEOUT_MULTIPLIER = 1
>>>>> +endif
>>>> Can you explain what scenario this is needed for, as unless I'm
>>>> missing something this change has no purpose. This assignment is
>>>> merely defining the defalt value, which can already be overridden
>>>> at runtime without this 'ifndef'
>>>>
>>>> eg
>>>>
>>>> $ make check-unit TIMEOUT_MULTIPLIER=7
>>>>
>>>> In another shell:
>>>>
>>>> $ ps -axuwwf | grep 'meson test'
>>>> berrange 1931657  3.9  0.1 330904 99344 pts/1    S+   18:29   
>>>> 0:00                  \_ 
>>>> /var/home/berrange/src/virt/qemu/build/pyvenv/bin/python3 
>>>> /var/home/berrange/src/virt/qemu/build/pyvenv/bin/meson test 
>>>> --no-rebuild -t 7 --num-processes 1 --print-errorlogs --suite unit
>>>>
>>>> shows TIMEOUT_MULTIPLIER being honoured
>>> Yeah... You are right!
>>> It is possible to set TIMEOUT_MULTIPLIER only to run tests.
>>> It is not necessary to set it for the whole build.
>>>
>>> Sorry, and thanks a lot!
>>>>
>>>>
>>>> With regards,
>>>> Daniel
>>> regards,
>>> Dmitry
>>>
>> 
>> This patch is still useful if we want to set TIMEOUT_MULTIPLIER as 
>> global env variable, and not explicitely on each make invocation.
>
> If so, I'd rather use conditional assignment [*]:
>
> - TIMEOUT_MULTIPLIER = 1
> + TIMEOUT_MULTIPLIER ?= 1
>
> [*] 
> https://www.gnu.org/software/make/manual/html_node/Conditional-Assignment.html

I can fix that up in qtest-next, thanks.
Re: [PATCH] tests/qtest: add TIMEOUT_MULTIPLIER
Posted by Дмитрий Фролов 1 year, 2 months ago
Hello, Daniel.

On 26.11.2024 21:32, Daniel P. Berrangé wrote:
> On Wed, Nov 13, 2024 at 12:43:40PM +0300, Dmitry Frolov wrote:
>> Some tests need more time when qemu is built with
>> "--enable-asan --enable-ubsan"
>>
>> As was discussed here:
>> https://patchew.org/QEMU/20241112120100.176492-2-frolov@swemel.r/u
>>
>> TIMEOUT_MULTIPLIER enviroment variable will be
>> a useful option, allowing non-invasive timeouts
>> increasing for a specific build.
>>
>> Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
>> ---
>>   scripts/mtest2make.py | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
>> index eb01a05ddb..ff60b62724 100644
>> --- a/scripts/mtest2make.py
>> +++ b/scripts/mtest2make.py
>> @@ -27,7 +27,9 @@ def names(self, base):
>>   .speed.slow = $(foreach s,$(sort $(filter-out %-thorough, $1)), --suite $s)
>>   .speed.thorough = $(foreach s,$(sort $1), --suite $s)
>>   
>> +ifndef TIMEOUT_MULTIPLIER
>>   TIMEOUT_MULTIPLIER = 1
>> +endif
> Can you explain what scenario this is needed for, as unless I'm
> missing something this change has no purpose. This assignment is
> merely defining the defalt value, which can already be overridden
> at runtime without this 'ifndef'
Without this 'ifndef', TIMEOUT_MULTIPLIER will be overwritten to 1.
With this 'ifndef',  the value, provided by the user will be used.
> eg
>
> $ make check-unit TIMEOUT_MULTIPLIER=7
>
> In another shell:
>
> $ ps -axuwwf | grep 'meson test'
> berrange 1931657  3.9  0.1 330904 99344 pts/1    S+   18:29   0:00                  \_ /var/home/berrange/src/virt/qemu/build/pyvenv/bin/python3 /var/home/berrange/src/virt/qemu/build/pyvenv/bin/meson test --no-rebuild -t 7 --num-processes 1 --print-errorlogs --suite unit
>
> shows TIMEOUT_MULTIPLIER being honoured
>
>
>
> With regards,
> Daniel
With best regards,
Dmitry

Re: [PATCH] tests/qtest: add TIMEOUT_MULTIPLIER
Posted by Fabiano Rosas 1 year, 2 months ago
Dmitry Frolov <frolov@swemel.ru> writes:

> Some tests need more time when qemu is built with
> "--enable-asan --enable-ubsan"
>
> As was discussed here:
> https://patchew.org/QEMU/20241112120100.176492-2-frolov@swemel.ru/
>
> TIMEOUT_MULTIPLIER enviroment variable will be
> a useful option, allowing non-invasive timeouts
> increasing for a specific build.
>
> Signed-off-by: Dmitry Frolov <frolov@swemel.ru>

Tested-by: Fabiano Rosas <farosas@suse.de>

and queued for qtest.
Re: [PATCH] tests/qtest: add TIMEOUT_MULTIPLIER
Posted by Pierrick Bouvier 1 year, 2 months ago
On 11/13/24 13:13, Fabiano Rosas wrote:
> Dmitry Frolov <frolov@swemel.ru> writes:
> 
>> Some tests need more time when qemu is built with
>> "--enable-asan --enable-ubsan"
>>
>> As was discussed here:
>> https://patchew.org/QEMU/20241112120100.176492-2-frolov@swemel.ru/
>>
>> TIMEOUT_MULTIPLIER enviroment variable will be
>> a useful option, allowing non-invasive timeouts
>> increasing for a specific build.
>>
>> Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
> 
> Tested-by: Fabiano Rosas <farosas@suse.de>
> 
> and queued for qtest.
> 

Hi Fabiano,

will a PR be sent for 9.2 release, or later?

Regards,
Pierrick
Re: [PATCH] tests/qtest: add TIMEOUT_MULTIPLIER
Posted by Fabiano Rosas 1 year, 2 months ago
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> On 11/13/24 13:13, Fabiano Rosas wrote:
>> Dmitry Frolov <frolov@swemel.ru> writes:
>> 
>>> Some tests need more time when qemu is built with
>>> "--enable-asan --enable-ubsan"
>>>
>>> As was discussed here:
>>> https://patchew.org/QEMU/20241112120100.176492-2-frolov@swemel.ru/
>>>
>>> TIMEOUT_MULTIPLIER enviroment variable will be
>>> a useful option, allowing non-invasive timeouts
>>> increasing for a specific build.
>>>
>>> Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
>> 
>> Tested-by: Fabiano Rosas <farosas@suse.de>
>> 
>> and queued for qtest.
>> 
>
> Hi Fabiano,
>
> will a PR be sent for 9.2 release, or later?

I queued this for 10.0, it doesn't actually fix anything (only allows
testing to proceed when the variable is set in the cmdline), so it's too
late for 9.2.
Re: [PATCH] tests/qtest: add TIMEOUT_MULTIPLIER
Posted by Pierrick Bouvier 1 year, 2 months ago
On 11/26/24 10:14, Fabiano Rosas wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> On 11/13/24 13:13, Fabiano Rosas wrote:
>>> Dmitry Frolov <frolov@swemel.ru> writes:
>>>
>>>> Some tests need more time when qemu is built with
>>>> "--enable-asan --enable-ubsan"
>>>>
>>>> As was discussed here:
>>>> https://patchew.org/QEMU/20241112120100.176492-2-frolov@swemel.ru/
>>>>
>>>> TIMEOUT_MULTIPLIER enviroment variable will be
>>>> a useful option, allowing non-invasive timeouts
>>>> increasing for a specific build.
>>>>
>>>> Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
>>>
>>> Tested-by: Fabiano Rosas <farosas@suse.de>
>>>
>>> and queued for qtest.
>>>
>>
>> Hi Fabiano,
>>
>> will a PR be sent for 9.2 release, or later?
> 
> I queued this for 10.0, it doesn't actually fix anything (only allows
> testing to proceed when the variable is set in the cmdline), so it's too
> late for 9.2.

Ok thank you!
Just wanted to make sure it does not get lost.

Regards,
Pierrick