[Avocado-devel] miniRFC: Call `tearDown` after `setUp` failure

[Lukáš Doktor]

Dne 22.3.2017 v 20:00 Ademar Reis napsal(a):
> On Wed, Mar 22, 2017 at 07:05:48PM +0100, Lukáš Doktor wrote:
>> Hello guys,
>>
>> I remember early in development we decided to abort the execution when
>> `setUp` fails without executing `tearDown`, but over the time I keep
>> thinking about it and I don't think it's optimal. My main reason is to
>> simplify `setUp` code, let me demonstrate it on an example.
>
> So you're basically proposing that setUp() and tearDown() are
> always executed "together". In other words, if setUp() is called,
> then tearDown() will also be called, even if setUp() resulted in
> error, exception or cancellation.
>
> I think it makes perfect sense.
>
> The only problem I see is in the case of test interruption
> (ctrl+c or timeout). I think this will have to be the only
> exception. In case of interruption, we can't wait to run
> tearDown().
>
> In summary, the behavior would be:
>
>  normal execution
>    - test status = PASS | FAIL | WARN
>    - setUp() is run
>    - tearDown() is run
yep

>
>  skip (test runner decision)
this one is tricky, but we can refine it when we have dep solver...

>  skipIf() (decorator)
>    - Test is completely skipped by the test runner
>    - Test status = SKIP
>    - Neither setUp() nor tearDown() are run.
yep

>
>  Uncaught error/exception
>    - test status = ERROR
>    - tearDown() is run
This is the same behavior as PASS | FAIL | WARN, right?

>
>  self.cancel()
>    - Can be called during any stage of the test execution.
>      Results in test status = CANCEL.
>    - tearDown() is run.
Again, the same behavior as PASS | FAIL | WARN.

>
>  Test is interrupted (ctrl+C or timeout)
>    - test status = INTERRUPT
>    - tearDown() is not run
On timeout I agree, on single ctrl+c we wait till the test is over. I 
think it'd make sense to let the tearDown finish as well unless the 
double ctrl+c is pressed, what do you think?

Lukáš

>
> Thanks.
>    - Ademar
>
>>
>> Currently when you (safely) want to get a few remote servers, nfs share and
>> local service, you have to:
>>
>>     class MyTest(test):
>>         def setUp(self):
>>             self.nfs_share = None
>>             self.remote_servers = []
>>             self.service = None
>>             try:
>>                 for addr in self.params.get("remote_servers"):
>>                     self.remote_servers.append(login(addr))
>>                 self.nfs_share = get_nfs_share()
>>                 self.service = get_service()
>>             except:
>>                 for server in self.remote_servers:
>>                     server.close()
>>                 if self.nfs_share:
>>                     self.nfs_share.close()
>>                 if self.service:
>>                     self.service.stop()
>>                 raise
>>
>>         def tearDown(self):
>>             if self.nfs_share:
>>                 self.nfs_share.close()
>>             for server in self.remote_servers:
>>                 server.close()
>>             if self.service:
>>                 self.service.stop()
>>
>>
>> But if the tearDown was also executed, you'd simply write:
>>
>>     class MyTest(test):
>>         def setUp(self):
>>             self.nfs_share = None
>>             self.remote_servers = []
>>             self.service = None
>>
>>             for addr in self.params.get("remote_servers"):
>>                 self.remote_servers.append(login(addr))
>>             self.nfs_share = get_nfs_share()
>>             self.service = get_service()
>>
>>         def tearDown(self):
>>             if self.nfs_share:
>>                 self.nfs_share.close()
>>             for server in self.remote_servers:
>>                 server.close()
>>             if self.service:
>>                 self.service.stop()
>>
>> As you can see the current solution requires catching exceptions and
>> basically writing the tearDown twice. Yes, properly written tearDown could
>> be manually executed from the `setUp`, but that looks a bit odd and my
>> experience is that people usually just write:
>>
>>     class MyTest(test):
>>         def setUp(self):
>>             self.remote_servers = []
>>             for addr in self.params.get("remote_servers"):
>>                 self.remote_servers.append(login(addr))
>>             self.nfs_share = get_nfs_share()
>>             self.service = get_service()
>>
>>         def tearDown(self):
>>             self.nfs_share.close()
>>             for server in self.remote_servers:
>>                 server.close()
>>             self.service.stop()
>>
>> which usually works but when the `get_nfs_share` fails, the remote_servers
>> are not cleaned, which might spoil the following tests (as they might be
>> persistent, eg. via aexpect).
>>
>> Kind regards,
>> Lukáš
>>
>> PS: Yes, the tearDown is unsafe as when `nfs_share.close()` fails the rest
>> is not cleaned up. This is just a demonstration and the proper tearDown and
>> a proper setUp for the current behavior would be way more complex.
>>
>
>
>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 502 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/avocado-devel/attachments/20170323/f0179a09/attachment.sig>