2

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.

  • 5
    IMHO if debug statements in any language are usually a code smell. This is usually log levels and configuration are for – Liath Jun 20 '18 at 10:00
  • I use log levels and configurations, and in the config file there is a parameter debug. Can you explain what is wrong with this simple example? Or how to do it better? – Hakaishin Jun 20 '18 at 10:07
  • 1
    Think about it. You have 2 different behaviors for the same code. The only difference is what mode you are running in (debug or not). Your unit tests should be written to cover the behavior of what that method is responsible for, but your debug code can break the non-debug code. That is building fragility into your application. – Berin Loritsch Jun 20 '18 at 16:14
  • 3
    This kind of approach is something I have learned (the hard way) not to do. If all you need is "to delete the log every time I want to rerun the program", put that in a script that you use to run or create a new python module that does this before running the actual program. – JimmyJames Jun 20 '18 at 16:38
5

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.

  • I have another if debug that prevents files from being deleted. 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. Now I fail to see how your approach would work for that case? It is not that deleting the file gives an error it is just that I want to do this 'if not debug: os.remove(my_file)' depending if I'm in debug mode or not. – Hakaishin Jun 20 '18 at 10:55
  • I should apologise that I've answered in something resembling C#... I don't speak python especially well – Liath Jun 20 '18 at 10:56
  • No problem, the idea counts and I got the idea, I just think it is not applicable in my case. – Hakaishin Jun 20 '18 at 10:58
1

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.

  • Ok, while I do like this answer and it does seem correct in theory, it feels like I'm unnecessarily adding variables, configurations and passing them around without using them too much. Usually I don't want the things that are in the debug branch done seperatly, but all together. Especially since it is really mostly for me, nearly never for the user, except for better log messages. But as I understand this answer it does advice on testing the "debug"/configuration part of the code too? – Hakaishin Jun 20 '18 at 15:57
1

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.