Python task manager from scratch, part 25: Handling a bug

We're implementing task modification in the repository. So far I've been presenting chunks of complete, tests-passing, correct-ish code. (Certainly not totally correct code, but enough to beat back the forces of tech debt.)

Right now I've messed up and need to fix something. Making mistakes is unremarkable, but this is a nice combination of illustrative mistakes.

  1. My test coverage isn't good enough.
  2. I'm stretching an ad hoc "database" to the point where the overhead of its hacky mechanics is affecting my ability to develop the prototype.

I've gone ahead and committed some buggy code. Here's the commit.

When I run python3 main.py and visit 0.0.0.0:8000, I see an error message in my browser, and my terminal reports:

Bottle v0.12.19 server starting up (using WSGIRefServer())... Listening on http://0.0.0.0:8000/ Hit Ctrl-C to quit.

Traceback (most recent call last): File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/bottle.py", line 868 [...] File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/bottle.py", line 1748 [...] File "/Users/nate/Documents/veery/main.py", line 16, in list_tasks return task_display(task_repository.get_all_tasks()) File "/Users/nate/Documents/veery/repositories/task_repository.py", line 74, in get_all_tasks tasks.append(TextFileTaskRepository.task_from_string(task_line.strip())) File "/Users/nate/Documents/veery/repositories/task_repository.py", line 53, in task_from_string status = CompletionStatus(int(separated[-1])) ValueError: invalid literal for int() with base 10: 'NONE'

As I write this, I haven't yet fixed the bug. Here are some things that jump out at me:

  1. 'NONE' is my custom separator.
  2. I am trying to turn something that isn't an int into an int.

This suggests that my deserialization logic is wrong. It's tempting to just go fix the code immediately, because it's probably just a matter of getting an index wrong, but we can turn this bug into capital. Our tests are passing but we have data that's crashing the program. Moreover, we can capture this data straightforwardly:

  1. Examine the stacktrace above to see where the code is failing: task_from_string().
  2. Put a breakpoint into the code there.
  3. Run it again.
  4. Capture the bad input.

This isn't just a matter of creating one more test case; it's fixing the system in response to an actual problem. Things that have actually caused bugs are, on average, vastly more valuable data. And we know it's a hole in our current systems that ensure our software's correctness (such as they are).

When I insert a breakpoint into the code and look at serialized before it breaks the code, I see:

(Pdb) ll
 50         @staticmethod
 51         def task_from_string(serialized: str) -> Task:
 52             separated = serialized.split(TextFileTaskRepository.SEPARATOR)
 53             breakpoint()
 54  ->         status = CompletionStatus(int(separated[-1]))
 55             due = TextFileTaskRepository._task_due_from_string(separated[-2])
 56             uuid = UUID(separated[-3])
 57             description = TextFileTaskRepository.SEPARATOR.join(separated[:-3])
 58             return Task(description=description, due=due, uuid=uuid, status=status)
(Pdb) serialized
'Foo,b1cdc545691f4c83a95720b99ff1d262,NONE'

Notes:

  1. pdb is a debugger. (It's well worth Googling!)
  2. ll tells the debugger to show you the surrounding code.
  3. By typing the name of a variable you can get the debugger to show you the value of the variable.
  4. So the value of serialized is 'Foo,b1cdc545691f4c83a95720b99ff1d262,NONE'.

The culprit is that I'm using data from the old format and trying to serialize / deserialize it with my new logic. The tests are passing because my test repository isn't carrying around my old data.

This is a silly mistake. An essential programming skill is to catch silly mistakes quickly (ideally in seconds or minutes, not hours or days).

How do we fortify our code against this sort of issue? This is, after all, a data integrity or migration process mistake. It's not something that unit tests, such as Pytest makes easy to write, are good at catching. Our options are:

  1. Write explicit tests checking the integrity of our live database.
  2. Ignore the problem.
  3. Put assertions in the code to enforce invariants.

The first of these is a great idea, but more time-efficient to postpone until after we are using a "real" database.

Ignoring the problem is fast, but only a little bit faster than adding asserts into our code.

Adding asserts might cause our program to crash when we don't want it to, but in the prototype stage (i) that's not so bad and (ii) we really do want a lot of crashes when unexpected things happen. (Usually we want that even beyond the prototype stage, but that's another issue.)

So! The tests pass. The repository is updated. Do note the first-order changes here: a helpers/ directory in order to make it easy to mark tasks as completed. Reasonable people will disagree about how best to implement something like this, but I preferred to keep something like this outside of the repository proper and to avoid bloating the repository's interface.

Here's the current commit in the veery/ repository.

Next post: Python task manager from scratch, part 26: Adding task-completion functionality

Home page