[libvirt] [test-API PATCH] Added screenshot test
This patch adds a test that obtains a screenshot of a domain and saves it in a file. --- repos/domain/screenshot.py | 57 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 57 insertions(+), 0 deletions(-) create mode 100644 repos/domain/screenshot.py diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py new file mode 100644 index 0000000..9986cab --- /dev/null +++ b/repos/domain/screenshot.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python +"""This test is used for creating a screenshot of a domain and saving + it in a file. The Screenshot format is hypervisor specific. + + mandatory arguments: guestname + screen + filename +""" + +import os +import mimetypes + +import libvirt + +def check_params(params): + """Verify input parameters""" + for key in ('guestname', 'screen', 'filename'): + if key not in params: + raise KeyError('Missing key %s required for screenshot test' % key) + + params['screen'] = int(params['screen']) + params['filename'] = os.path.abspath(params['filename']) + +def saver(stream, data, file_): + return file_.write(data) + +def screenshot(params): + """This method takes a screenshot of a running machine and saves + it in a filename""" + ret = 1 + try: + logger = params['logger'] + + check_params(params) + + conn = libvirt.open(params['uri']) + dom = conn.lookupByName(params['guestname']) + + st = conn.newStream(0) + mime = dom.screenshot(st, params['screen'], 0) + + ext = mimetypes.guess_extension(mime) or '.ppm' + filename = params['filename'] + ext + f = file(filename, 'w') + + logger.debug('Saving screenshot into %s' % filename) + st.recvAll(saver, f) + logger.debug('Mimetype of the file is %s' % mime) + + ret = st.finish() + + finally: + # Some error occurred, cleanup + if 'conn' in locals() and conn.isAlive(): + conn.close() + + return ret -- 1.7.8.5
On 04/02/2012 08:18 PM, Martin Kletzander wrote:
This patch adds a test that obtains a screenshot of a domain and saves it in a file.
Maybe we shoud later on modify this or add another test to allow comparing the returned image file against a pre-defined pattern, so we could actualy check if the complete stack including the OS, qemu, video drivers and libvirt's streams work.
--- repos/domain/screenshot.py | 57 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 57 insertions(+), 0 deletions(-) create mode 100644 repos/domain/screenshot.py
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py new file mode 100644 index 0000000..9986cab --- /dev/null +++ b/repos/domain/screenshot.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python +"""This test is used for creating a screenshot of a domain and saving + it in a file. The Screenshot format is hypervisor specific. + + mandatory arguments: guestname + screen + filename +""" + +import os +import mimetypes + +import libvirt + +def check_params(params): + """Verify input parameters""" + for key in ('guestname', 'screen', 'filename'): + if key not in params: + raise KeyError('Missing key %s required for screenshot test' % key) + + params['screen'] = int(params['screen']) + params['filename'] = os.path.abspath(params['filename']) + +def saver(stream, data, file_): + return file_.write(data) + +def screenshot(params): + """This method takes a screenshot of a running machine and saves + it in a filename""" + ret = 1
I'm starting to notice this pattern. My test contain something very similar ... a try block, that contains all of the code for testing and then some exception and other cleanup code. I think this pattern will be very common and we should consider moving it one level up, to free the test writers from copying blocks of common code across all test cases.
+ try: + logger = params['logger'] + + check_params(params) + + conn = libvirt.open(params['uri']) + dom = conn.lookupByName(params['guestname']) + + st = conn.newStream(0) + mime = dom.screenshot(st, params['screen'], 0) + + ext = mimetypes.guess_extension(mime) or '.ppm' + filename = params['filename'] + ext + f = file(filename, 'w') + + logger.debug('Saving screenshot into %s' % filename) + st.recvAll(saver, f) + logger.debug('Mimetype of the file is %s' % mime) + + ret = st.finish() + + finally: + # Some error occurred, cleanup + if 'conn' in locals() and conn.isAlive(): + conn.close() + + return ret
Looks good, but I'm not a very experienced Pythonist, so if somebody else (Guannan?) could have a quick look and confirm my ACK before pushing. Peter
On 04/03/2012 09:34 PM, Peter Krempa wrote:
On 04/02/2012 08:18 PM, Martin Kletzander wrote:
This patch adds a test that obtains a screenshot of a domain and saves it in a file.
Maybe we shoud later on modify this or add another test to allow comparing the returned image file against a pre-defined pattern, so we could actualy check if the complete stack including the OS, qemu, video drivers and libvirt's streams work.
Looks good, but I'm not a very experienced Pythonist, so if somebody else (Guannan?) could have a quick look and confirm my ACK before pushing.
Peter
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Thanks for the review. pushed. Guannan Ren
Hi Currently, the testcase parser supports to cleanup testing environment after each testcase finished. we only need to add a flag command 'clean' after each testcase in testcase config, for example: ... domain:screenshot guestname rhel6 filename /tmp/filescreen screen 0 clean domain: start guestname ... the 'clean' flag will make the framework invoke the function named screenshot_clean in screenshot.py. That means each testcase needs two mandatory functions, one is the main function, the other is main function name with '_clean' appended. So I send a patch for this testcase as an example. According to the above testcase config the '/tmp/filescreen.ppm' file will be removed after running. There are some other flags, I need to update the documentation sooner:) --- repos/domain/screenshot.py | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 9986cab..eeda2b5 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -55,3 +55,8 @@ def screenshot(params): conn.close() return ret + +def screenshot_clean(params): + """clean testing environment""" + filename = params['filename'] + os.system('rm -f %s.*' % filename) -- 1.7.7.5 Guannan Ren
On 04/04/2012 07:13 AM, Guannan Ren wrote:
Hi
Currently, the testcase parser supports to cleanup testing environment after each testcase finished. we only need to add a flag command 'clean' after each testcase in testcase config, for example:
... domain:screenshot guestname rhel6 filename /tmp/filescreen screen 0 clean
domain: start guestname ...
the 'clean' flag will make the framework invoke the function named screenshot_clean in screenshot.py. That means each testcase needs two mandatory functions, one is the main function, the other is main function name with '_clean' appended. So I send a patch for this testcase as an example. According to the above testcase config the '/tmp/filescreen.ppm' file will be removed after running. There are some other flags, I need to update the documentation sooner:)
--- repos/domain/screenshot.py | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 9986cab..eeda2b5 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -55,3 +55,8 @@ def screenshot(params): conn.close()
return ret + +def screenshot_clean(params): + """clean testing environment""" + filename = params['filename'] + os.system('rm -f %s.*' % filename)
The extension can be different every time, so we have to check that. I'd prefer something like this: diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 9986cab..c620085 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -39,8 +39,8 @@ def screenshot(params): st = conn.newStream(0) mime = dom.screenshot(st, params['screen'], 0) - ext = mimetypes.guess_extension(mime) or '.ppm' - filename = params['filename'] + ext + params['ext'] = mimetypes.guess_extension(mime) or '.ppm' + filename = params['filename'] + params['ext'] f = file(filename, 'w') logger.debug('Saving screenshot into %s' % filename) @@ -55,3 +55,8 @@ def screenshot(params): conn.close() return ret + +def screenshot_clean(params): + """clean testing environment""" + filename = params['filename'] + params['ext'] + os.remove(filename)
On 04/04/2012 06:30 PM, Martin Kletzander wrote:
On 04/04/2012 07:13 AM, Guannan Ren wrote:
Hi
Currently, the testcase parser supports to cleanup testing environment after each testcase finished. we only need to add a flag command 'clean' after each testcase in testcase config, for example:
... domain:screenshot guestname rhel6 filename /tmp/filescreen screen 0 clean
domain: start guestname ...
the 'clean' flag will make the framework invoke the function named screenshot_clean in screenshot.py. That means each testcase needs two mandatory functions, one is the main function, the other is main function name with '_clean' appended. So I send a patch for this testcase as an example. According to the above testcase config the '/tmp/filescreen.ppm' file will be removed after running. There are some other flags, I need to update the documentation sooner:)
--- repos/domain/screenshot.py | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 9986cab..eeda2b5 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -55,3 +55,8 @@ def screenshot(params): conn.close()
return ret + +def screenshot_clean(params): + """clean testing environment""" + filename = params['filename'] + os.system('rm -f %s.*' % filename)
The extension can be different every time, so we have to check that. I'd prefer something like this:
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 9986cab..c620085 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -39,8 +39,8 @@ def screenshot(params): st = conn.newStream(0) mime = dom.screenshot(st, params['screen'], 0)
- ext = mimetypes.guess_extension(mime) or '.ppm' - filename = params['filename'] + ext + params['ext'] = mimetypes.guess_extension(mime) or '.ppm'
This modification on params couldn't be passed in screenshot_clean() The params to screenshot_clean() is the same as the screenshot() which is from testcase config file.
+ filename = params['filename'] + params['ext'] f = file(filename, 'w')
logger.debug('Saving screenshot into %s' % filename) @@ -55,3 +55,8 @@ def screenshot(params): conn.close()
return ret + +def screenshot_clean(params): + """clean testing environment""" + filename = params['filename'] + params['ext'] + os.remove(filename)
On 04/04/2012 01:23 PM, Guannan Ren wrote:
On 04/04/2012 06:30 PM, Martin Kletzander wrote:
--- repos/domain/screenshot.py | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 9986cab..eeda2b5 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -55,3 +55,8 @@ def screenshot(params): conn.close()
return ret + +def screenshot_clean(params): + """clean testing environment""" + filename = params['filename'] + os.system('rm -f %s.*' % filename) The extension can be different every time, so we have to check that. I'd
On 04/04/2012 07:13 AM, Guannan Ren wrote: prefer something like this:
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 9986cab..c620085 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -39,8 +39,8 @@ def screenshot(params): st = conn.newStream(0) mime = dom.screenshot(st, params['screen'], 0)
- ext = mimetypes.guess_extension(mime) or '.ppm' - filename = params['filename'] + ext + params['ext'] = mimetypes.guess_extension(mime) or '.ppm'
This modification on params couldn't be passed in screenshot_clean() The params to screenshot_clean() is the same as the screenshot() which is from testcase config file.
Darn :( That's exactly why I wanted the parameter passing between tests :) What do you suggest? Should I save the extension into another file or do we have any other option?
On 04/04/2012 07:53 PM, Martin Kletzander wrote:
On 04/04/2012 01:23 PM, Guannan Ren wrote:
On 04/04/2012 06:30 PM, Martin Kletzander wrote:
--- repos/domain/screenshot.py | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 9986cab..eeda2b5 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -55,3 +55,8 @@ def screenshot(params): conn.close()
return ret + +def screenshot_clean(params): + """clean testing environment""" + filename = params['filename'] + os.system('rm -f %s.*' % filename) The extension can be different every time, so we have to check that. I'd
On 04/04/2012 07:13 AM, Guannan Ren wrote: prefer something like this:
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 9986cab..c620085 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -39,8 +39,8 @@ def screenshot(params): st = conn.newStream(0) mime = dom.screenshot(st, params['screen'], 0)
- ext = mimetypes.guess_extension(mime) or '.ppm' - filename = params['filename'] + ext + params['ext'] = mimetypes.guess_extension(mime) or '.ppm'
This modification on params couldn't be passed in screenshot_clean() The params to screenshot_clean() is the same as the screenshot() which is from testcase config file.
Darn :( That's exactly why I wanted the parameter passing between tests :) What do you suggest? Should I save the extension into another file or do we have any other option?
The idea to share variable is reasonable, we can create a shared_mod.py in root of test-API, then import it in all of testcases, The first testcase put the value in this module for sharing with other modules. the connection object, domain network .etc could be shared in this way. Except these fixed share object. we could define 3 or 5 customized shared variable for use. The contents of shared_mod.py should be like this: #shared_mod.py con = None domobj = None netojb = None stgobj = None ... defined_var1 = None defined_var2 = None defined_var3 = None Guannan Ren
On 04/04/2012 03:10 PM, Guannan Ren wrote:
On 04/04/2012 07:53 PM, Martin Kletzander wrote:
On 04/04/2012 01:23 PM, Guannan Ren wrote:
On 04/04/2012 06:30 PM, Martin Kletzander wrote:
--- repos/domain/screenshot.py | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 9986cab..eeda2b5 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -55,3 +55,8 @@ def screenshot(params): conn.close()
return ret + +def screenshot_clean(params): + """clean testing environment""" + filename = params['filename'] + os.system('rm -f %s.*' % filename) The extension can be different every time, so we have to check that. I'd
On 04/04/2012 07:13 AM, Guannan Ren wrote: prefer something like this:
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 9986cab..c620085 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -39,8 +39,8 @@ def screenshot(params): st = conn.newStream(0) mime = dom.screenshot(st, params['screen'], 0)
- ext = mimetypes.guess_extension(mime) or '.ppm' - filename = params['filename'] + ext + params['ext'] = mimetypes.guess_extension(mime) or '.ppm'
This modification on params couldn't be passed in screenshot_clean() The params to screenshot_clean() is the same as the screenshot() which is from testcase config file.
Darn :( That's exactly why I wanted the parameter passing between tests :) What do you suggest? Should I save the extension into another file or do we have any other option?
The idea to share variable is reasonable, we can create a shared_mod.py in root of test-API, then import it in all of testcases, The first testcase put the value in this module for sharing with other modules. the connection object, domain network .etc could be shared in this way. Except these fixed share object. we could define 3 or 5 customized shared variable for use. The contents of shared_mod.py should be like this:
#shared_mod.py con = None domobj = None netojb = None stgobj = None ... defined_var1 = None defined_var2 = None defined_var3 = None
Guannan Ren
I had an idea with expanding the parameters, tests could then share whatever they want, but this would break current tests when not handled correctly. I think your idea can be used now, just to fix these kind of things and we can treat the expansion into more sophisticated code as a major change and implement it after some release. Martin
On 04/04/2012 09:43 PM, Martin Kletzander wrote:
On 04/04/2012 03:10 PM, Guannan Ren wrote:
On 04/04/2012 07:53 PM, Martin Kletzander wrote:
On 04/04/2012 01:23 PM, Guannan Ren wrote:
On 04/04/2012 06:30 PM, Martin Kletzander wrote:
--- repos/domain/screenshot.py | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 9986cab..eeda2b5 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -55,3 +55,8 @@ def screenshot(params): conn.close()
return ret + +def screenshot_clean(params): + """clean testing environment""" + filename = params['filename'] + os.system('rm -f %s.*' % filename) The extension can be different every time, so we have to check that. I'd
On 04/04/2012 07:13 AM, Guannan Ren wrote: prefer something like this:
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 9986cab..c620085 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -39,8 +39,8 @@ def screenshot(params): st = conn.newStream(0) mime = dom.screenshot(st, params['screen'], 0)
- ext = mimetypes.guess_extension(mime) or '.ppm' - filename = params['filename'] + ext + params['ext'] = mimetypes.guess_extension(mime) or '.ppm' This modification on params couldn't be passed in screenshot_clean() The params to screenshot_clean() is the same as the screenshot() which is from testcase config file.
Darn :( That's exactly why I wanted the parameter passing between tests :) What do you suggest? Should I save the extension into another file or do we have any other option? The idea to share variable is reasonable, we can create a shared_mod.py in root of test-API, then import it in all of testcases, The first testcase put the value in this module for sharing with other modules. the connection object, domain network .etc could be shared in this way. Except these fixed share object. we could define 3 or 5 customized shared variable for use. The contents of shared_mod.py should be like this:
#shared_mod.py con = None domobj = None netojb = None stgobj = None ... defined_var1 = None defined_var2 = None defined_var3 = None
Guannan Ren
I had an idea with expanding the parameters, tests could then share whatever they want, but this would break current tests when not handled correctly. I think your idea can be used now, just to fix these kind of things and we can treat the expansion into more sophisticated code as a major change and implement it after some release.
Martin
Okay, I will send patches for this as soon as possible. Guannan Ren
participants (3)
-
Guannan Ren -
Martin Kletzander -
Peter Krempa