[virt-tools-list] [PATCH 01/10] uitests: Improve error message caused by another virt-manager being open

[Povilas Kanapickas]

On 04/09/2018 19:48, Cole Robinson wrote:
> On 09/01/2018 06:17 PM, Povilas Kanapickas wrote:
>> If an instance of virt-manager is open when running UI tests, they will
>> reuse it and will fail on the first action or check with completely
>> non-descriptive message. We can catch errors when initially acquiring
>> the window and provide a better error message.
>>
> 
> Firstly thanks for the patches. I think you are the only other person
> who has attempted to run the ui tests :)
> 
>> Signed-off-by: Povilas Kanapickas <povilas at radix.lt>
>> ---
>>   tests/uitests/utils.py | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/uitests/utils.py b/tests/uitests/utils.py
>> index b9c3c62c..c701d330 100644
>> --- a/tests/uitests/utils.py
>> +++ b/tests/uitests/utils.py
>> @@ -22,6 +22,7 @@ class UITestCase(unittest.TestCase):
>>       """
>>       def setUp(self):
>>           self.app = VMMDogtailApp(tests.utils.URIs.test_full)
>> +
>>       def tearDown(self):
>>           self.app.stop()
>>
> 
> Spurious whitespace change, in general these should be avoided unless
> you are touching that exact area of the code
> 
>> @@ -364,8 +365,15 @@ class VMMDogtailApp(object):
>>           cmd += extra_opts
>>             self._proc = subprocess.Popen(cmd, stdout=stdout,
>> stderr=stderr)
>> -        self._root = dogtail.tree.root.application("virt-manager")
>> -        self._topwin = self._root.find(None, "(frame|dialog|alert)")
>> +
>> +        try:
>> +            self._root = dogtail.tree.root.application("virt-manager")
>> +            self._topwin = self._root.find(None, "(frame|dialog|alert)")
>> +        except RuntimeError:
>> +            msg = "Could not access the top virt-manager window.
>> Maybe " \
>> +                "another instance of virt-manager is active? Please
>> close it " \
>> +                "before running tests"
>> +            raise RuntimeError(msg)
> 
> I hit this issue too, accidentally having the app already running when I
> invoke the tests. Mine usually doesn't fail immediately but instead gets
> some clicking in before failing. Besides being confusing, it's also
> dangerous because depending on the test case it could make changes to
> the host.
> 
> So instead of this patch I pushed one of my own which will check dbus to
> see if virt-manager is already running and immediately fail the test.
> It's the only way to be certain. Let me know if that works for you. I'm
> still reviewing the rest of your patches

Agreed, your solution fixes the issue properly.

Thanks,
Povilas

> commit 40461c58efbdd3435102d299da025ecf46089c6a (HEAD -> master)
> Author: Cole Robinson <crobinso at redhat.com>
> Date:   Tue Sep 4 12:36:50 2018 -0400
> 
>     tests: uitests: Fail if virt-manager is already running
> 
> Thanks,
> Cole