backends: add new test type virttools #3270
Conversation
smitterl
force-pushed
the
virt_tools
branch
2 times, most recently
from
cc850eb to
ec09f0f
Compare
smitterl
force-pushed
the
virt_tools
branch
2 times, most recently
from
640990b to
cadcbff
Compare
|
@chunfuwen @Yingshun @dzhengfy Please, can you help review? |
| include tests-shared.cfg | ||
|
|
||
| variants: | ||
| - @libvirt_quick: |
There was a problem hiding this comment.
could be virttools_quick to syn up with your intention?
There was a problem hiding this comment.
Agree, it should be 'virttools_quick' from my observation in other test types' peer file.
| hvm_or_pv = hvm | ||
| use_os_variant = yes | ||
| use_os_type = yes | ||
| only qcow2 |
There was a problem hiding this comment.
Do we really need below filters when you enable virttools type?
There was a problem hiding this comment.
@chunfuwen In short, yes, I believe they are necessary, at least I prefer to keep them. I tried to run tests without them and I conclude it's better to keep them.
Details:
I believe they are not necessary functionally. However, I've found they have a big impact regarding performance. When I tried test case without these filters, they never started in a reasonable time while I found the cartesian config was still creating its test dictionaries in the background (can be seen in the crash log after ^C).
I assume the reason is that the number of variants explodes without these standard filters.
Please notice that many of these filters usually don't have an impact on the tp-libvirt test functions but are rather tp-qemu specific. I took these filters to be the same as the ones the tp-libvirt/libvirt test type uses, where they possibly aren't functionally necessary either. I'm not sure.
In any case, as mentioned before, without these filters I never got the test to start in a reasonable time, so I propose let's keep them. :)
| include tests-shared.cfg | ||
|
|
||
| variants: | ||
| - @libvirt_quick: |
There was a problem hiding this comment.
Agree, it should be 'virttools_quick' from my observation in other test types' peer file.
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
This file only has a blank line, is it necessary? Can we remove this line? or this file?
There was a problem hiding this comment.
I successfully ran the test case without the file so I removed it :)
Add new test type 'virttools' for test provider 'tp-libvirt'. Signed-off-by: Sebastian Mitterle <[email protected]>
|
@smitterl , some checks were not successful, perhaps you may take a look before it merged |
|
The CI failed on fedora 34 AVOCADO_SRC with error message :ERROR: run() got an unexpected keyword argument 'allow_output_check' (1.89 s), which had been similarly fixed by #3300. |
Adds a new test type 'virttools' intended to be used for tools offered by the 'virt-manager' repository:
A passing test case for this new type is available in autotest/tp-libvirt#3923
Signed-off-by: Sebastian Mitterle [email protected]