The FOOLPROOF Way To Test Your Fix In Legacy Code

so no more saying I can't add tests and instead we should be asking is it worth it as software Engineers we hear this kind of thing all the time when it comes to things like bug fixes someone will get the bug fixed they'll have the code all ready to go and then when someone's checking out the code and the code r view or the pull request they say well where are all the tests for this and then that person responds well the fix is in the Legacy code it's just not testable and maybe this person is you sometimes so in this video I'm going to talk about a different mindset shift that we can go into scenarios like this to make anything more testable and the goal here is really to eliminate that whole argument of not being able to add test in the first place so no more saying I can't add tests and instead we should be asking is it worth it before I dive in just remember to check that pin comment to subscribe to my free weekly software engineering newsletter it's totally free and it's jam-packed with awesome content and we're going to start this one by looking at some block diagrams and fortunately for you I didn't draw these ones in Ms paint oh wait that's the wrong window all right that's much better so what I have on my screen right now is just a bunch of different blocks that represent components in a system and I want to take this perspective cuz we're going to zoom in a little bit and talk about what happens when we have an issue so the way that I have this colorcoded and sorry if it's a little bit confusing but the components a b and F are this green color and that's going to indicate that they have pretty good test coverage we're really confident with the code in there because there's unit tests maybe there's functional tests but overall these are components that we feel good about we have confidence if we're making changes there the orange components might represent parts of our system that are something like Legacy code maybe they have missing tests or really crappy test coverage it could be that there's tons of spaghetti code and it's really difficult to add tests this is kind of for you to think through and decide what types of limitations there are because there's countless in any real system that has code that's existed for a while it's very likely there's going to be parts of it that are deteriorating a little bit and if you've worked in systems for long enough you'll probably notice this so I just want you to use your imagination here and think about a system that you've worked in and maybe some of these orange components are the ones that you've had some challenges with in the past you can think through some scenarios where there were bug fixes you had to do or maybe you were adding features and it was difficult to work in these because they didn't have good test coverage and for this video I'm not going to be dictating what's good or bad test coverage I just want you to think about testing as a tool to build confidence this is always what I go back to when I talk about testing so if you're thinking about unit test coverage or functional test coverage to me it doesn't really matter and it's not super important for this video for me to tell you one way or the other but just think about enough test coverage to have confidence and for the orange components we don't have that confidence but let's talk about this problem scenario that we have so big surprise in this component e that was orange before this is a component that had poor test coverage maybe it's some Legacy code and this is the component where our bug fix is located so again think through this using your imagination you're working in your code base and you fix that bug and you're putting that code up for review and someone's saying hey where are your tests for this and you give them that age-old response of oh this is just the Legacy code you know we can't really test this but I did fix the bug I'm very confident but we're just going to have to push it and assume that it works and to pause for a moment when I'm talking about code that is not testable I don't mean that we couldn't spend tons of time refactoring it to make it testable I want you to think about situations where you need to get this fix ready for production it might be a critical bug fix or something else maybe it isn't even time sensitive but it's really important to deliver it and there's other priorities that are stacked up so in situations where the bug fix was either straightforward or small enough if you're thinking about that effort compared to the amount of work that would have to go into refactoring this entire component it might be such that that refactoring effort is days or weeks of effort compared to this bug fix given the other priorities that are in front of us we're kind of looking at it like look we got to get this fix in and we got to get to these other priorities as well we know that this is some Legacy code or it's like spaghetti code we know that there's poor test coverage and we know that we need to invest time into it but perhaps right this moment is not the time this is the kind of situation that I want you to think through here because if you had unlimited time we would probably just press pause and we'd say cool let's go refactor this code then do the bug fix properly and then go add the test in because we refactored it but a lot of the times in real life we don't have luxuries like that so this technique that I'm going to show you is just something that you can leverage to be able to get more confidence than zero and not necessarily have to go spend days and weeks refactoring all of this Legacy code just to be able to do that let me jump back over to the diagrams if we think about component e which is the component that needed the bug fix if we take this idea of this little diagram that we have with all the components and if we imagine zooming into component e we might have another similar diagram where it's composed of other pieces now we said the component e is not testable and if you take the idea of zooming in further and further into component e and decomposing it into smaller components the part that we usually try to think through is how far can we zoom in to be able to get something that's coverable with test but we're going to shift the Paradigm here I don't want us to think about trying to keep zooming in until we finally find the parts of component e that are testable instead I want us to acknowledge the fact that we're saying component e is not testable we know that that code might be crappy it might be Legacy it might be spaghetti it might be whatever so we're going to say cool we get it we can't test component e but we do want to have some type of confidence over the code that we touched and a lot of the time when we have stuff that's buried deep inside a component I want want you to think about maybe a method that's inside of a class and you had to go tweak something when that stuff is buried really deep so we're somewhere really deep inside of this component e and that's where our bug fix is it makes it really difficult for us to write coded tests over it because maybe component e isn't testable easily like we've acknowledged here and that might mean that to write tests for this we have to go set up a configure component a component B maybe some other components that are going to interface with the component e so that we can finally go exercise that and by the time we do that we might not have the right tooling to be able to see that we're executing the change inside of component e that we added so like I said we're going to acknowledge that we're not going to be able to make component e more testable at least not as a whole what we are going to do is focus on making our single change testable instead and the way that we do this is quite simple and what we're going to do is simply extract the parts of the code that we touched seriously it's that straightforward when we can extract the parts that we touched we can take a shortcut we can instantiate the class that we're going to create inside of the Constructor of the object that we're already dealing with so we can skip all the dependency injection if we want and just new it up inside the Constructor and once we have that code pulled out into its own class from there we can write tests on that class we can expose that logic as a public member on that class and if that's making the hair on your arms stand up I get it but the point here is that you were saying we couldn't test anything inside of component e right you said it's Legacy code it's spaghetti we can't possibly write tests on this and I'm saying yeah you can what I'm not saying is that you always should and this is important for us to think about because just because we can does not mean we should if this is a critical fix and you need to have some amount of additional confidence I would say you might want to think about how you can prove prove to yourself people on your team maybe the product owner that you have some type of logic that's being exercised as part of a test that proves the change in Behavior now that's some confidence and it's not full confidence it's not full automation coverage on component e like we were talking about right we're changing the Paradigm here we're saying component e is not testable let's zoom in specifically on our change pull that out and write test on that code that means we're adding some confidence on that change but what it doesn't mean is that we're necessarily designing a system to be composed of the right pieces and this taken to an extreme can lead to a lot of little pieces that are pulled out so if you're going to take this approach and continue to repeat it I would say maybe pause a little bit and see if you're starting to extract some of these classes that can be logically grouped otherwise if you keep repeating this you might end up with a ton of little classes and all of these single methods on them and things are really disjoint but the point is you got more test coverage on your change I'm going to jump over to visual studio now in a project that I have take a random spot and then see if we can just pull out some code and pretend that we have a bug fix and how we'd go WR tests on it all right I'm in my backend code for my meal coach asp.net core application I have some authentication code here that interacts with AWS Cognito and I'm just going to take this random part here highlight it on my screen and let's assume that there's a bug here let's assume that we have the wrong client ID so we have something like this is where the bug is right so this code was here and when we're debugging we realize oh crap this part here on line 417 that's the wrong thing so we come through and we go make this change to have the right thing in place and then we say well this code that Nick wrote is totally untestable Nick has really terrible design tendencies and as a result this was built up over time and it's completely unun testable and writing functional tests on this would be a total nightmare but how can we get some coverage just on this how do we skip trying to test this huge class it's over a thousand lines long because Nick sucks at coding how do we get this covered instead of trying to make the entire thing more testable and what I said was that we could pull this out into its own class so I'm just going to do this by highlighting the code and then I'm going to extract it with the visual studio tools for debugging to pull it out into a method once I'm happy with the name I'll press enter and if I scroll down a little bit you can see that it pulled all that code out right here the next step that I'm going to do is cut this completely out and I'm going to add it to my own class just to keep it really simple I'm going to add it at the top of this file so that we can talk about it only for this video I've gone ahead and made a class but I'm going to have to give it a name and what I've done is I pasted the code into here and two things that we have to do at this point now are give this a class name that's not ridiculous as well as get access to this Cognito config well this Cognito config was passed in via the Constructor in the other class that we were in so we can do the same thing here all right now that I have the Cognito config passed into the Constructor we just have to give this a name that doesn't suck I'm going to go ahead and call it something like Cognito request Factory is that a perfect name probably not but I think it'll be good enough for this video but our codee's not going to compile just yet because we took that method completely out and now it doesn't have access to it in the original spot so what I'm going to do is take this class here and then we're going to instantiate it inside of our Constructor and yep that's the Constructor it's got a ton of stuff already passed in Via dependency injection which is a great indicator that this whole thing needs to be decomposed but that's a story for another day so instead of adding to the this dependency injection mess I'm just going to instantiate an instance in here I'm still going to have to go declare this variable that's underscore Cognito request Factory but you can see that I'm instantiating it right here and because we had the Cognito config passed in Via dependency injection already we have access to it and I wanted to highlight that I don't want to go chasing dependency injection for this change because I'm trying to minimize the footprint of my change personally if I were creating this class or if was taking my time to do all of this I wouldn't new up this dependency inside here I would go to dependency injection I would eventually pass it in as another parameter that's on here instead but the reason I'm not doing that is because every additional change we're making to make this more testable is another chance that we could be introducing yet another bug so instead of doing that I'm going to keep it very scoped to just having a new instance of this so I need to go declare that field now with my field created all we have to do now is call it from the original spot and on line 442 I've gone ahead and added that call to the Cognito request Factory create Cognito login request and one thing that I missed is that when I extracted the method I left it as a private method so I just had to make it public if I jump back over to it you can see that I've gone ahead and made that quick little change here so earlier in the video it was left as private so great we have this now pulled out and it's an internal sealed class it has a public method on it and it has one single dependency that we could easily go mock if we needed to so if we scroll down a little bit we can see that when we're creating a request in here we do need to access a Cognito config a Cognito config is just a data transfer object a dto so we could go mock it out but it's a little bit Overkill we could probably just pass in a real instance of it and therefore just use a functional test instead of mocking things out and having a unit test in either case we're essentially just testing one single unit of this class so whether or not you're using mock or using the real instance of the dto I feel like it's your choice the point here is that we can now easily Go cover this code with a test and this code that we're looking at this line right here that has client ID equal to Cognito config userpool client ID is exactly where the bug is so if you were the person that I was trying to prove this fixed to what we could do is go write the test on here and assert that have the right client ID on the request when we go to have this instance returned we could write a failing test that would go check for the right Cognito client ID and instead we would get this bug value passed back so our test would fail if you and I were pair programming on this we could go yep that's totally the test that's catching this bug then we could go change it so that we have the right value here this userpool client ID and then our test would end up passing now did we make the rest of this Cognito authentication Service any more testable absolutely not we didn't focus on that one bit and in fact we tried to minimize the amount of change to it instead we just simply extracted the part that was giving us a bit of pain and then we were able to talk about writing tests strictly over this and like I said this approach is super simple but that doesn't mean that you should always go use it I do want you to think about how valuable this would be or would not be for you in my opinion it's low risk if you stick to what I was showing you and you don't get too crazy with the pendency injection and trying to do a bit of refactoring along the way you might find that you can get away with that and do some minimal amount of extraction clean things up and that's a good balance for you I don't want to prescribe any one way or the other I just want to illustrate that if you change your Paradigm from how do we make this whole thing more testable you say no we're not going to try to boil the ocean here we're not going to make all of the changes that we need to to make it more testable we're strictly going to look at the spot where we had to make a change and try to figure out how to test that if your bug fix had to touch a lot of different locations you can imagine that doing something like this probably sucks and in fact for something like that unless you had a nice clean extraction and you ended up with classes that you probably wanted to keep longer term I would say this probably isn't a great approach however if you really needed something to get that confidence you could leverage this I try to say this as much as possible but everything we do has pros and cons and you have to weigh what's a pro and what's a con and in different situations those weights will change so if this was the most critical bug fix that you needed to have more confidence before you could ship it I would say you might want to lean into that a little bit more and do something like this to get that confidence you need if you've been repeating this pattern and your code is not getting the opportunity to get cleaned up and refactored and you have a bunch of these tiny little classes that you can't group together you might say you know what it's not worth it right now we should get this landed and then we're going to come back and spend more time to clean it up but that's going to be it for you and your team to decide I just want to illustrate in this video that you do have different options to consider if you want to see me walk through a different example of this you can check out this video next thanks and I'll see you next time and