Thursday, December 13, 2012

preparing for changes in dependencies

Three or four days ago, I sat down to write a post 'announcing' pytest-moztrap (which I thought I had finished in late September). I figured it'd be a good idea to capture the output of a test run in both quiet and verbose modes for this announcement. In order to make this happen, I went and modified the project's unit tests so that they could be run to report their own results using pytest-moztrap (well duh!).

When I went and ran it, I discovered not one but six bugs!

  1. A change to the MozTrap API had broken moztrap-connect (which pytest-moztrap depends on).
  2. A conflict between moztrap-connect being set to retrieve all objects (no pagination), the number of objects in the database, and the default django throttle was causing cryptic Service Unavailable messages. Alternately, setting limit to a non-zero number without looping on the 'next' pages could cause premature NoSuchObject exceptions.
  3. The tests-within-tests were numbered using their run-case-version IDs rather than their test-case IDs. The contortions the code went through to handle this were kinda insane.
  4. pytest-moztrap does not support the 'Test Series as a template for a Test Run' feature recently implemented (and made the default for new Test Runs) in MozTrap. Nor does MozTrap's API expose it.
  5. The setup.py file for pytest-moztrap did not include a class provided in the repo and required by it, so user installations done with 'python setup.py install' would fail while developer installs done with 'python setup.py develop' (ie, mine) would succeed.
  6. Due to updates to py.test, xfails are not being detected and skips are being handled wrong. 
This got me started thinking about how to prevent #1 and #6 from happening again, or at least notice much sooner if they do. 

Regarding #1, I think moztrap-connect should be included within the main MozTrap repo so it's tests can be run at the same time. If the senior programmer doesn't agree to that, I can arrange some kind of semaphore instead so that I can trigger moztrap-connect test runs off of MozTrap changes. I think the two can co-exist in the same repo because they are installed in different ways. MozTrap is installed by calling 'bin/install-reqs' whereas moztrap-connect is installed via 'python setup.py install'.

Regarding #6, I am certain the owner/maintainer of py.test would not find pytest-moztrap relevant enough to his user-base to include it in the main repo. While it might be possible to arrange a semaphore,  I also had another idea. I could use the two-different install methods to define two different test jobs, one that installs via 'python setup.py install' and the other which installs via 'pip install -Ur requirements.txt'. The setup.py version, which users would use, would have its version pinned to the most recent known good py.test version. The requirements.txt version would install the most recent version. Result, the maintainer knows about changes to the dependency before the users run into it.

While I've already fixed #3 and #5, and I look forward to fixing #4, the upshot of items #1-6 is that I won't be hitting 'publish' on the pytest-moztrap announcement until after all of those bugs are fixed and I've finished the work I started on MozTrap's API for the benefit of the moztrap-tests, moztrap-connect and pytest-moztrap's test fixtures.

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.