Finally, got hands on the very old any huge legacy code method, when we have every dependency you can ever imagine: file system, statics, HttpContext, services, objects creation right into the method, DB calls via static methods, configuration parameters readings etc.
And the length of this code is 5 or 6 screens.
And we have very good if if if else if else else structure inside, code duplication all over, complex conditions. And on top of it try catches block in the middle who mostly silently catches all the problems (luckily setting error state 500 as the result).
As everybody knows, to start refactoring legacy code you need some tests coverage first on top of it, so you have a little bit confidence in what you're doing in the very beginning. But we're calling this code legacy for the reason. And this reason... lack of unit tests on top of it.
Hugh... some kind of recursion here...
So to get out of this recursion we need to cover this legacy piece of crap with some kind of temporary integration tests, who going to live just for the sack of initial refactoring. We name them pinttests. These tests supposed to run on the fully configured live system, on the real file system, real DB, real everything as much as possible to cover the desired method. And then we're going to chip things out one by one, refactoring problems and covering it with real unit tests.
So, pinttests! Horay.
File system is okay. DB is live and ready. services working as expected, all real objects created okay, statics make static calls - all good.
But we have one pain here. Old pain from the old ASP.NET web forms. And it's HttpContext.
To make things perfect it's a naked class. Just a couple of common interfaces implemented. And, finally, as the final nail in the coffin, it's sealed.
So when to start? Finally I found some example of creating empty HttpContext to pass in.
And initially this fake works pretty well.
You can set cookies here
(thanks to Pavel Vladov for C# coloring)
httpContext.Request.Cookies.Add(new HttpCookie("SomeCookieKey", "MySuperValue"));
And you can set some cache values as well.
The first problem you're facing here is adding some staff to server variables.
If you try to make it like
httpContext.Request.ServerVariables.Add("REMOTE_ADDR", "123.123.123.123");
If compiles okay, but when you try to run it - failed. You just can't do it.
Some smart guys already solved this problem for us using another smart feature MS made for us: extension methods. Idea here: you can write some extension on top of HttpRequest object and it will be looking like
httpContext.Request.AddServerVariables("REMOTE_ADDR", "123.123.123.123");
And works great! (code of the two extension classes to make it work will be here later).
But... but it's not the final problem you're going to face.
In Response we have a method BinaryWrite() and we can't fake it because internally it checks the stream type he has in hands was created appropriately through the normal Response creation process. And we don't have it for sure in our artificial one.
And in my case this final problem actually switched my focus from pure pinttest to some kind of fair change without full coverage, but I think it was fairly easy and straightforward change - so we can carefully make it.
We're going to replace each HttpContext with HttpContextBase, as advised.
And in every public place, when we pass HttpContext in we're going to create sister method with HttpContextBase and call it from the old one getting the HttpContextBase via the new HttpContextWrapper(httpContext).
And the length of this code is 5 or 6 screens.
And we have very good if if if else if else else structure inside, code duplication all over, complex conditions. And on top of it try catches block in the middle who mostly silently catches all the problems (luckily setting error state 500 as the result).
As everybody knows, to start refactoring legacy code you need some tests coverage first on top of it, so you have a little bit confidence in what you're doing in the very beginning. But we're calling this code legacy for the reason. And this reason... lack of unit tests on top of it.
Hugh... some kind of recursion here...
So to get out of this recursion we need to cover this legacy piece of crap with some kind of temporary integration tests, who going to live just for the sack of initial refactoring. We name them pinttests. These tests supposed to run on the fully configured live system, on the real file system, real DB, real everything as much as possible to cover the desired method. And then we're going to chip things out one by one, refactoring problems and covering it with real unit tests.
So, pinttests! Horay.
File system is okay. DB is live and ready. services working as expected, all real objects created okay, statics make static calls - all good.
But we have one pain here. Old pain from the old ASP.NET web forms. And it's HttpContext.
To make things perfect it's a naked class. Just a couple of common interfaces implemented. And, finally, as the final nail in the coffin, it's sealed.
So when to start? Finally I found some example of creating empty HttpContext to pass in.
httpContext = HttpContext.Current =
new HttpContext(
new HttpRequest(null, "http://tempuri.org", null),
new HttpResponse(new StreamWriter(new MemoryStream())));
And initially this fake works pretty well.
You can set cookies here
(thanks to Pavel Vladov for C# coloring)
httpContext.Request.Cookies.Add(new HttpCookie("SomeCookieKey", "MySuperValue"));
And you can set some cache values as well.
The first problem you're facing here is adding some staff to server variables.
If you try to make it like
httpContext.Request.ServerVariables.Add("REMOTE_ADDR", "123.123.123.123");
If compiles okay, but when you try to run it - failed. You just can't do it.
Some smart guys already solved this problem for us using another smart feature MS made for us: extension methods. Idea here: you can write some extension on top of HttpRequest object and it will be looking like
httpContext.Request.AddServerVariables("REMOTE_ADDR", "123.123.123.123");
And works great! (code of the two extension classes to make it work will be here later).
But... but it's not the final problem you're going to face.
In Response we have a method BinaryWrite() and we can't fake it because internally it checks the stream type he has in hands was created appropriately through the normal Response creation process. And we don't have it for sure in our artificial one.
And in my case this final problem actually switched my focus from pure pinttest to some kind of fair change without full coverage, but I think it was fairly easy and straightforward change - so we can carefully make it.
We're going to replace each HttpContext with HttpContextBase, as advised.
And in every public place, when we pass HttpContext in we're going to create sister method with HttpContextBase and call it from the old one getting the HttpContextBase via the new HttpContextWrapper(httpContext).
No comments:
Post a Comment