Working to get this tutorial under test, we do a simple refactoring, so that we can test something in isolation. Trivial in this case, but a useful technique worth remembering and practicing. A wild rabbit appears!

It’s really easy to create a test file for any file or class in PyCharm. I could probably create a snippet or editor macro to make it even easier. But it’s super easy even now. All it takes is something like this:

class TestLibClient:
    def test_hookup(self):
        assert False

Voila! We have a test! Not a very good one, but we have busted through the initial resistance that we feel—well, that I feel—to putting a file under test. I am hoping that I can adopt this trick, maybe even find a way to automate it a bit, and then use it to get me started testing things that might otherwise put off.

In my work, which I freely grant is small in scale, I have never regretted writing a test, and I have often regretted not having a test for something that was always wrong, or went wrong during changes. It’s true that sometimes I have to revise tests, because the code evolves and the tests sometimes need to track, and because sometimes I write rather “sloppy” tests, that do the job but aren’t terribly satisfactory, and later on I need or want to change them.

Tests good, no tests not good. You’d think I’d learn.

So maybe this trick will help me do what I’d like to do. Now if there were just a trick for exercise. But I’m not that easily tricked.

Musing
I’m thinking about how best to present what follows. I’m going to browse the Message class and figure out things to test. Part of that figuring out will be identifying things that are relatively easy to test, but that still deserve testing. But you and I can browse code quickly and pick things out, while displaying the code here and talking about it can take pages and pages. I think it must become tedious for the reader. (Hey! That’s you!!)

I’ll try just picking something, displaying it, and talking about why I picked it.

After a delay
I was browsing the server side to get a better sense of what its response looks like, and I fell into a bit of a rabbit hole. The server accepts a message ‘search’, and you can search for ‘morpheus’ or ‘ring’. (Very small database.) I was trying to figure out this code:
request_search = {
    "morpheus": "Follow the white rabbit. \U0001f430",
    "ring": "In the caves beneath the Misty Mountains. \U0001f48d",
}

In particular, what are those weird \U things? Why are they there? Are they critical to formatting the output? So I wandered through the code for a while and then finally just looked up ‘U0001f430’ on the Internet. As I’m sure you knew, because you have these things memorized, that’s the Rabbit Face emoji. (🐰) Thanks for that delay, RealPython.

Recovering from that, I notice this code:

    def write(self):
        if not self._request_queued:
            self.queue_request()

        self._write()

        if self._request_queued:
            if not self._send_buffer:
                # Set selector to listen for read events, we're done writing.
                self._set_selector_events_mask("r")

    def _set_selector_events_mask(self, mode):
        """Set selector to listen for events: mode is 'r', 'w', or 'rw'."""
        if mode == "r":
            events = selectors.EVENT_READ
        elif mode == "w":
            events = selectors.EVENT_WRITE
        elif mode == "rw":
            events = selectors.EVENT_READ | selectors.EVENT_WRITE
        else:
            raise ValueError(f"Invalid events mask mode {mode!r}.")
        self.selector.modify(self.sock, events, data=self)

It turns out that the call to set_selector_events_mask that you see there is the only one. From the viewpoint of a tutorial writer, I can see doing that. In real code, I don’t think I’d do it. I can think of two things I might do:

  1. I might just do the self.selector.modify right in line. Short and sweet.
  2. I might look for other references to self.selector and see whether perhaps we need a little helper class for manipulating selectors. Again, for a tutorial on sockets, I might not do that. But in production code, I’d want to see if this is a hint about an object wanting to be born.

In this case the only other reference to selector is a call to unregister, so there’s certainly no need for an object. But there’s also not much need for that method. Or is there?

If we did go ahead and just put the call directly into write, at some future time there might be a need to set the socket back to read-write, or, conceivably, to write-only. And we might be concerned about duplication or something that would cause us to want to consolidate the encoding.

Honestly, I don’t see it. Good for tutorial, shows me how one might do it. But I don’t love the code.

Does it need testing? Well, it’s certainly conditional logic, which argues for testing. And it’s got just enough duplication that I can imagine getting it wrong.

Just for fun, let’s test it. It’ll be a decent example of how we might test something.

If we were the sort of person who used mock objects, we might configure a Message with a mock selector and test to see whether it gets called with the right events. This is not the way.

We need to extract a method that computes the events mask thing, and test that. This is a machine refactoring, so we trust it to be correct:

    def _set_selector_events_mask(self, mode):
        """Set selector to listen for events: mode is 'r', 'w', or 'rw'."""
        events = self.make_event_mask(mode)
        self.selector.modify(self.sock, events, data=self)

    def make_event_mask(self, mode):
        if mode == "r":
            events = selectors.EVENT_READ
        elif mode == "w":
            events = selectors.EVENT_WRITE
        elif mode == "rw":
            events = selectors.EVENT_READ | selectors.EVENT_WRITE
        else:
            raise ValueError(f"Invalid events mask mode {mode!r}.")
        return events

Now we can test it to our heart’s content. (That’s “content”, not “content”.)

    def test_make_event_mask(self):
        msg = Message(None, None, None, None)
        mask = msg.make_event_mask('r')
        assert mask == selectors.EVENT_READ

And, of course, the others:

    def test_make_event_mask(self):
        msg = Message(None, None, None, None)
        assert msg.make_event_mask('r') == selectors.EVENT_READ
        assert msg.make_event_mask('w') == selectors.EVENT_WRITE
        assert (msg.make_event_mask('rw') ==
                selectors.EVENT_READ|selectors.EVENT_WRITE)

Reflection

OK, I’m sure we all “know” this, but at least one of us, YT, will benefit from some underlining. Often, there are key parts of a method that would be quite easy to test, if only they were untangled from the rest. Our refactoring tools can often do the untangling, and they can be relied upon not to break things. (Relied upon 100%? No, nothing is perfect. But they are quite good.) Even careful manual refactoring can be OK as well, especially if we happen to have some characterization tests to verify that nothing has changed. We do not have those here, by the way, and I’m not sure whether I’ll do one or not. It might not be too difficult. (By not too difficult, I actually mean mot too much of a pain. It wouldn’t actually be difficult.)

Now that we have that method broken out, we might want to improve it. Let’s have a look:

    def make_event_mask(self, mode):
        if mode == "r":
            events = selectors.EVENT_READ
        elif mode == "w":
            events = selectors.EVENT_WRITE
        elif mode == "rw":
            events = selectors.EVENT_READ | selectors.EVENT_WRITE
        else:
            raise ValueError(f"Invalid events mask mode {mode!r}.")
        return events

I really rather dislike that. I would put returns in each branch:

    def make_event_mask(self, mode):
        if mode == "r":
            return selectors.EVENT_READ
        elif mode == "w":
            return selectors.EVENT_WRITE
        elif mode == "rw":
            return selectors.EVENT_READ | selectors.EVENT_WRITE
        else:
            raise ValueError(f"Invalid events mask mode {mode!r}.")

If there were a lot more of those, I can imagine creating a dictionary and looking them up. The code isn’t particularly time critical, and at some small number of elements, a dictionary is faster than a series of if statements. But for these few values, I’m happy enough with this.

The test continues to run, so we’re good. Should we test the exception? We sure should:

    def test_make_event_mask_exception(self):
        msg = Message(None, None, None, None)
        assert pytest.raises(ValueError, msg.make_event_mask, 'x')

And we’ll commit: extracted make_event_mask and tested.

We’ve done some good, we have a sensible-size article. Let’s sum up.

Summary

Had I not decided to test that method, I would very likely not have improved it, but, by my lights, the improvement was worth doing. Testing causes us to think a bit more deeply about something, and that can pay off. When we’re creating the code, it pays off, and it can pay off even if we’re testing after the fact. Thinking is good, it turns out.

In addition, it occurred to me while making my morning iced chai that there was a clue in the code we refactored calling for the refactoring even before we looked at it for testing. Here’s the original again:

    def _set_selector_events_mask(self, mode):
        """Set selector to listen for events: mode is 'r', 'w', or 'rw'."""
        if mode == "r":
            events = selectors.EVENT_READ
        elif mode == "w":
            events = selectors.EVENT_WRITE
        elif mode == "rw":
            events = selectors.EVENT_READ | selectors.EVENT_WRITE
        else:
            raise ValueError(f"Invalid events mask mode {mode!r}.")
        self.selector.modify(self.sock, events, data=self)

There is a principle of code quality called “Composed Method”, which I learned from Kent Beck about a quarter century ago. The idea is that a method’s code should either do one thing or call a number of methods that make up (compose) one idea. So all the lines of a method “should” be at approximately the same level of abstraction.

The method above does two things: it computes an events mask, and it installs it. But the code for installing is one line and the code for computing is eight lines including a possible exception. This is not symmetric, it’s not smooth. So just looking at that code, even if we weren’t trying to make it testable, suggests that this form would be better:

    def _set_selector_events_mask(self, mode):
        """Set selector to listen for events: mode is 'r', 'w', or 'rw'."""
        events = self.make_event_mask(mode)
        self.selector.modify(self.sock, events, data=self)

In that form, we have two calls, roughly addressing the same idea, the events mask. Better by my lights. And, as a bonus, the conditional logic is now testable.

But today’s big reminder, for me, is that I can often begin to bring a legacy class under control, with tests, by safe refactorings, especially machine refactorings, to break out parts of the object that make decisions, but that do not require access to things that are hard to test.

This approach, for me, is more satisfactory than setting up a big story test with real classes, and more satisfactory than setting up mock objects. (I do have to admit that, since I almost always roll my own test doubles, I do get kind of a kick out of doing it, the lightning crackling from my fingertips kind of thing. (Thanks to Kent Beck for that metaphor. Two Beck gifts today. Nice.))

Musing
It occurs to me that it might be amusing to replicate this RealPython example from scratch, using an evolutionary TDD-style of development. It also occurs to me that the tutorial itself is quite long. Showing all the development … might make a decent book. Maybe I won’t try that.

So there we are. Some fun with rabbits 🐰🐰🐰🐰, and then a small refactoring that makes this tutorial object a bit more like the kind of thing we like to have. See you next time!