python-tuf icon indicating copy to clipboard operation
python-tuf copied to clipboard

Improve subtests clean up

Open sechkova opened this issue 3 years ago • 6 comments

Description of issue or feature request:

In tests we use the home-made decorator @utils.run_sub_tests_with_dataset when we need to execute the same test case consecutively for many datasets. The decorator works perfectly fine when no clean up is needed after each sub test but in the cases where clean up code needs to be executed, the whole sub test needs to be included in a "try - finally" block in order to run the clean up code even if a sub test fails.

Expected behavior: Improve the decorator by possibly including the clean up logic in it to avoid the big "try" blocks in the test code. See https://github.com/theupdateframework/python-tuf/pull/1689#issuecomment-989794850

sechkova avatar Dec 09 '21 14:12 sechkova

another related thing is case_name that is set in the decorator now: it's quite useful but leads to multiple workarounds with both pylint and mypy correctly complaining about it. I think if the decorator is part of a class (something derived from unittest.TestCase) then this would probably be a fixable situation.

jku avatar Dec 13 '21 14:12 jku

@jku I went through your comment and thought of this.

Have a SubTestCase(unittest.TestCase) class which would use an __init__() method to initialize the original function (the one that uses the decorator) and tie it to the class instance and then use a __call__() method to allow for calling the class. __call__() would essentially contain the decorator code and cls.setup_subtest(), cls.teardown_subtest() (which I believe is the cleanup code) would also be managed inside this class.

I would like your thoughts on this.

abs007 avatar May 26 '22 08:05 abs007

Sounds like a reasonable idea. By implementing the __call__ method you will call the decorator and the cls.setup_subtest() logic, but when and how do you call the cls.teardown_subtest() logic? As I understand it we will call the __call__ method with each test case we are given and that's why I am wondering how do you imagine fitting the teardown_subtest logic in that?

MVrachev avatar Jun 09 '22 09:06 MVrachev

Would something like this work? (please excuse me if I'm not making sense logically)

class SubTestCase(unittest.TestCase):
    def __init__(self, orginal_function):
        self.orginal_function = orginal_function
        
    def wrapper():      
        #decorator code
    
    def __call__(self):
        wrapper()
        cls.setup_subtest()
        cls.teardown_subtest()

What I'm thinking is that __call__() won't be called multiple times now since we're putting the decorator code in the wrapper() func.

abs007 avatar Jun 11 '22 17:06 abs007

Would something like this work? (please excuse me if I'm not making sense logically)

class SubTestCase(unittest.TestCase):
    def __init__(self, orginal_function):
        self.orginal_function = orginal_function
        
    def wrapper():      
        #decorator code
    
    def __call__(self):
        wrapper()
        cls.setup_subtest()
        cls.teardown_subtest()

What I'm thinking is that __call__() won't be called multiple times now since we're putting the decorator code in the wrapper() func.

It seems to make sense. You call the decorator and then the setup function. I haven't tested what is first now - the decorator or the setUp function.

Can you choose a testing function using the decorator and link to an example how what it would look like? It will be great if you push an example branch with this change in your fork, so we can have a look closer. Then it would be easier to validate that idea.

MVrachev avatar Jun 20 '22 12:06 MVrachev

Sorry, it looks like I got things mixed up. I thought @jku meant a decorator class by his comment (A class mimicking a decorator function). Some things that I'd like to ask first.

My question is that do we need to pass setup_subtest() inside the setUpClass() method (I'm guessing this is where he meant to put it) since we seem to use it after calling the decorator like in here. I'm saying this since setUpClass() will pass only once before all the test cases are run. Also, will setUpClass() run in the first place if we don't have any test functions inside the class? (which I don't think we're supposed to have since we only have our decorator logic)

abs007 avatar Jul 02 '22 12:07 abs007