Saturday, December 8, 2012

abstract test cases and test harnesses / collection

One thing that bothers me in both developers' unit tests and QA's automated tests is duplication in test code. Test cases that follow roughly the same 'script', but act on different elements, and need differing information about these objects. I often see projects where the same test case is copy-and-paste-ed from file to file, changing variables names and values as you go. I regularly see projects using base test classes to provide tools common to many test cases (methods which do not match 'test*'). I also see QA teams using the Page Object Method in their web (Selenium) tests. I have never seen a project I didn't write use base classes to provide actual tests.

I recently started working on providing create, update, and delete capabilities to an API that so far was only doing the 'read' part of CRUD. I know that each of the endpoints (there are 6 or 7) is going to have the following test cases

  • create X
  • create X fails with wrong permissions
  • read list of Xs (permissions not required)
  • read detail of X (permissions not required)
  • update detail of X
  • update detail of X fails with wrong permissions
  • update list of X forbidden
  • delete X
  • delete of X fails with wrong permissions
  • delete list of X forbidden

Most test harnesses I know of collect test methods starting with (or including) the word 'test' out of classes that include the word 'Test', out of files matching 'test*.py' (or some other extension), and that pattern seemed to be born out by how the existing tests were arranged. So I put my abstract class called ApiCrudCases in tests/case/api/crud.py.

Then I tried to run the tests, and discovered that the test harness was collecting 'test_create' etc. from both the abstract class (undesired) and the child class (desired). It was clear why the test runner would pick up the test methods with names matching 'test*', but I was mystified why the it was trying to run the test cases in this abstract class...I had named it 'crud.py', not 'test_crud.py'. I checked the documentation / code.

in django.utils.unittest.loader:
    def discover(self, start_dir, pattern='test*.py', top_level_dir=None):
        ...
        tests = list(self._find_tests(start_dir, pattern))

    def _match_path(self, path, full_path, pattern)
        # override this method to use alternative matching strategy
        return fnmatch(path, pattern)

It wasn't until later in the afternoon (after I had already coded a workaround) that I realized it was because 'test*.py' also matches 'tests/case/api/crud.py'.

My question is, is this a bug or a feature? Is my abstract test class methodology a valuable pattern or a stinky test smell? Is this a use case that the developers of the test harness hadn't thought of, or is it an attempt to keep me from doing something stupid?

It is obviously a good thing for projects to have all of its tests in a single directory, both in terms of inclusion for running tests and in terms of exclusion of those directories for coverage. It is also logical to name that directory 'test[s]', because it is a great standardized name for contributors to use to find the tests. It also makes sense for my abstract test class to be located in the test hierarchy  since it doesn't need coverage run on it.

It took me a couple of tries to work around this issue. Both the new way of indicating abstract method / property (abc.AbcMeta, @abstractmethod, @abstractproperty) and the old way (NotImplementedError) were causing the test runner to report Error for each test case. I tried using a decorator to tell the test harness to skip the whole class, but was unable to introspect into either the class nor the methods to determine if it was the abstract class (to be excluded) or a child class (to be included).

What I eventually ended up using was NotImplementedError, an instance property called is_abstract that would compare self.__class__.__name__ to the abstract class's name, and returning early from each of the test method if is_abstract() was true.

Now I'm off to find out how much giggering I'd need to do to the test harness in order to get it to look at the file name rather than the file path when matching, and talk to the senior developer about which solution to use.

[Edit] The senior developer voted to keep the workaround rather than create a more specific test runner.

No comments:

Post a Comment