Fooling Yourself.

I'm working in a code base now where we frequently see lists of objects pushed into a creational method that often times passes delegates to a persistence method somewhere else. Typically this is some kind of convert JSON strings into database rows activity. 

Yesterday I came across some tests that claimed to validate the insertions. To my sorrow, the validation was bogus. I then spent several hours scanning code looking for this pattern and discovered that our codebase is riddled with this mistake. I then went about fixing these issues. Fortunately I didn't discover any new issues once I had the tests fixed. 

Setup

We have a SqlAlchemy data object named Foo. Foo has two attributes, id and name. Then there is the FooDBWriter that ingests JSON strings and makes instances of the data object. Once that object has been created it gets inserted into the database via the session object.

You can see the entire code suite here. Below I will just show and discuss the code that has problems and how to fix it.


This is a pretty typical pattern in our code base and elsewhere. This is a fine example of an establish micro-pattern in our development environment.

False Positive Tests

Here is a simplification of the issue that I discovered;



The class WorkingFalsePositiveTestOfDBWriter represents the erroneous testing approach. What you can see here is that the developer wanted to check that the session.insert was called once for each model object in the call to write_to_db. For a reason I cannot to fathom, the check is done with mock objects. 

If you aren't familiar with MagicMock, they are part of the Python testing framework and simply put they can be used as programmable mock substitute for any other object that provides reasonable defaults. In this case, __eq__ returns True despite the fact that the objects might not actually be equal to each other. 

So, the WorkingFalsePositiveTestOfDBWriter implementation uses a number of MagicMock instances equal to the number of model objects and checks for equality via the assert_has_calls method. (assert_has_calls is another mocking method that allows you to verify the occurrence of calls on the mock).

The reason this doesn't work is because the MagicMock is always going to return True. So it is possible that the objects being inserted have different values for their fields and the MagicMock will return True in all cases. So that isn't a very complete verification of the behavior.

Authors Intent

I'm guessing at this because the original author does not appear to be available any more. But I think the original approach was something like this;


So you'd hope that an approach like this works. However, our Foo class doesn't define an __eq__ method. I put the test run output in a comment below the assertion, you can see that it claims the results are not equal (despite appearances).

The issue is in how the two lists are compared. Each element of each list is compared with the other by identity, because these aren't the same instances __eq__ returns False. You could get around this by adding an __eq__ method to the data object, but you may have any number of reasons that you don't want to or cannot make that change. [Like an angry goblin that doesn't believe in collective code ownership, or an external library you have no control over]

Working Solution

The following is a more effective means of checking the call list without adding an __eq__ method to the data object. 


This approach uses a matcher class. The matcher class simply wraps the desired object and intercepts the call to __eq__. The matcher's __eq__ method then checks attribute by attribute for equality. (There are some other handy advantages to this approach that I'll address elsewhere). 


Conclusion

So the point here is, be careful with your assumptions when you are writing tests. Understand as much as possible how the comparisons will be made by the testing framework. Validate that your tests really work by tweaking expectations to ensure that the assertions you are making are real. Sometimes you end up fooling yourself; and the consequence is a lack of faith in your test suite.