Should I test the debug branch of my code in a unit test?

Question

Lets say I have a function that gets a list of what files to send that are later send over a socket to some other pc. Now usually I keep track of what files have been sent and don't send these files again. For easier coding and debugging I have this:

if debug:
   files_to_send = os.listdir()
   files_to_send.remove(files_sent_log)

otherwise I would have to delete the log every time I want to rerun the program.

Another example: Usually I would want the files to be deleted after they are on the receiving end. But for debugging I want them to stay on the sender, for this I use:

if not debug:
   os.remove(file)

Now my question is should I test this? My intuition is no, since it is only used to help me but not used later when the customer uses the code.

I am using python and this code stays in the production version and can be toggled with a flag in a config file. That's also partly the idea to have only important messages in the log when debug is off and a lot of debug messages when it is on.

Answer 1

If understand correctly you're doing something like this:

public void SendFiles()
{
   var files = _sendDirectory.ListFiles();

   #IF DEBUG
     RemoveTempTextFile();
     CreateFileListingFileNamesToTempTextFile(files);
   #END DEBUG

   foreach(var file in files)
   {
      SendFile(file);
   }
}

I see a couple of problems with this approach.

• Conditional compilation is a powerful tool but as soon as you put that in there's a difference between what you're working with and what you're delivering to your clients. In this case it shouldn't make much of a difference, but when you're developing you should always think about keeping your code as maintainable as possible.
• If every developer drops in random debug files all over the solution then you're going to end up with a mess when it comes to pulling this information all together. Better to have a standard process everyone follows.
• Writing unit tests (as you mentioned in testing in your question) will be very difficult. This code won't be in the builds coming from your build server (which is the most valuable place to run unit tests) so unless you're going to do conditional compilation unit tests... yuck, I'm stopping that idea right there!
• If this information is useful to you in development it could well be valuable to someone diagnosing issues in production. Why deprive them of data which would make their lives easier?

My suggestion would be do do something like this:

public void SendFiles()
{
   var files = _sendDirectory.ListFiles();
   foreach(var file in files)
   {
      try
      {
        _logging.Info($"Sending {file} from {_sendDirectory} to remote server"};
        SendFile(file);
      }
      catch(Exception ex)
      {
        _logging.Error(ex, $"There was an error sending {file} from {_sendDirectory} to remote server"};
      }
   }
}

This not only removes the conditional compilation but provides more information which could be used to diagnose potential issues in production. To answer your original question (about testing) if _log is some kind of ILog object (in .NET I'd use NLog or Log4Net) then it's very easy to DI and unit test that messages (and perhaps more importantly) errors) are being logged.

Answer 2

The idea is right, but the configuration (or rather, the naming in the configuration) is wrong. Having an option to allow the files to be kept on the sending machine could be seen as a feature, and not just a help when debugging. So, change your code to be:

if not configuration.keep_files:
   os.remove(file)

Functionally, the code is the same since you said your debug value was a part of the configuration. But, the intent is clearer and you've added value for your users.

You can make a similar change for your other bit of code to exclude sending files which have already been sent:

if not configuration.send_log:
   files_to_send.remove(files_sent_log)

This decouples the two pieces of code so they can be configured separately, and it can then be easily expanded. For example, maybe you'll want to exclude other files or types of files as well - just add another parameter (or parameters) to your configuration and your code expands easily:

if not configuration.send_log:
   files_to_send.remove(files_sent_log)
if not configuration.send_txt_files:
   files_to_send.remove(all_txt_files)
if not configuration.send_dll_files:
   files_to_send.remove(all_dll_files)

Now, you can freely test each of these configurations as these are configurations that would ship with the product and that users could actually use. No worrying about differences between your tested code and release code.

Answer 3

So there is a feature in your code which is for you exclusively, noone else, and only you can activate it (yes, it is a "debug" feature, but I think it helps not to focus on that term and look at it as just a regular feature for a user). If you want to decide if it is worth to be tested (automatically), you need to ask what the biggest risk is if the feature has a bug:

• if the bug creates an obvious malfunction, you may be forced to fix it before you can use this. But since you have this all under your control, this should not bother you much, you can fix it just at the moment when you need it to work.

• if the bug creates an non-obvious issue, I would say it depends heavily on the details what happens, and how important it is that this does not occur

For the given example of a feature for managing some debug logs, it is IMHO quite unlikely that a malfunction stays undetected, and even if that's the case, I guess the potential impact is quite low. For other such features the situation may look differently.

Note also, if you add such a debug feature, the feature itself may not be worth the effort of creating an automated test, but it may be worth testing that the feature stays correctly invisible when it is configured to be deactivated. So it may be worth to test that in non-debug mode the log files are really deleted.