qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/3] iotests: clean up resources using context m


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH 0/3] iotests: clean up resources using context managers
Date: Mon, 28 Aug 2017 12:55:26 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux)

Stefan Hajnoczi <address@hidden> writes:

> On Fri, Aug 25, 2017 at 03:32:29PM +0800, Fam Zheng wrote:
>> On Thu, 08/24 19:04, Stefan Hajnoczi wrote:
>> > On Thu, Aug 24, 2017 at 04:38:43PM +0800, Fam Zheng wrote:
>> > > On Thu, 08/24 08:21, Stefan Hajnoczi wrote:
>> > > > Tests should declare resources upfront in a with statement.  Resources 
>> > > > are
>> > > > automatically cleaned up whether the test passes or fails:
>> > > > 
>> > > >   with FilePath('test.img') as img_path,
>> > > >        VM() as vm:
>> > > >       ...test...
>> > > >   # img_path is deleted and vm is shut down automatically
>> > > 
>> > > Looks good but still requires test writers to learn and remember to use 
>> > > FilePath
>> > > and with.
>> > 
>> > You cannot forget to use FilePath() unless you love typing at
>> > os.path.join(iotests.test_dir, 'test.img').  It's much better than open
>> > coding filename generation!
>> > 
>> > > These are still boilerplates.  Here goes my personal oppinion, so may
>> > > not be plausible:
>> > > 
>> > > - For VM() maybe add an atexit in the launch() method also makes sure 
>> > > the VM is
>> > >   eventually terminated.
>> > > 
>> > >   This means vm.shutdown() is still needed in tearDown() if there are 
>> > > multiple
>> > >   test methods and each of them expects a clean state, but that is 
>> > > probably
>> > >   still less typing (and also indenting) than the with approach, and 
>> > > also easy
>> > >   to remember (otherwise a test will fail).
>> > 
>> > I looked into atexit before going this route.  atexit does not have an
>> > unregister() API in Python 2.  This makes it ugly to use because some
>> > tests do not want the resource to remain for the duration of the
>> > process.
>> > 
>> > A related point is that the Python objects used by atexit handlers live
>> > until the end of the process.  They cannot be garbage collected because
>> > the atexit handler still has a reference to them.
>> 
>> I think this shortcoming can be solved with a clean up list ("all problems in
>> computer science can be solved by another level of indirection"):
>> 
>> _clean_up_list = set()
>> def _clean_up_handler():
>>     for i in _clean_up_list:
>>         try:
>>             i()
>>         except:
>>             pass
>> 
>> atexit.register(_clean_up_handler)
>> 
>> class VM(...):
>> 
>>     def launch():
>>         ...
>>         _clean_up_list.add(self.launch)
>> 
>>     def shutdown():
>>         _clean_up_list.remove(self.launch)
>>         ...
>
> atexit is still less powerful than context managers because its scope is
> fixed.  Handler functions are only called when the process terminates.
> Many test cases do not want resources (especially the VMs) around
> forever because they run several iterations or sub-tests.
>
> The with statement can be used both for process-lifetime and for more
> fine-grained scoping.  That's why I chose it.

Moreover, context managers are Pythonic.

> If you stick to atexit then sub-tests or iterations require manual
> vm.shutdown() - something that is not necessary using the with
> statement.
>
> Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]