Code review comment for lp:~adeuring/launchpad/bug-458160-fix-submissionparser-checkconsistency

A two-line change or "real" code together with 100 lines of a new test helper (including tests of the helper) and ~450 lines of changed/improved/fixed tests...

The method SubmissionParser.checkConsistency() in l/c/l/scripts/hwdbsubmissions.py did not specify enough parameters when it called SubmissionParser.checkConsistentUdevDeviceData(). This bug was introduced in a previous branch when I modified the latter method to do more checks which required one more parameter (dmi_data).

The bug in checkConsistency() was not detected because of the way this method was tested: All tests of it created a regular instance of SubmissionParser and then monkey-patched the methods called by checkConsistency(), thus avoiding to set up lots of quite convoluted looking test data required for the non-monkey-patched versions of these methods. The problem is that the surrogate of checkConsistentUdevDeviceData() did not get the new parameter...

I assume that such an error can occur in other tests too, so my idea to avoid a similar problem in the future was this: Don't monkey-patch class SubmissionParser, but use instead a mock class, derived from the real class, and ensure that the methods of mock class have the same signatures as the methods of the base class.

Such a test can be made by calling the new function validate_mock_class() in lp.testing. The test is quite simple: It compares the signature of methods defined in the mock class with the signature of the first method with same name found in the base classes. Even the names of method parameters must not be changed. I think this not a serious problem: having identical parameters names isn't that bad, and allowing differernt names would require a somewhat more complicated comparison, which isn't worth the effort, I think.

the "doc string tests" in lp.testing.__init__ were not tested at all, so I added a test suite for them.