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.






Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to