Lesson 8 of 14
Unity Test Review
Roy and the class Review Unity.codeplex.com tests
In this lesson
After the audience reviewed the tests of Microsoft's Unity (external link, opens in new tab) project, Roy Osherove goes through their findings, including a test with no assert (external link, opens in new tab) and setup methods (external link, opens in new tab) that hide the arrange part, which he replaces with factory methods (external link, opens in new tab). They discuss bug recreation tests, which belong in a clearly named class, a test that depends on release or debug build flags and is therefore an integration test (external link, opens in new tab), and a structure that does not separate unit from integration tests. He ends by writing an Assert.Throws-style (external link, opens in new tab) helper for MSTest, which lacks one.
Transcript
[00:07] (external link, opens in new tab) Okay. So, now that you have you've had some time to review the Microsoft Unity (external link, opens in new tab) project, why don't we find out some things that have bothered you about these tests? We'll go over them together. So, who wants to go first? Yep. >> Yes. I have some some general things that repeated a lot of the the patterns repeated where the first thing is obvious and I think I didn't put it one down more than than once is the naming convention (external link, opens in new tab) is different.
[00:44] (external link, opens in new tab) >> The naming convention is different doesn't necessarily mean a bad thing as long as it's consistent and it contains the three important pieces. for example, >> the thing we're testing the behavior behavior and the scenario. >> Okay. >> Okay. So, the naming convention doesn't have to be exactly like the one I'm teaching. I'm just teaching some one that would work. There are many ways to do it. >> Okay. So, so one file that I opened, I just started off the top of the Unity test project.
[01:15] (external link, opens in new tab) >> what's the name? >> the Unity test the main Unity test project. >> Yeah. What's the file name that we're looking to look at? >> Build plan and child container page. >> Build plan and >> by the way this is a with your ReSharper you can just use uppercase letters for the partial words. So BPA will find this. CtrlN to find this file. Okay. Specific line number. >> So yeah in line 64
[01:48] (external link, opens in new tab) G line 64. >> Yeah. So this test there were a couple of things for this and and detect the next test below it and so we we don't have a a range and we don't have an assert the arrange is in in a setup in a test initialize on top and the assertion is implicit that nothing bad happens even says here we expect we are good I must admit that is I would say that's a bit too implicit in my opinion
[02:20] (external link, opens in new tab) Okay. So, let's take a look. So, what you're saying is the test initializes gear range part. >> Yeah. >> Mhm. >> And Yeah. And there's no assertion. >> And there's no assertion here. And it even says no exception means we're good. There's no assert to anything. And you have a problem with this because >> Well, I I don't know what I'm checking here. And it's also the question. So it says we have no exception, but there's no test for the other way that we get an exception if something goes wrong.
[02:54] (external link, opens in new tab) We're just having >> So he's only testing the happy cases, not the unhealthy ones. Let's take a look at the name of the test. Child containers for unconfigured types. Put constructor parameter param resolvers in parent. Okay. Parent container create child container resolve. Yes. And it doesn't throw an exception. But we're not there is no assert here to prove that something is true or false. That definitely feels like it's missing.
[03:25] (external link, opens in new tab) Yes, I would agree that an assert here would make more sense than no assert. Maybe it doesn't do anything and doesn't throw an exception. That might be a case. Yeah. Okay. The fact that it could resolve it without an exception doesn't mean that it's working. >> Yeah. And Yeah. And the other issue I had with this you said that this setup method (external link, opens in new tab) on the on the top >> yes >> that this would rather be nonoptimal so either using something like a factory method (external link, opens in new tab)
[04:03] (external link, opens in new tab) It is a bit implicit in this case the class is not so long so and but I must admit it's yeah it's it's not that I kind the test without going checking oh is there initialize if there would be a factory method or something like a setup method called I would at least know where to go here I have to check that there is something happening before >> mhm okay so yeah we could we could create factory methods of both of these like this
[04:34] (external link, opens in new tab) Make parent and actually not what I meant. What I meant is that we would say like this make parent container and
[05:14] (external link, opens in new tab) Make child container. And then we would just do both of these at the beginning of each test.
[05:56] (external link, opens in new tab) Yeah, it definitely feel to to me I I agree that feels more readable and we still get the same maintainability (external link, opens in new tab) aspect. >> And the other thing is with that the next step including this is usage of fields instead of a variable sudden see the necessity to have the scope beyond that. >> You mean these fields? >> Yes, the parent no container child container. Yeah, we don't need these anymore. >> Make child. Oh, because the the parent is creating the child.
[06:27] (external link, opens in new tab) Okay, that now it makes sense. >> Yeah. Well, anyway, it's a >> but we can accept this as a parameter. >> Yeah, >> this make child my parent container etc. So then we could use these factory methods by giving it the parent container here.
[07:03] (external link, opens in new tab) Okay. Yeah. Now it becomes a bit of a problem if we have more tests. But I would still do that even though it became a bit more of a problem because the setup method is just that much more unreadable. Now that's assuming that all tests need a child and a parent of course that >> okay what you said especially with the mocking if you don't if you don't see the plumbing >> you mean faking >> faking whatever yeah if you don't see the plumbing between the objects it's it's all implicit you always have to go somewhere to see what's happening there >> yes >> okay good what else >> number
[07:43] (external link, opens in new tab) >> where >> >> these ones >> valley injection from child and value injected from parent that's being used here. Well, they're not magic values (external link, opens in new tab) because they're in cons and they're saying this is some value injected from child but it's better if it was one and two but >> yeah but should it then be it's some value be name some value? Yeah, a better name would be some value.
[08:15] (external link, opens in new tab) But as long as it says value from, then if I was to assert against this in the test, I would say, oh, so I'm expecting the value injected injected from child. Then it should be no problem. In terms of readability (external link, opens in new tab), I don't think it's a problem because they're actually using it correctly. The problem is there's only one usage. So it's better to just say some value here
[08:51] (external link, opens in new tab) And use some value right now because no one is using that value anyway. If they had pre-engineered this so they could use the value later but they're not. It's overeng engineering. The point of refactoring tests is that you make things as local as possible to the test and then you pull them out if you need them in multiple tests. Okay. Anything else? Yes. >> In the same file >> wasn't mine.
[09:27] (external link, opens in new tab) >> I think you deleted one. >> Yeah. >> The first test is gone. >> Sure. There it is. >> Okay. This test. >> Yeah. What's the problem? >> well, I have the same things that we talk about, but we don't see. >> Okay. So, here they're actually using value injection from parent and child. So, it makes sense that it's a field though. >> Yeah. >> Okay. >> But to have two >> to have what?
[09:57] (external link, opens in new tab) >> Two. >> They're testing two things. So two asserts assert are equal value injected for parent from parent and from child of value. Values injected are correct when resolving from parent first. Well here the thing is that the they're testing the ordering.
[10:28] (external link, opens in new tab) >> Okay. So the unit of work (external link, opens in new tab) here spans both of these things. is that the ordering matters. So in this case, okay, >> if even one of the values is not true, then we don't even care about the second one because it's already not not correct. >> But we know which one that's >> when resolving from parent first. First means that you you're resolving more than one. So you have to have two results. So and it says values. So it means all values
[11:00] (external link, opens in new tab) Injected are correct when resolving from parent first. Resolve from parent first to elicit the buggy behavior. So this is probably what happens. They found a bug and now they're reproducing it when they when using the parent first to resolve. Okay. So this would be a good place to say okay I'm putting this somewhere else. I'm putting this method or this test in a special class called
[11:31] (external link, opens in new tab) Bug recreation because bugs to recreate them sometimes you you don't use a normal unit test (external link, opens in new tab). You would do something a bit more complicated that a user would do. It's always better to simplify and simplify and simplify. But in this case it's a bug recreation. It makes sense. It could be that this whole class was created by a bug recreation of this and then they added a couple more tests, but it would be nice to say in the class name u bug bug related tests or something like that.
[12:04] (external link, opens in new tab) Okay. >> Okay. More things in not necessarily here. I mean more things that you found. >> Yeah. In the code gen bug fixture. >> Sorry. Codegen bug fixture. So control N codeg gen bug fixture. So bug fixture means they're already there they have a class especially for bug recreation. Okay. >> What's the problem?
[12:35] (external link, opens in new tab) >> I didn't get very far but on the top there test for dynamic method creation in the CL this test would only fail if you run run. >> Uhhuh. Good catch. So what does that mean? >> It means that most lightweave or normal also developed on a keyboard. >> It's an integration test (external link, opens in new tab). It means that you're relying on an external system on the file system and on the build compiler flags.
[13:05] (external link, opens in new tab) It's an integration test. It shouldn't be in the unit test project. Okay? because the test does not have control of whether it's being built in release or debug. So if you don't have control over everything in the test, it's not a unit test. Okay. Yeah. And this file also the fake (external link, opens in new tab) class that they have also have conditional. We were talking about >> can you I don't understand.
[13:36] (external link, opens in new tab) >> Yeah. Sorry. The code gen class that's fake class >> or whatever it does has a conditional down there for this argument null exception and yeah so this is untested code if I if I recall correctly yeah >> this is a code that recreates the bug they're trying to throw an exception when something wasn't >> called yeah sure >> a better way to do this was to Say
[14:07] (external link, opens in new tab) Assert fail static ctor was not >> old. Now it's it's easier to understand. Yeah. >> Okay. >> Mhm. Good. >> Okay. More things, more issues not necessarily in this file.
[14:40] (external link, opens in new tab) What about minus 12? Here you said we should only initial once. So fix value we have that. >> Sorry. >> You said we should only report one particular problem once because the you have the the magic value thing already. I've seen this but I didn't report it because said we should report it once. >> Okay. But do you agree that Microsoft is amazingly beautiful here? >> No, but you said we should only report one thing once. >> Okay. Sorry.
[15:11] (external link, opens in new tab) >> Yeah. Well, it's not difficult to see, but you said we should check for individual properties. >> Yeah, definitely. >> Yeah. >> But as I'm looking through the code, it's always nice to say, "Oh, I found another one and another one." Okay. >> okay. Anything else in another file or here? >> Okay. >> Yes. So next so next file you can open from solution explorer or directly the CodePlex issues fixure.
[15:48] (external link, opens in new tab) >> CodePlex issues. CodePlex issues fixture. There are two there's one in test and one in interception extension test >> the one in unity test on top. So this is by the way the regression tests that we were talking about. So all the so all the regression tests I was wondering when you said this to the code to the other thing that we found that it was reproducing a bug. This is also reproducing bugs. >> Yeah. >> So it's really difficult to see what what's question I had.
[16:21] (external link, opens in new tab) So what in terms of Unity as being a dependency injection (external link, opens in new tab) framework that not so often relies on external factors it's this is still simulating the the whole thing we do with Unity. So the so for the nature I would rather say this was an an integration test because it's because we using >> back testing multiple things at the same time because the tests are not as simple as they could be because they're like throwing in a lot of interesting different things.
[16:55] (external link, opens in new tab) I would say that there's a well these are bug recreations so they're more complicated than regular tests. If they weren't participations, they would still be unit test, but they would be like experiment tests just to play with the system and see what it feels like. I would still put them in a separate folder. If they're all in memory and they're consistent, they have full control. They're unit tests. They're automated, of course. Okay. Anything else special that you can see here?
[17:26] (external link, opens in new tab) Anyone? You know again we have if it's about repetition we we have again a couple of tests that like for example in line 99 there's one without assertion and even we said this should throw here it's I think there's so it means that we're not getting an exception.
[17:58] (external link, opens in new tab) Oh, it's it's expected exception (external link, opens in new tab). I didn't see it. >> No, you I agree. It's harder to see expected exception. >> Yeah. Okay. >> It's definitely harder when it's at the attribute level. I fully agree with that. >> Okay. >> Okay. That was a good thing. >> Okay. Any new stuff? More stuff next? What about you guys? give you some more stuff that you found.
[18:33] (external link, opens in new tab) What about the basic structure that we can see here? What do you think about the structure? Let's talk about that for a second and what they chose to do. First of all, in terms of conventions, they have the source and then they have a test folder and then all the tests are the name of the same name of the project tests. Some other projects sometimes start with the word test and then the name of the project. As long as you're consistent, it should be okay.
[19:04] (external link, opens in new tab) You'll notice that nothing here says integration tests, although some of them you can guess that they're integration test. For example, configuration because you're probably going to need some configuration files like here. So I can guess it's an integration test. Interception, not sure. service location not sure this might be unit test might be not test support is all the infrastructure but there's also configuration stuff here so there's still not a good separation between unit integration tests although we have a lot of different projects
[19:42] (external link, opens in new tab) Test doubles and then we have test objects and utilities these. Can map generic type to new generic type create something name type build key policy generic type missing policy result.
[20:32] (external link, opens in new tab) To me, it looks pretty good actually. Maybe I would separate these just to make it even clearer. But overall the test looks pretty okay. Looks simple, understandable. Expected exception. Again, we wouldn't use expected exception, but would use assert throws or something like that because with expected exception, this line could throw the exception or this line or this line. And we and only the act line is the one that's supposed to throw the exception. So, we shouldn't be using expected exception at all. >> But if you're using
[21:05] (external link, opens in new tab) This is the MSTest framework that doesn't have the throws. So if I'm using MSTest, it doesn't have assert throws, but it's very easy to build my own. >> Yeah, you build catch around. >> Basically, this is how you would build your own version of assert throws in in MS or any other framework class exception
[21:44] (external link, opens in new tab) Public static void exception. You can make it generic. So you can say type of t. Then you can actually take a delegate. We want a delegate that can do anything. So we can be an action or a funk. So we can have two overloads.
[22:21] (external link, opens in new tab) And then basically we will do a try catch try we'll say act and catch t see am I missing something here? I'm probably missing something. We can catch any type of exception actually if you wanted to.
[22:53] (external link, opens in new tab) We can say if type of I think we can even say if x is p then return. Everything's great. Otherwise throw throw X. Now if we got all the way here we can say assert fail
[23:32] (external link, opens in new tab) Expected exception of type T etc blah blah blah blah. We can also do this here. Maybe we shouldn't just throw. We can say assert fail expected this but we actually got a different exception. But basically this is what we do. This is a simple way to build it. And I'm sure there's even a cleaner way to use generics to catch a specific exception where you would use type of key probably or something like that.
[24:08] (external link, opens in new tab) Let's see probably means catch type should extend system exception. Yeah. So we have to say where t inherits from system exception or something to that matter. Yeah. Now it should work.
[24:39] (external link, opens in new tab) Now we don't even have to do this. We can just say look we caught t so everything's great. Just return now assert fail. And probably we would have a couple of tests for this stuff. Okay, questions about this? So you see it's very simple and I guess it's too simple. That's the reason why Microsoft hasn't put this into MSTest in about four years now. Okay. Any more stuff?
[25:16] (external link, opens in new tab) Okay. So, what we're going to do now is we're going to take a five minute break. We're going to prepare the next exercise and then we're going to come back.