[libvirt] [PATCH] domain:screenshot: Added cleanup function
Added cleanup function to the screeshot testcase. This makes use of the new "sharedmod" module. --- WARNING: don't push this before the patch with sharedmod is pushed in the repo, otherwise this will not work. Thanks. repos/domain/screenshot.py | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 3e727a7..5a12c4b 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -6,6 +6,7 @@ import os import mimetypes import libvirt +import sharedmod required_params = ('guestname', 'filename',) optional_params = ('screen',) @@ -36,6 +37,7 @@ def screenshot(params): logger.debug('Mimetype of the file is %s' % mime) ret = st.finish() + sharedmod.dict['screenshot_filename'] = filename finally: # Some error occurred, cleanup @@ -43,3 +45,7 @@ def screenshot(params): conn.close() return ret + +def cleanup(params): + if sharedmod.has_key('screenshot_filename'): + os.remove(sharedmod['screenshot_filename']) -- 1.7.8.5
On 2012年04月13日 23:44, Martin Kletzander wrote:
Added cleanup function to the screeshot testcase. This makes use of the new "sharedmod" module. ---
WARNING: don't push this before the patch with sharedmod is pushed in the repo, otherwise this will not work. Thanks.
repos/domain/screenshot.py | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 3e727a7..5a12c4b 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -6,6 +6,7 @@ import os import mimetypes
import libvirt +import sharedmod
required_params = ('guestname', 'filename',) optional_params = ('screen',) @@ -36,6 +37,7 @@ def screenshot(params): logger.debug('Mimetype of the file is %s' % mime)
ret = st.finish() + sharedmod.dict['screenshot_filename'] = filename
Is it neccessary to set a shared variable in the same case? IIUC the purpose of sharemod.py, it's for variable sharing among different test cases. Though the case will work fine, but it's just redundant, and let's keep things simple. Osier
On 04/16/2012 04:43 AM, Osier Yang wrote:
On 2012年04月13日 23:44, Martin Kletzander wrote:
Added cleanup function to the screeshot testcase. This makes use of the new "sharedmod" module. ---
WARNING: don't push this before the patch with sharedmod is pushed in the repo, otherwise this will not work. Thanks.
repos/domain/screenshot.py | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 3e727a7..5a12c4b 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -6,6 +6,7 @@ import os import mimetypes
import libvirt +import sharedmod
required_params = ('guestname', 'filename',) optional_params = ('screen',) @@ -36,6 +37,7 @@ def screenshot(params): logger.debug('Mimetype of the file is %s' % mime)
ret = st.finish() + sharedmod.dict['screenshot_filename'] = filename
Is it neccessary to set a shared variable in the same case? IIUC the purpose of sharemod.py, it's for variable sharing among different test cases. Though the case will work fine, but it's just redundant, and let's keep things simple.
Osier
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
We can share just the extension if you want. I can also change it to share it in the module "global" variable. However, the parameter can be used by other tests this way, just in case. We talked about what came to this patch here a little bit: https://www.redhat.com/archives/libvir-list/2012-April/msg00172.html I'm willing to change it to whatever suits the others but I'd like to have it asap so I can finish part of documentation based on this test. Martin
On 2012年04月16日 14:52, Martin Kletzander wrote:
On 04/16/2012 04:43 AM, Osier Yang wrote:
On 2012年04月13日 23:44, Martin Kletzander wrote:
Added cleanup function to the screeshot testcase. This makes use of the new "sharedmod" module. ---
WARNING: don't push this before the patch with sharedmod is pushed in the repo, otherwise this will not work. Thanks.
repos/domain/screenshot.py | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 3e727a7..5a12c4b 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -6,6 +6,7 @@ import os import mimetypes
import libvirt +import sharedmod
required_params = ('guestname', 'filename',) optional_params = ('screen',) @@ -36,6 +37,7 @@ def screenshot(params): logger.debug('Mimetype of the file is %s' % mime)
ret = st.finish() + sharedmod.dict['screenshot_filename'] = filename
Is it neccessary to set a shared variable in the same case? IIUC the purpose of sharemod.py, it's for variable sharing among different test cases. Though the case will work fine, but it's just redundant, and let's keep things simple.
Osier
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
We can share just the extension if you want. I can also change it to share it in the module "global" variable. However, the parameter can be used by other tests this way, just in case.
I don't see any other test case will need the "screenshot_filename" now, if we have some ones in future, change it to shared var at that time then. The principle is not to put the parameter in shared module if it's no need. Osier
On 04/16/2012 10:00 AM, Osier Yang wrote:
On 2012年04月16日 14:52, Martin Kletzander wrote:
On 04/16/2012 04:43 AM, Osier Yang wrote:
On 2012年04月13日 23:44, Martin Kletzander wrote:
Added cleanup function to the screeshot testcase. This makes use of the new "sharedmod" module. ---
WARNING: don't push this before the patch with sharedmod is pushed in the repo, otherwise this will not work. Thanks.
repos/domain/screenshot.py | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 3e727a7..5a12c4b 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -6,6 +6,7 @@ import os import mimetypes
import libvirt +import sharedmod
required_params = ('guestname', 'filename',) optional_params = ('screen',) @@ -36,6 +37,7 @@ def screenshot(params): logger.debug('Mimetype of the file is %s' % mime)
ret = st.finish() + sharedmod.dict['screenshot_filename'] = filename
Is it neccessary to set a shared variable in the same case? IIUC the purpose of sharemod.py, it's for variable sharing among different test cases. Though the case will work fine, but it's just redundant, and let's keep things simple.
Osier
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
We can share just the extension if you want. I can also change it to share it in the module "global" variable. However, the parameter can be used by other tests this way, just in case.
I don't see any other test case will need the "screenshot_filename" now, if we have some ones in future, change it to shared var at that time then. The principle is not to put the parameter in shared module if it's no need.
Osier
OK then, I'll save it in the test and send v2. Martin
--- v2: - removed sharedmod for persistence of the filename repos/domain/screenshot.py | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 82425f3..2761dc5 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -10,6 +10,8 @@ import libvirt required_params = ('guestname', 'filename',) optional_params = ('screen',) +last_filename = None + def saver(stream, data, file_): return file_.write(data) @@ -27,7 +29,7 @@ def screenshot(params): mime = dom.screenshot(st, int(screen), 0) ext = mimetypes.guess_extension(mime) or '.ppm' - filename = params['filename'] + ext + last_filename = params['filename'] + ext f = file(filename, 'w') logger.debug('Saving screenshot into %s' % filename) @@ -37,3 +39,7 @@ def screenshot(params): ret = st.finish() return ret + +def cleanup(params): + if last_filename: + os.remove(sharedmod['last_filename']) -- 1.7.8.5
On 2012年04月16日 17:32, Martin Kletzander wrote:
--- v2: - removed sharedmod for persistence of the filename
repos/domain/screenshot.py | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 82425f3..2761dc5 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -10,6 +10,8 @@ import libvirt required_params = ('guestname', 'filename',) optional_params = ('screen',)
+last_filename = None + def saver(stream, data, file_): return file_.write(data)
@@ -27,7 +29,7 @@ def screenshot(params): mime = dom.screenshot(st, int(screen), 0)
ext = mimetypes.guess_extension(mime) or '.ppm' - filename = params['filename'] + ext + last_filename = params['filename'] + ext f = file(filename, 'w')
logger.debug('Saving screenshot into %s' % filename) @@ -37,3 +39,7 @@ def screenshot(params): ret = st.finish()
return ret + +def cleanup(params): + if last_filename: + os.remove(sharedmod['last_filename'])
Shoud this be the following instead? os.remove(last_filename) ACK with the nit fixed. Regards, Osier
On 04/16/2012 12:14 PM, Osier Yang wrote:
On 2012年04月16日 17:32, Martin Kletzander wrote:
--- v2: - removed sharedmod for persistence of the filename
repos/domain/screenshot.py | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 82425f3..2761dc5 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -10,6 +10,8 @@ import libvirt required_params = ('guestname', 'filename',) optional_params = ('screen',)
+last_filename = None + def saver(stream, data, file_): return file_.write(data)
@@ -27,7 +29,7 @@ def screenshot(params): mime = dom.screenshot(st, int(screen), 0)
ext = mimetypes.guess_extension(mime) or '.ppm' - filename = params['filename'] + ext + last_filename = params['filename'] + ext f = file(filename, 'w')
logger.debug('Saving screenshot into %s' % filename) @@ -37,3 +39,7 @@ def screenshot(params): ret = st.finish()
return ret + +def cleanup(params): + if last_filename: + os.remove(sharedmod['last_filename'])
Shoud this be the following instead?
os.remove(last_filename)
ACK with the nit fixed.
Regards, Osier
Yes, of course, stupid error, sorry. Fixed and pushed. Thanks, Martin
participants (2)
-
Martin Kletzander -
Osier Yang