61

I recently completed a black-box refactoring. I am unable to check it in, because I can't work out how to test it.

At a high level, I have a class whose initialization involves grabbing values from some class B. If class B is "empty", it generates some sensible defaults. I extracted this part to a method that initializes class B to those same defaults.

I have yet to work out the purpose/context of either class, or how they would be used. So I can't initialize the object from an empty class B and check that it has the right values/does the right thing.

My best idea is to run the original code, hardcode in the results of public methods depending on the initialized members, and test the new code against that. I can't quite articulate why I feel vaguely uncomfortable with this idea.

Is there a better attack here?

  • 28
    I feel like you've started in the wrong end. You should first understand the code, then test it, *then* refactor. Why are you refactoring without knowing what the code is for? – Jacob Raihle Feb 23 '17 at 13:15
  • 11
    @JacobRaihle It's a rather specialized program for people with degrees in stuff I've never touched. I'm picking up context as I go, but it's simply not practical to wait to have a solid understanding before I start. –  Feb 23 '17 at 13:19
  • 4
    What's not practical is rewriting things and, when the changes are in production, discovering why you shouldn't have. If you'll be able to test *thoroughly* before then, fine, this can be a good way to get to know the code base. If not, it's *imperative* that you understand before you change. – Jacob Raihle Feb 23 '17 at 13:28
  • 37
    There's a specific kind of testing called [Characterization testing](https://en.wikipedia.org/wiki/Characterization_test) for when you want to test the system's actual behaviour. You just take your original system, then add tests that assert whatever it actually does (and not necessarily what was intended to do!). They serve as a scaffolding around your system, which you can safely modify since you can ensure it retains its behaviour. – Vincent Savard Feb 23 '17 at 13:34
  • 3
    Can't you ask / get it reviewed by someone who *does* understand it? – pjc50 Feb 23 '17 at 20:18
  • If the code is already working, why are you changing it? – David Conrad Feb 24 '17 at 05:39
  • Refactoring code that you don't understand and apparently works fine ... What??? – Num Lock Feb 24 '17 at 05:49
  • "simply not practical to wait to have a solid understanding before I start" To my mind, for a random project selection from the universe of all possible software projects, the chances of this being true are akin to winning the lottery. In the vast majority of cases, its simply not practical NOT to have a solid understanding before one starts – Bradley Thomas Feb 24 '17 at 14:10
  • It's a good idea to do some characterization testing with legacy code, even if you do have a specification, as sometimes there are bugs/quirks in the implementation that are depended upon by consumers of the code, despite it being out-of-spec behavior. With nominally (but poorly) documented code, you may also have specifications that are out of date, with subsequent features added/modified, yet never documented. Caveat implementor. – Dan Bryant Feb 24 '17 at 16:13

2 Answers2

123

You are doing fine!

Creating automated regression tests is often the best thing you can do for making a component refactorable. It may be surprising, but such tests can often be written without the full understanding of what the component does internally, as long as you understand the input and output "interfaces" (in the general meaning of that word). We did this several times in the past for full-blown legacy applications, not just classes, and it often helped us to avoid breaking things we did not fully understand.

However, you should have enough test data and make sure you have a firm understanding what the software does from the viewpoint of a user of that component, otherwise you risk omitting important test cases.

It is IMHO a good idea to implement your automated tests before you start refactoring, not afterwards, so you can do the refactoring in small steps and verify each step. The refactoring itself should make the code more readable, so it helps you to increase your understanding of the internals bit by bit. So the order steps in this process is

  1. get understanding of the code "from outside",
  2. write regression tests,
  3. refactor, which leads to better understanding of the internals of the code
Doc Brown
  • 199,015
  • 33
  • 367
  • 565
  • 22
    Perfect answer, also exactly as described in the book "Working with Legacy Code" – JDurstberger Feb 23 '17 at 15:58
  • I had to do something like this once. Collect typical output data from the application before I modified it, then check my new version of the application by running the same test data through it. 30 years ago... Fortran... It was some kind of image processing / mapping thing, so I couldn't really know what the output 'should' be by looking at it or writing test cases. And, I did it on a Tektronix vector (persistent) display. Government work... 2 Teletypes banging away behind me. –  Feb 23 '17 at 17:02
  • 4
    One might add, you can still write your tests for the old code after the fact. Then you can try them on your refactored version, and if that breaks, do a bisection search through your commit history to find the point where it starts breaking. – CodeMonkey Feb 24 '17 at 11:44
  • 2
    I'd suggest doing one more thing. While collecting the test data, collect code coverage statistics if possible. You will know how well your test data describe the code in question. – liori Feb 24 '17 at 12:49
  • 3
    @nocomprende, It's funny that I did that exact thing with a legacy scientific fortran 77 code last week. Add printing of ascii data to a file, set up test directories with the inputs and expected output, and my test case was just a diff of the two sets of output. If they don't match character for character, I broke something. When the code is mostly two subroutines that are each 2-3k LoC, you have to start somewhere. – Godric Seer Feb 24 '17 at 16:17
1

An important reasons for writing unit tests is that they document the component API somehow. Not understanding the purpose of the code under testing is really a problem here. The code coverage is another important goal, difficult to achieve without knowing which execution branches exist and how are they triggered.

However if it is possible to reset the state cleanly (or construct the new testing object every time), one may write a "trash in-trash out" type tests that just feed mostly random input to the system and observe the output.

Such tests are difficult to maintain, as when they do fail, it may be complex to say why and how serious it is. The coverage may be questionable. However they are still much better than nothing. When such test fails, the developer can revise the latest changes with more attention and hopefully spot the bug there.

h22
  • 905
  • 1
  • 5
  • 15
  • 1
    Any kind of info is better than flying blind. I used to locate bugs in server programs that were in production by invoking the debugger on a crash dump file (Unix) and asking for the stack trace. It gave me the name of the function where the fault occurred. Even with no other knowledge (I didn't know how to use this debugger) it helped in what would otherwise be a mysterious and unreproducable situation. –  Feb 24 '17 at 18:22