Lesson 10 of 14
Static Forwarding on a real legacy app
another refactoring technique
In this lesson
In BlogEngine.NET, downloaded from CodePlex, Roy Osherove looks at the static Send method of a ping manager class, which finds links to other blogs in a new post and sends trackbacks or pingbacks. Since the class is static, extract and override (external link, opens in new tab) cannot be used, so he creates a static forwarder: the static method calls a non-static copy on a new instance, whose dependencies become virtual methods. He advises having integration tests before such a risky refactoring (external link, opens in new tab) and sets an exercise: test when trackbacks and pingbacks get sent.
Transcript
[00:25] (external link, opens in new tab) So what we have here is the blog engine which we just downloaded from CodePlex. Our exercise for the for the rest of the day will be to write some tests for it. But first I'm going to show some another technique for working with legacy code (external link, opens in new tab) that will help us get some this this thing under test. So be because our first exercise will be about writing tests for this without anything like Typemock Isolator (external link, opens in new tab) etc. then we're going to have to learn how to do some extra refactoring stuff.
[00:59] (external link, opens in new tab) Now, this project surprisingly actually does have some tests, but if we look at them, the tests are unusually simple or even simplistic. A lot of them are integration tests (external link, opens in new tab) actually. There's some tests for loading files and making directories.
[01:31] (external link, opens in new tab) There's some tests for creating posts and some tests actually use IE which means that these tests are integration tests that are using selenium if I'm not mistaken to run an instance of the browser and actually click buttons and links and then check various HTML on the page. So these are not unit tests (external link, opens in new tab), but they are very helpful when you're working with legacy code.
[02:03] (external link, opens in new tab) For now, we're not going to try to make these tests fast because we're missing all the libraries for running these integration tests. But you can take it as an exercise to try to get these tests to run. But later, not today. What we're going to do is we're going to write some tests for a blog engine manager. Nothing like a core manager class.
[02:34] (external link, opens in new tab) So this is actually a ping manager class our main interest in this class is the send method. So manager send is going to be our main interest here. So first of all let's talk about what the purpose of this is. Blog engine is a blog engine. You can install it and you can create your own blog. blogs.
[03:04] (external link, opens in new tab) At the at the bottom of each post of a blog, there is a thing there are a couple of sometimes we get automatic links that get posted into there. These are called pings or trackbacks. Pings and trackbacks allow us to create instant conversations when someone else talks about us. And we can create instant conversation in someone else's blog by having a link to their blog in our blog. The the purpose of the send method here is to basically go through the HTML of a new post we just created and search for links to external blogs.
[03:42] (external link, opens in new tab) And then for each one of them to try to send a ping to a special server out there which collects this information and then that server saves the information and and when that when other blogs ask the server did someone talk about me it will say yes here's the link to whoever talked about you. So it's a way to connect blogs without talking to each other directly. pings and trackbacks only differ by the amount of information they collect if I if I'm not mistaken.
[04:12] (external link, opens in new tab) But the basic idea is that we have blog settings whether trackback is enabled. We try to send the track back and if we can't send the track back or there's there was a problem where it's not enabled, we send a ping back. So there are two possible end results (external link, opens in new tab) from this unit of work (external link, opens in new tab). Okay, one end result is this. Another is this. So our main our main problem is going to be how to test that this was called when it was supposed to with the correct message or that this was called when it was supposed to with the correct message.
[04:48] (external link, opens in new tab) And there are several things that are going to get in our way when we go through this because these methods are actually static. We cannot replace the instance of trackback to make sure it was called because send is a static method. Luckily, we can maybe use extract and override (external link, opens in new tab) to check that something got called. The same here. We also have track back sorry blog settings instance which is a singleton
[05:23] (external link, opens in new tab) Which we will probably need to break as a dependency as well because we don't want to use the real file system. So, we could use extract and override here as well or create an interface with I settings. But there's another problem. We can't extract and override these things because the class that we're in is a static class. It cannot have non-static methods. So, send is a static method. All the methods here are actually static methods like get track back URL from page,
[05:58] (external link, opens in new tab) Get URL from content. Okay, so if we just try to write a simple test for this, what's going to happen is that we're just going to get through all these dependencies and nothing's going to work. So how do we make this class more testable? Well, to get rid of the whole idea of the static so we can extract an override, we can do what's called a a static forwarder.
[06:30] (external link, opens in new tab) We can keep this static method here, but just a signature. So, what I'm going to do is I'm going to actually create a copy of this cl this method, duplicate it, and have the static removed. and call it send non-static for example. But the problem is that the compiler will say but you're in a static class.
[07:03] (external link, opens in new tab) So I'm actually going to remove the word static here to make things more testable. The only difference between when I have the word static or not is that the compiler will shout at me if I have non-static methods. So it's kind of useless for us. Now that I can have a non-static method, I'm going to go to the original static method and say just create an instance of new manager and call send non-static with the exact items.
[07:38] (external link, opens in new tab) So it's a static method that all all that it does is just forwarding the information. And the reason I need the static method is because I don't want to break all the existing callers to the method. Okay, this way the refactoring doesn't break the whole system, but it does allow me to now actually have a manager class that has instance methods. And now I can go to all the private static methods here. What's a private static method?
[08:09] (external link, opens in new tab) It means it's only going to get used by this class. So it doesn't really matter if it's static or not. And I can make them even protected and virtual. And I can then go and say, okay, I want to break something. Let's go to blog settings, enable trackback send. And instead of actually using a file, I can just extract a method from it and says
[08:44] (external link, opens in new tab) Enable trackback send and don't make it static. Please make it virtual. And that's it. Now in my test I can actually create a testable manager class that overrides this method and returns whatever we want without using a blog settings configuration file. And the same I can do from anything here. For example, get URLs from content.
[09:15] (external link, opens in new tab) What does that even mean? It means it goes and it downloads the actual URLs from links inside my content. If I did that, then I'm relying on the web to be present in my tests. It will become an integration test. So I can override this and return whatever I want. The same here like this whole thing remote file. I don't want to have a remote file whatever that means like read from web.
[09:46] (external link, opens in new tab) Get track back URL from page parses using regular expressions. If a regular expression is too complicated to generate inside a test so I will easily find something then maybe it's a dependency as well and finally I can just extract and override these things to extract this into a virtual method etc. So again what I did to make this testable is I removed the static
[10:16] (external link, opens in new tab) And I made I took the existing static method created a copy of it which is non-static gave it a different name and made the static method just forward to a new instance of my class and called the non-static method. Yes. But in a reality or in a real life case we would we if we don't have any tests in a legacy code we would rather sub integration test first before we do this. Yes. In the real world what you would do before you would do this because this is really risky is that you would have at least one or two integration tests that really use the web page and everything to make sure that the existing flow doesn't break and then you can refactor.
[10:59] (external link, opens in new tab) I wouldn't recommend this kind of refactoring without any integration test but it's worth it. An example is that one time I was sitting with a person I was consulting for them. They had a big legacy system written in C++ and they wanted to add a new attribute to the configuration file. Okay. But they we wanted to do it in a test-driven (external link, opens in new tab) way. So what we did is first we wrote an integration test that actually loads the entire system in memory and with the old configuration file and we checked that the settings were loaded.
[11:33] (external link, opens in new tab) And then we wrote another test that loads them and then checks for the miss the new setting that was not there yet. And then we made it pass with the attribute. So we actually did a test-driven integration test. It was still worth it. >> >> other questions? Okay. So what I want you to do is I is as an exercise I want you to write a couple of tests or more that prove that trackback send gets sent
[12:10] (external link, opens in new tab) When you have at least one URL that in the content of a post it get you want to prove also that it gets sent that it gets called twice if you have two right because basically let's see. There's a loop here. So, you want to make sure this loop actually works. So, you want to prove that if you have just one, it gets called once. If you have two, it gets called twice.
[12:40] (external link, opens in new tab) And you also want to prove that if the configuration says that you should not enable trackback, then you should send a ping back. Okay. You can also prove that if the trackback URL is null because there is no trackback URL then it should send a ping back. All these things have a reason. So we should test them. Okay. So the refactoring we saw was create a static forwarder
[13:12] (external link, opens in new tab) And then you can use extract and override. Okay. Good luck. If there are any questions, just let me know. >> No, you don't have to write the integration test for now. Just write the unit test as if you have integration.