In my last post I discussed some exercises put together by Luca Minudel. He was using them as part of a study into how developer’s skill at removing SOLID violations was related to their skill at TDD.
I initially did the exercises in Java, then translated them into Python so we could look at them in our local Python User Group meeting last week. What follows are my own opinions, but I must thank all the pythonistas who were at the meeting, and Andrew Dalke who couldn’t be there but was kind enough to share his opinions and code anyway. What I write below owes a lot to their input. I’m so lucky to be in this community!
We found that in Python, some violations of the Open-Closed Principle are much easier to handle than in Java or C#. You can monkeypatch, and exploit the fact that data is only private by convention. The advantage of being able to get code under test without changing it is of course that you reduce the need for risky refactorings where you unintentionally break the code and get no failing tests to alert you to it.
So the TirePressure example has a violation of the Open-Closed principle where it’s hard to change the specific Sensor used without opening up the existing code. In Python it was dead easy to get under test without modifying the code, because although the sensor is marked as private in the Alarm class, nothing in the language stops you from assigning to it. (See this explanation of how Python considers private data.)
Here’s what (some of) the test code looks like, without making any modifications to the production code: (using the testing framework py.test)
from tire_pressure_monitoring import Alarm
class StubSensor(object):
def __init__(self, pressures):
self.pressures = pressures
def pop_next_pressure_psi_value(self):
return self.pressures.pop()
def test_pressure_in_expected_range_doesnt_trigger_alarm():
alarm = Alarm()
alarm._sensor = StubSensor([18])
alarm.check()
assert not alarm.is_alarm_on()
def test_pressure_below_expected_range_triggers_alarm():
alarm = Alarm()
alarm._sensor = StubSensor([15])
alarm.check()
assert alarm.is_alarm_on()
This means you can quickly get some tests in place. You probably shouldn’t leave the tests like that though, since they are relying on implementation details of the class. This could make them fragile in
the face of refactoring – we’d rather the test only relied on the public interface. These tests also use hard coded numbers where it’s not obvious why those values are chosen – another sign the production code could be improved.
Similarly in the HTMLConverter example, there is a violation of the Open-Closed principle that makes it awkward to have the code read from a string instead of a file. One way to get it under test initially is to use monkeypatching. You just pass in a different implementation of the “open” method that doesn’t in fact open a file, but rather provides a file-like object constructed from a string. Since we have duck typing, the production code doesn’t notice the substitution. You do have to be careful to put the “open” method back to normal at the end of the test though, or the test will have side effects.
So for example:
from cStringIO import StringIO
import unicode_to_html_converter
from unicode_to_html_converter import UnicodeFileToHtmlTextConverter
class StubOpen(object):
def __init__(self, text):
self.text = text
def __call__(self, *args, **kwargs):
return StringIO(self.text)
def test_convert_to_html():
try:
stub_open = StubOpen("text to convert <>&\"\n")
unicode_to_html_converter.open = stub_open
converter = UnicodeFileToHtmlTextConverter("a filename that will be ignored by StubOpen")
# unfortunately this next line is being mangled by my syntax highlighter.
# I think you know what I mean though
assert "text to convert <>&"
" == converter.convert_to_html()
finally:
unicode_to_html_converter.open = open
Again you can quickly get some tests in place, at the cost of a test that is somewhat awkward to read. The other way to get the code under test without changing it is the same as for C# or Java – put the text in a temporary file that is deleted afterwards. This may be simpler to understand, but will be slower to execute. It’s a tradeoff.
I guess the thing with these exercises is that you can get them under test in various ways without correcting the SOLID violations, or the other problems in the code. The idea is that skilled developers will not leave it at that. They will listen to the feedback the tests give them about the code being unecessarily hard to test, and respond by improving the design.
