Test Review Practices

[00:24] (external link, opens in new tab) Okay, let's continue. I want to talk about a bit more formally about test reviews (external link, opens in new tab) and things to look for because what we've done so far is we've looked at test and randomly I said things but there is a there is a system to the whole idea there is a list of things that I look for so first we're going to talk about test reviews and

[00:54] (external link, opens in new tab) Then the three pillars (external link, opens in new tab) of what to me make good tests. The first thing to understand is that test reviews are one of the most effective ways that you will have to spread knowledge in your organization and to help unit testing (external link, opens in new tab) actually be adopted in your organization. So if you if you want to have any hope whatsoever of having unit testing adopted,

[01:26] (external link, opens in new tab) You're going to have to learn and actually do test reviews in your organization. And the reason is is that there is nothing better than simple face-to-face human communication sitting down in front of the same code and finding interesting things and how to make them even better. test reviews. But there's also a benefit to doing test reviews versus just regular code reviews. If you start code reviews from the tests,

[01:58] (external link, opens in new tab) The main benefit you're going to get is that your reviews will usually be shorter, sometimes up to 10 times. The main reason is that when you have tests, then the test itself reveals the intent of what the developer was trying to do. When you look at code, you don't see intent. you have to read a bunch of lines of code to understand the intent. So in real code, sometimes you're going to have to read 20, 30, 50 lines just to understand that they're doing the wrong thing that they got the requirement wrong.

[02:29] (external link, opens in new tab) Sometimes when you look at it, so you might say you go to the end of the method and then you say, "Oh, so you delete the object from the database only when this is true." But it takes time to understand this. When you sometimes by just looking at the name of the test, you realize that something that there was a misunderstanding even before looking at a single line of code. So test reviews are a good way to understand the intent of developers and they're a good way to drill down into code

[03:02] (external link, opens in new tab) When you want to review how things actually got done. So you see what the developer tried to do. Then you could drill down from the method which is a test to see how it actually got implemented. The three main pillars of good tests are readability (external link, opens in new tab), maintainability (external link, opens in new tab) and trustworthiness (external link, opens in new tab). If you even lose one of them then you're then the rest will break. Why do we need readability? What's the point of readability?

[03:33] (external link, opens in new tab) What do we get? >> Nothing. >> A quicker view of what's happening. >> It allows us to do review quicker. It allows us to understand why a test is failing when it's failing. So, it will allow us to find the bugs faster. What about maintainability? What's a maintainability in a test? What does it mean? Yeah. Yeah. Well, if we leave if we change a structure of the code that they that's easy to to to maintain the test as well.

[04:06] (external link, opens in new tab) So we don't have so so it's it's not a complicated structure that we have to mess around a lot with. So it's easy to to change. So if we change the structure of the code, the code becomes easier to change. The the tests are actually easier to change than if we weren't keeping them attainable. In fact this is one of the biggest problems with tests is that they add extra maintainability cost to our code. So the maintainability aspect is one of the things we have to worry about a lot in terms of tests even more so than regular code because in regular code we might have

[04:42] (external link, opens in new tab) We might have one thing but in test a you might have multiple tests for that one thing. So this is actually you actually have a shotgun effect that when you change that one thing you actually get to maintain five or more different other tests. Trustworthiness is the the one thing that it's always the hardest to explain but the way I think about trustworthiness is what happens when you see the test result of a of a passing test.

[05:16] (external link, opens in new tab) Do you still feel the need to debug the test? If you do, then you don't really trust (external link, opens in new tab) the result and it's not really saving you any time. Also, trust is when tests can lie to you and they say that they fail for the wrong reason. So that you stop believing the tests because the result that you get is not the result that should have been. You know the test should pass and it fails and you know that it's a configuration issue. And this is a problem because when we stop believing our test then again we're not really saving any time by listening to them.

[05:52] (external link, opens in new tab) So we don't really know that things are broken or not. We have to go and find out for ourselves. Now if we lose trust then then we won't care about the readability or maintainability of the test. We just won't run them or we won't change them when they start failing. If we lose readability, we won't be able to maintain the tests and then eventually we won't trust them. If we can't maintain the tests, it's just a matter of time until we stop using them altogether. One of my first projects ever was

[06:23] (external link, opens in new tab) A project where about six months into the project or eight months, I don't remember exactly, we had to stop working on the test. We had to actually delete some of them, a lot of them, about half because they kept failing and we didn't have time to keep fixing them and that's where the maintainability really came in. And so all that time we spent writing them and learning how to write them etc. it actually just we actually threw it in the garbage. So we don't want to go through that again.

[06:56] (external link, opens in new tab) So one of the things that I already said is that in terms of readability and trust we want to make the tests easy to run and we want to separate the unit tests from integration tests (external link, opens in new tab). Now here in this structure this is the NerdDinner (external link, opens in new tab) tests and there's only one project that contains test. So there's no separation between unit and integration and that's one of the easiest thing to things to fix. But also when I look at tests, I look at the structure and I look at the fact that we have fakes (external link, opens in new tab) and models and controllers

[07:33] (external link, opens in new tab) And and what I'm looking for is if I look at the piece of code of production code, is it easy for me to find the test for that production code? That's my that's my test for readability. Is it is the structure readable? Is it easy to find one thing from another? And it should be. So usually we would have a project of tests or unit test per per project of production code. We would have at the very beginning a a cla a test class per production class.

[08:05] (external link, opens in new tab) If we wanted to, we could we could have different matchings, but this is the easiest way to find things that match each other. And if we had a problem with how it's organized later, we can reorganize. But at the beginning this really helps to trust your tests and to get better understanding of how it works. Code coverage (external link, opens in new tab) is actually important but it's only important in specific levels.

[08:36] (external link, opens in new tab) So my question is if you have 99 or 100% code coverage, what does that mean? Does it mean anything? >> Well, you at least have a test that in some condition passed the code line. >> Sorry. You have you have enough tests to to to pass that code line in in one condition. You have enough tests that run that don't break.

[09:07] (external link, opens in new tab) That's all it means. It doesn't mean that anything works. It just mean that nothing is throwing exceptions. If we have a 100% test coverage, it doesn't mean anything unless we review the tests and we know that they're good. For example, we can have all the tests without asserting them, but we can still have 100% code coverage. So code coverage, which is high, doesn't mean anything without a review. What about 12% code coverage? Does that many mean anything?

[09:45] (external link, opens in new tab) If I have 12% code coverage, what does that mean? >> So the classes are not tested. >> It means that I'm missing tests. It definitely means something. If it's high, it doesn't mean anything. Unless I review the code in the tests. If it's low, it definitely means something that I'm missing tests. That's what test coverage helps. But I use code coverage or test coverage also for a couple of other reasons.

[10:15] (external link, opens in new tab) When I review other people's tests and code, I like to sometimes change things manually. I look at the specific test and I'm not sure whether I trust it or not. So, I go to the code that's kind of under the test and I physically change something in the code. I'll take a true and turn it into a false. I'll take an if and I'll make it always true or something like that. Now if I do that which is an obvious bug in the system what's what should happen one test should break somewhere >> at least one test somewhere has to break but if nothing breaks what does that mean

[10:53] (external link, opens in new tab) Tested >> means it's not really being tested or the test that we think is okay is not really okay now the reason I do this manually is is about communication I sit down with a person they can see my face they can see me talking and thinking out loud and then trying this. And also when I do this, I ask them, "What do you think? Is anything going to break?" And just by looking at their face before they answer, I can tell if they wrote it in a test-driven (external link, opens in new tab) way or not. Because if you wrote it in a test-driven way, you're absolutely going to be sure that there's something going to break.

[11:27] (external link, opens in new tab) Yeah, totally going to break. Yeah, I know it. Because they know that that code had a reason to live because they wrote it only after something broke. If they didn't, they're going to have a lot of question marks on their faces. Maybe. I'm not sure. I think so. And so it's a good way to tell what is the culture. How sure are people about their code and their tests? It's not about punishment. It's about communication. It's about seeing how people react to that situation.

[12:03] (external link, opens in new tab) Another thing that we want in our test is to prevent logic. Logic is also a trust issue. The reason is that logic can lead to bugs in our tests. So we want our test to have as little logic as possible. Where's the logic in this test? Is there logic here? There is. Logic is anything that's an if or a switch or a loop.

[12:34] (external link, opens in new tab) Anything that that can change the course of of work. So here's the logic right here. And what's interesting is that there are many other things wrong with this test. But for now, what jumps out to me is the fact that there is a loop right there at the bottom. So let's try and understand this test a little better. Let's look at the name. Select constructor with two parameters.

[13:10] (external link, opens in new tab) U okay. Is this a good name or not? Not a good name. Why? What's missing? The unit of work (external link, opens in new tab) >> the unit the unit of work is missing. >> Well, the unit of work in this case is actually select constructor. So, it's not missing. Okay. What's in there? Select constructor >> behavior. >> The behavior is missing, but we do have the scenario with two parameters. So,

[13:41] (external link, opens in new tab) You're the reader of the test. You say okay selective structure with two parameters that's good. What's supposed to happen? And now the problem is that we have multiple things being tested as the thing that's supposed to happen. So which one of them is the important one? Because if we look at the bottom we have we're getting we have asserts here in the middle assert are equal for the constructor and assert that we get two parameter keys. Which one of them is the important one? Are they both the important one?

[14:12] (external link, opens in new tab) What about this? Then we have a loop for each key in keys assert that the policy is correct for the key whatever that means. So is the bottom loop the important one or the the first assert the important one because if they are both important then they should be separated into two different tests. And the big reason is that if the asserts at the top break then the asserts at the bottom will never get executed. However, if all of these are just the asserts on the same thing,

[14:47] (external link, opens in new tab) Then we don't care about the asserts at the bottom if the first one's right. And for that, we would have to know the domain. We need to know in the domain what what's important, what's not important. But the fact that there are multiple asserts (external link, opens in new tab) that can break each other that can break and then we won't know what's going on at the bottom actually tells me that I think that the that this should be separated into two different tests. If the test name (external link, opens in new tab) had told me what's the expected one then I would know but one of the reasons the test doesn't tell me I could guess is because the test is testing more than a single thing and so the name has to be more generic.

[15:27] (external link, opens in new tab) Okay, if all these things have to be true, then they should be separated. But there are also some things that repeat themselves. We're asserting if we get back to here and then we asserting we get back to here. It's the same thing. This is selected ctor. This is selected ctor. >> How could this be that the same assert is repeated twice in the same test? Any ideas? Yeah, they're coming in.

[15:58] (external link, opens in new tab) They merge two tests to one or something. They merge two tests into one test. Interesting. That's possible. So, two different people. And again, this is something that a test reviewer would catch immediately. So many questions just reading the test. A test should not leave you with questions. It should leave you with answers. And if you leave the test asking more questions about what the hell is going on here, maybe maybe that's a good thing because you need to learn things.

[16:30] (external link, opens in new tab) But if no test give you answers gives you good answers, then you're stuck. Okay, so how do I get rid of the loop? Well, first of all, if I know I have two keys, I don't need to loop. I can just have two statements to check each one of these. Second of all, maybe I don't need to look to check assert on the both of them. Maybe checking just on the first one is good enough.

[17:01] (external link, opens in new tab) If checking on the second one is a separate concern, then maybe I should have a separate test that proves that if I have multiple, then the the last one also gets gets a good assert policy because if a single if a single key proves my point, I don't need to use two keys. And that's another important thing about unit tests. If you can simplify them and use less things in the test, then do that. If you can prove this test with just a single parameter instead of two and it proves the same thing, then do that.

[17:36] (external link, opens in new tab) A lot of times people complicate the test just to get a sense of more confidence. That's not a bad thing. But then I would put all these tests in a special class called confidence boosting tests or something like that or experiments. And the reason is that if I look at the class, I then know that these tests don't really have the same rules as the other tests. The whole point of these other tests is to just feel more confident. Like what happens if I now have 17 parameters and three different flags and all that stuff?

[18:09] (external link, opens in new tab) And the point of those tests is to boost confidence. There's nothing wrong with that as long as you make your intent clear. What about other things in this test? Are you happy with this test otherwise? Is it okay? Yeah. The one thing is the act is just a select constructor line and it's not separated from it. So it's it so we have the assert as if I see C would be the first three statements and then we have the act separate it actually looks like it belongs to the other block.

[18:42] (external link, opens in new tab) It was a bit confusing. >> So I didn't understand you're saying that the act assert or arrange is just broken here. >> Yes. Visually. >> Yeah. Visually we have assert and then another act and then another assert. Are you sure? >> No. We have if I understand correctly the first three lines constructor info specified builder context are a range the next is the act and that is not visually clear. >> So the first two parts you're not sure where what's the act and what's the arrange

[19:13] (external link, opens in new tab) >> yeah is well it's usually the last line before the first assert that's the the act part >> usually. Yeah but yeah I wish I had that problem only that problem in the test. What else? >> It's a magic number (external link, opens in new tab). >> Magic number. 37. >> Yeah, 37 is definitely a magic number. How do I find out what it means? I now have to look like finding files throughout the solution. Where does 37 come from? And maybe I'll never find it. Maybe 37 is the number of methods in a specific class.

[19:47] (external link, opens in new tab) Maybe it's the number of files in a specific folder. Maybe it's the number of seconds that have to go before a specific thread somewhere actually gets released. We might never find out. And that's a horrible, horrible problem. So no magic number. If 37 doesn't matter, just put it in a const and be done with it. ABC is kind of okay because ABC looks like nothing.

[20:19] (external link, opens in new tab) Okay, here's another example of logic. In our test, a lot of times we would have fake classes that implement various parameters, right? So, if we have fake classes that implement various interfaces, etc. We don't want to have logic in them as well. But here is an example of one of them that has logic. You see the epher here below. So sometimes the object will behave in a specific way and sometimes it will will behave differently.

[20:52] (external link, opens in new tab) Now our test might be using this and since this is not part of the test, it has it's located somewhere else. What's going to happen is that our test is going to behave in a specific way and we won't know why. We're going to have to drill down and understand why and start debugging instead of reading what how it's supposed to behave. So to avoid this, I would tell from the test how everything will behave. Just like in previous lessons, we've learned that when you when you have a behavior like throwing an exception, your test should be specific about telling you that the exception will be thrown from a specific fake object.

[21:30] (external link, opens in new tab) It's not supposed to be magical. Also configuration interface is assignable from is an in a reflection method that is very very confusing. If you ever used this method in reflection net before can tell you that you always use it the opposite of how it's supposed to be used. You always think that you're supposed to send in specific type and it's the opposite of the type that you're trying to get because I try to use it. So you can actually have a bug in this piece of logic which is part of the tests and so your test will behave in in a in a in an interesting way and you won't understand that you even have a bug because you don't have tests for this code.

[22:12] (external link, opens in new tab) And that's one of the reasons you want to avoid logic. Even if you think you're okay, you can have a serious bug here where you return false instead of null or whatever. Here's another example of things that could be related to logic. This is another helper method (external link, opens in new tab) in tests from ASP.NET MVC (external link, opens in new tab). And this is a method that creates a fake HTTP context. So we can see that it has the app path and various other parameters.

[22:44] (external link, opens in new tab) And then it has a check. If we have an app path, then create a fake application path. If we have a request path then check and replace the app relative current blah blah blah blah. If we have a port then create the URI this way otherwise create URI that way and then set the URI to whatever and then the HTTP context will behave in a fake way. But the problem again is that not only is this almost unreadable,

[23:15] (external link, opens in new tab) It's also prone to errors. So we want to avoid errors over time. This is prone to a lot of errors. Even though the ifs look simple even simple stuff we can have bugs in. So now we have a lot of we might have tested that that created HTTP context. And the HTTP context might behave badly. And you don't look at the helper methods to understand why bugs happen because that's usually the last place you look at.

[23:48] (external link, opens in new tab) How can we avoid all these ifs? Any ideas? >> Test. >> So, this is not a test. >> It's not a test. >> No, this is just a utility method in the tests. How can we avoid multiple ifs? You're on the right track though. Yeah, >> you set up directly in the tests and not reuse it in this helper method. Well, you can set it up directly in the test, but a utility method does make sense because you keep you could you go repeat the five lines of code every time.

[24:20] (external link, opens in new tab) But instead of having all these if we can have various versions of the same utility method. So we can have multiple utility method that says for example get HTTP context with app path get HTTP context with request get HTTP context with port and if it didn't have a port then it will they wouldn't have an if because by the fact that they're named a specific way they will always be expect to have an app path or they will actually break.

[24:52] (external link, opens in new tab) Now I looked and the amount of tests that use this are exactly two by the way. So it's also a symbol of overengineering. Here's an example of trust where you connect. We actually have logic that you assert against. So here the assert at the bottom is asserting that a specific string the expected string is built dynamically.

[25:22] (external link, opens in new tab) That means that we can actually have a bug with the plus there. Now, usually what happens is that when you build the expected value dynamically (external link, opens in new tab), you more likely to be repeating production code. It's more very likely production code has kind of the same code in there. So, if the production code has a bug and you're repeating the same production code, you have a bug in the test (external link, opens in new tab) as well. And so you expect the bad result in the test. The test will actually pass when it should have failed.

[25:55] (external link, opens in new tab) So it's much better to use hard-coded values and strings in the test than to generate them dynamically. You're less likely to have bugs and it's also much more readable. Okay, so we this is a previous example of the NerdDinner test. This is an example where we had many more magic stuff like the event date was time now ScottGu was hosted by instead of some user etc.

[26:32] (external link, opens in new tab) But there are other things wrong with this test. Let's look at the name. Dinner should be valid when all properties are correct. And then we have in the act we have is valid and then we assert that it's true. So what's wrong with that?

[27:08] (external link, opens in new tab) It's great. It's amazing. Well, what's wrong is that we are the the test is when all properties are correct. Are these all the properties? What does correct mean for each one of them? Is the correct title mean that it has to have a space inside it? Is it the minimum number of characters? What is the correct event date? What's the minimum event date hosted by? Does it have to start with Scott? Country has to be USA. contact phone has to have two dashes inside it.

[27:40] (external link, opens in new tab) We don't know what does it mean to be valid. And also this test because it's checking so many things. It's the name is very very generic. And every time we add a new property, we're going to have to add it to here and we're going to forget to do that of course. So it's much better to have separate tests for each of the properties in terms of validity. Yes. So that would mean that in your your opinion we should repeat every single validation logic line in a separate test.

[28:14] (external link, opens in new tab) >> you should repeat every logic line in a separate test. >> Validation logic. >> you should have a good name that explains what it is that you're testing for each piece of validation logic. If you don't have a good name that explains why it fails, how do you know how to fix it? That's my problem. My problem is that I don't know what's valid and what's not. So I need to find a way to do that. And also there are more things here. If this test fails, how do I know which one of them was the invalid one?

[28:46] (external link, opens in new tab) >> I just try to to think of reality and not this seminar here. And we >> I don't know projects that you have done. So say you have 5,000 6,000 validation rules after a while if you have hundreds of objects or whatever. >> Mhm. So you would whether make one test per per validation rule. So say we have a and I just fix a couple of validation rules before we started today and that would mean so for every single validation rule we get individual tests hopefully more than one because validation rules can will have a true and a false result usually more than a single false result more than a single true result.

[29:27] (external link, opens in new tab) So if you really want it to work, you're probably going to have to test it multiple ways for each validation rule. Otherwise, it's not that important to you, right? The whole point of a validation rule is that it validates something. Well, how do you know? And validation by by by by nature is a piece of logic. You're going to have to test it multiple ways from Sunday basically. So yeah, it should have a separate test at least one. How do I know it's never, you know, that it always returns valid? Maybe how do I know that it has to return nonvalue at some point right?

[30:01] (external link, opens in new tab) >> Yeah. >> So I do know that this question will be raised and that's why I don't have an answer for that. No >> what was what do you mean you don't have an answer? >> Yeah. How to to make this in any fashion in any productive fashion but that is one of the main criticisms if I if I start having so I have say one piece of validation logic for one entity say it's about 35 lines of code we talking about well at least 10 times the amount of tests for that for stuff that is built in in the framework the validation itself is just using stuff that is built using attributes So I'm checking all this stuff again and >> I'm not talking about attributes here.

[30:46] (external link, opens in new tab) I'm talking about pieces of logic that you want to test. >> Yeah, good. >> Okay. So if you wrote custom code with logic for it, you have to test it. You're not supposed to test the framework. But for validation, you this want to prove that no one deleted the attribute, right? You want to make sure I get your point. Now, a lot of times what you're going to get is one big validation method that checks 70 different things. And so in your test, what you're going to have to do is for each one of the validations, you're going to have to simulate that all the other validations are actually okay,

[31:25] (external link, opens in new tab) Right? And then only the code that validates a specific rule that you want, you want to test that. And one of the ways to do that is with extract an override the way we talked about before is that each one of the validations is actually its own virtual method and we have an override for them and then we can just set flags to return true or false true false whatever we want except the method that's under test and that way we can check a specific piece of validation logic even if it's hidden and buried inside one big method.

[32:00] (external link, opens in new tab) But it's much better to have separate classes for each validation rule because then you can test them separately. Okay. But in terms of legacy code (external link, opens in new tab), extraction override is great for that kind of stuff. If you're writing new code, write a separate class or a separate method that doesn't depend on anything and test that directly with as a validation rule. Okay.

[32:31] (external link, opens in new tab) >> It doesn't look like you got your answer actually. No, >> no, I got I got my answer. Just the same answer I had before I came here. And that's so yeah, my opinion hasn't changed that. Okay. >> So, what's your opinion? I don't understand. Your opinion is that there's a problem somewhere. >> There definitely a couple of problems in terms of the ratio between effort and what I get for it. Whatever. I don't have to repeat your opinion. I know this already. So the problem is still I have to sell this to people and I still >> So so your problem is selling this to people. Do you believe that it's a good idea though?

[33:03] (external link, opens in new tab) >> Not yet. >> Not yet. Okay. So you're going to have to try it and see because usually what you do it takes about five or 10 seconds to create a test for that kind of stuff. So the cost is actually not that big. And the only way to test it without an integration test is to test it separately. And if you're going to use an integration test, it's kind of going to look like this where you don't know what's supposed to be wrong. And if one of them fails, then maybe you won't know about the others that failed. Maybe you will depends on the validation structure, etc.

[33:35] (external link, opens in new tab) Should >> so what you're saying here is instead of doing it like this it would in the titles set method you you call validate title method instead and test build test method instead. >> Yeah. So your question if I would instead of writing the title here I would I would call a validate title method. Yes, >> I would call a validate title method and I would have val different tests for it like validate title that's empty should actually return false.

[34:12] (external link, opens in new tab) Validate title bigger than X character to return false etc. I would have one one or two true cases and most of the time I'm looking for a lot of false cases in the validation. Okay, other questions. >> I'm still amused about the latitude. >> Oh, latitude. >> It's way beyond the North Pole. >> we're pretty close to the North Pole right now, aren't we?

[34:43] (external link, opens in new tab) Feels like it. >> We're about 60 latitude of 60. >> Okay. >> And the North Pole is latitude of 90. >> Yeah. So, >> so just 30. >> It's just 30. >> Just two. >> Two/3. That's very close to me. >> okay. So, this is the same example as a piece of logic that we assert against. What about this? This is an example

[35:14] (external link, opens in new tab) Of multiple asserts. Okay. Can you tell? So, I didn't even have room to put all at the beginning of the test, but this is one test method. And what it does is that it creates an array.

[35:47] (external link, opens in new tab) So, there's an array of stuff here. A and each item in the array is an anonymous object that has two properties. One is a property name parameter. Notice how they all have the same name and one is an action which is really an a lambda or a delegate that invokes a specific overload of HTML route link. So there this is a call to a different HTML route link etc etc

[36:19] (external link, opens in new tab) And the difference between so in the array we have multiple delegates each one of them calling a different version a different overload of HTML helpper.outlink route link and then here at the bottom we have a loop that goes through each one of them and uses exception helper expect argument exception null or empty. Okay. Now what this means what is exception helper? This is using MSTest.

[36:50] (external link, opens in new tab) MSTest doesn't have assert throws like NUnit (external link, opens in new tab) although it takes about five lines of code to create it. So this team who wrote this created an exception helper that does exactly what assert throws does. So this is why it uses that. So it goes through each one of the items in the array and then asserts that it throws an error or in terms of readability I'm not sure. It might assert that it's not throwing an error.

[37:20] (external link, opens in new tab) So, I'm not sure if it means you expect an argument exception that is null or empty or you expect an exception of an argument null or empty. Okay, so it's kind of a problem in terms of readability. I'm not sure. And this is exactly the opposites. I'm not sure if you expect an exception or expect no exception here. That's a problem. But usually when people would use exception helpers is that they expect throws.

[37:51] (external link, opens in new tab) So I'm assuming they're expecting exceptions for So what's the problem with this? Is this a good test? >> Isn't it basically lots of tests in the same test? >> Yes, it's multiple tests in the same test method. So what's the problem? If it fails, do you really know which other tests that fail? >> If it fails, you don't know which one of them failed. Yes. What else?

[38:33] (external link, opens in new tab) If it fails in the middle, everything after the middle will never get executed. The loop is going to break, right? because there is no try catch inside the loop here. So if the exception helper throws an exception because it fails the test fails and so maybe the bottom three overloads never get executed and this is an example of a case where you do care about each one of them separately because each one of them is a separate overload is a separate method actually.

[39:03] (external link, opens in new tab) Okay. So yes, you do need a separate test for each one of them or at the very least you have to fix the loop so that it doesn't break at the test and then you have to log everything that failed when the test finally does fail. So you have to build some infrastructure in this loop and loop by definition are a piece of logic. They can actually have a bug etc etc. So the simpler case would just be a separate test case for each one of them. Yes, >> one question. So

[39:35] (external link, opens in new tab) Hopefully I will never create lots of frameworks with with multiple overloads because it's it creates this problem. So I have a lot of different overload that I have to check but also when I go back if I understand correctly and I would check the individual the individual values. So all the different version have to be checked. So I get lots and lots of tests for that. Is that correct? Would you do this? >> Well, in this case, I I says it really depends on the domain.

[40:08] (external link, opens in new tab) What's important, what's not. Let's assume that these are the important tests. So, whatever they test is what I would test. It's separate tests. Okay? Maybe for this domain, it would make sense to also add a couple of more tests. Maybe not. It really does depend on the domain. Okay? So whether I would have multiple tests with different inputs to different parameters for each overload or not really I don't know. If I knew the domain I would know.

[40:43] (external link, opens in new tab) Here's an example of something that this is this is an example of oversp specification. Now there are multiple things wrong in this test. Let's start with trying to understand what's going on here. So let's look at the name of the tests. Does anyone care to read the name?

[41:19] (external link, opens in new tab) Invoke action throws if begin invoke action method throws thread abort exception. Now you should say that five times fast. This is where the underscores start to make sense. Which where does something start and something end? It's really hard to read that. So invoke action throws. Now it throws when when beginning invoke action throws something else. So we are expecting at the end to expect an exception

[41:50] (external link, opens in new tab) And there is this one here. So this is the name tells us this is this is the important part but then if I look at the end of the method there's something interesting going on here mockhelper.verify verify what is mock (external link, opens in new tab) helper all these things. Okay, so all these things are a usage of Moq (external link, opens in new tab) and expect is

[42:21] (external link, opens in new tab) The previous version of setup method (external link, opens in new tab) if you've if you've seen it before and basically it looks at everything that's verifiable and if I set verifiable when I call verify at the end then verify will go through all the verifiables and make sure that they're called. Okay, so really I have 1 2 3 4 five probably six asserts in this single verify. And the funny thing is is that I shouldn't even care because this is what I care about.

[42:55] (external link, opens in new tab) This is the expected end result (external link, opens in new tab) is that I get back an exception. So the big question is why do I have both an expected exception (external link, opens in new tab) and the verify in this method? What's the basic idea? Well, there is no good reason. The only good reason that used to be is that people would, including me, would verify all the interactions just because they could. That used to be the norm. So, I used to I remember writing tests where at the tear down (external link, opens in new tab) method, at the end of each test, I would also verify all the interactions just just to make sure.

[43:37] (external link, opens in new tab) There was something about that. You felt like you had power to do that. But the problem is that you're really cementing all the internal implementation that you have over here. This is all internal implementation. This is all really stubs (external link, opens in new tab) that return specific results so that I can test whatever it is that I'm testing. And this is kind of like me sending my kid out to get pizza and telling him, "Look, I want when you get back, I need a pizza that looks like this." And then when the kid comes back with the pizza, not only do I verify the pizza, I also pick up the phone and I call the the guy at the pizza store and ask, "Hey, did a come kid come by?

[44:23] (external link, opens in new tab) Did he pay you exactly this amount of money? Did he say exactly these specific words? Did he did he arrive? Did he put the right foot first when he got got in through the door or the left foot? Because it would have been the right foot. And if the guy said, "Look, he was the left foot." I would throw the pizza out to the garbage because that wasn't what I said would be. And that's exactly what's happening here. If tomorrow the internal implementation of the method would change,

[44:57] (external link, opens in new tab) As long as I still got the expected exception at the end, what do I care, right? The method, the application still works. I shouldn't care about this. But in fact, this test is the strictest test I could ever I could ever see. Because it it says that every method has to be called. And you know what happens if it had if if a method gets called twice instead? Test will break. It's going to say, "Uh-uh, you said it's going to be called once."

[45:32] (external link, opens in new tab) So this is an example of what's called oversp specification where you have tests that verify internal implementation instead of just the expected end result. Okay, there are two problems here. One is that I shouldn't have a mock object anyway. It just stubs. And while I have the mock object, I shouldn't be verifying multiple things on it

[46:07] (external link, opens in new tab) Okay, here's another example of a test. This is a different test, although it's hard to see. Execute core with a synchronous invoker and action completes successfully. What does complete successfully mean? Complete successful. What does that mean? What do you think it means? >> Didn't fail. >> It didn't fail. No exception. Okay. So, it's not it's not expecting any exception.

[46:39] (external link, opens in new tab) Why do I have two mock objects right here? What the hell is going on here? Well, it's the same as before. We verify just because we can. But there's a different problem here. Here we have two mock objects in a single test. And the test should have no more than a single mock object because every mock object is testing a specific thing. So at the very least, one of them is either unnecessary or has to be used in a separate test.

[47:09] (external link, opens in new tab) Because if the first one verify it fails, we'll never know if the second one fails in this test. But do we even need both of them? Based on the name of the test, we don't. Based on the name of the test, we just are expecting no exception. Okay. Now, even from the name of the mock objects, I can see and I I can see that one is called mock temp data provider. What is provider? Provider means that the direction of the information is internal.

[47:40] (external link, opens in new tab) It's into the application. It's not external. It's something that provides information into the application. When I see the word provider, I'm thinking stub because I'm going to use it to create fake data so I can test my application. This is probably not supposed to be a mock object, but people verified because they can because that used to be the norm. What about the second one? Mock invoker. Where does this get used?

[48:13] (external link, opens in new tab) So we have expect here begin invoke action and then it returns something then end invoke action returns true. So this is again a stub it returns something into the application but we don't need to verify that it was called. We just need to say if this returns true and it returns whatever we need then do we get back no exception execute with the synchronous and action complete successfully.

[48:45] (external link, opens in new tab) Now the problem is is that we're verifying that it got called. A better way to check that it actually got called or that it actually works is that the asynchronous action actually does something would be a much better way to get a real value that went through the system as it worked. Okay. But this is an example where people didn't know what to test for, so they used a mock object.

[49:18] (external link, opens in new tab) But again you were verifying that begin invoke and end invoke action got called when in fact you shouldn't be doing that. You should just assume that whatever is execute core where is execute core execute core is this. You should assume that begin execute core does whatever it needs and if it does the work then you should get back some kind of value from it. That would be a better way, a statebased test that you get back a return value or that something in the system changes.

[49:50] (external link, opens in new tab) I wouldn't use a mock object here, but just stubs and then check a value that I can do something in the synchronous action like set a variable locally and then see that it actually got set. Okay, right now all it proves is that it calls the begin stuff and end stuff, but it doesn't prove that it does anything. And I want to prove that he did anything. If my kid came back with a pizza, I wouldn't even look at the pizza. I would just call the delivery guy or the the pizza guy and and that's how and that's how I would become

[50:26] (external link, opens in new tab) Less hungry if he told me that my kid bought the pizza. That doesn't make sense. I need to eat the pizza. Okay, I'm not just saying that because we're just before lunch and I'm hungry. Okay, but this is an example of a test that says what's really important to me is that you called this. I don't care if it actually got something back or that you did anything. I just care that you called someone. And I've seen a lot of tests like this where people verify that some virtual method got called

[50:58] (external link, opens in new tab) Or verify that some base class method got called instead of verifying the actual action. And when you can verify an actual action, it's much better than verifying an interaction. So I would go for so far as saying that mock objects or interaction tests (external link, opens in new tab) should be no more than maybe 5% of your projects of your code. They should be not the norm. They should be the exception of having mock objects in your tests because they're harder to read. And they are and they don't tell you what really happened.

[51:30] (external link, opens in new tab) They just tell you that something got called. Sometimes that's the only thing you can do, but sometimes not. And whenever you can do something different, you should do that. Okay. This is an example of another antiattern that I can see in a lot of tests in terms of readability. and maintainability

[52:02] (external link, opens in new tab) Where people will create at the top you can see how people will create a chain of objects. So you have a mock principle which is really a fake principle that returns a fake identity. It returns a fake identity here. So that's how people might return a chain of fake objects. But all the latest frameworks including Moq, Rhino and isolator have introduced the idea of recursive fakes (external link, opens in new tab) where you can create an instance of IPrincipal

[52:35] (external link, opens in new tab) And principle.identity will already be fake in the single line of code. So every property that is an object of a fake object will be a fake object and every property that is a a value object will have a default value. So we shouldn't do hard-coded chains in code anymore. We can just use the frameworks to give us that extra edge in terms of readability and maintainability in the tests because what happens if someone now uses the identity do something in our tests?

[53:09] (external link, opens in new tab) Now, we have to go and add another line of code. If we were doing it the the way we did, the upper way to to all the tests that that use this, that's a problem. So, we talked about structure. We talked about method naming, but let's just make sure this is an example of a really bad name. Register get there is no expected behavior. Just look at the assert here. Assert R equals six. result view data password length. What does that even mean?

[53:40] (external link, opens in new tab) What's the assert? What are we trying to prove here? Where did six come from? Well, the name of the test doesn't really say anything. So, we have to assume that the the person who wrote this has no idea did had no idea why what to assert here. So, they just asserted something on the result. But six is also a magic number. And again, where do magic numbers come from? Well, in this case, this magic number came from here.

[54:13] (external link, opens in new tab) Can you see the six? So, we have somewhere in our test, we have a mock or a fake membership provider that has a hard-coded minimum required password length of six. That's an example why our test should be setting these values in the first place so that we don't have to do this kind of stuff. Here's another example of magic values and behavior. This is an override of creating a user in the system in the fake system.

[54:44] (external link, opens in new tab) Now here's the important thing. There is a a lot of logic here. So if I have a username and the username equals some user and password equals good pass and email equals an email then the status of creating a membership is success. So every user in every test to create a good user has to provide these values and somehow they magically are supposed to know that these are supposed to be the good values.

[55:17] (external link, opens in new tab) What if I wanted to return a non-successful one? Whoa, that's very simple. So in the email, I would send in a string with the name, sorry, with the name of the enum for which I want the status to be. Okay. So the email instead of email should contain the name of an in value from MembershipCreateStatus.

[55:50] (external link, opens in new tab) So it should be MembershipCreateStatus something as a string in the email. It's like a really powerful magical incantation that you have to really know. It's like a magic password that says sh I want you to create a nonworking status. Just look in the email. No one will find this out. Okay? There's no way for people to find this out. It's not explicit. It should be explicit. either

[56:20] (external link, opens in new tab) By creating a public static property or some settings somewhere that the test would set and this would use these settings. That's how it should be. It shouldn't be hidden from the tests. It should be explicit in the test what's going to happen. which is a good idea for the interface that you are testing that it should include these types of enumerations for texts and so >> well this is not the interface >> yeah is it a good idea for the interface I'm testing to have these enumerations well this is not the interface I'm testing

[56:59] (external link, opens in new tab) >> this is the stub that I'm using to test something else >> test test of this class. You have a class, you will test. So the class or in conjunction with the class, you also have these texts that you will use in the implementation of of your class or your library. And then you will use the same enumerations in the test for setup and for

[57:33] (external link, opens in new tab) Checks and >> yeah so I would have some maybe shared const or enumerations that are visible in my tests through some class and then I would say that my test will create something and and use that const to create it and then here I would use the same const in this code >> and so that I would have a shared knowledge of what's supposed to happen. Yeah, >> you can have all the arrow texts

[58:04] (external link, opens in new tab) Could be defined in the class or maybe if you are are checking against the error texts or contains this. If I'm looking for a specific error that's hardcoded into a stub in the system, >> then the stub would use a const and I would assert using that const as well >> so that I know where it's coming from. >> Yeah. And also maybe the class you're the you're testing if that class has defined these texts

[58:37] (external link, opens in new tab) Like that you can use some post setting them >> in the I mean >> yes if the class contains the these text then I can go directly and say I'm expecting the error to be the class dot some expected error or some error default value >> because that's then that type of information is a part of the API >> that If it's a part of the API I'm testing, then I can use the con in that API to expect something. But the important thing is that even before that, I need the reader of the test to understand why should he get that error in the first place.

[59:13] (external link, opens in new tab) So there should be a stop somewhere that sets a false result. I need to see the test telling that stop to return a false result. to have it both in the definition of the test and in the in the verification or >> yes I would has in your range part I would see what's going to happen and then the assert I will see what should have happened yeah here's an example of a magic number inject me and 15 not sure what 15 is could be a million different things the number of methods in somewhere

[59:50] (external link, opens in new tab) Number of files etc. We're talking about naming. Okay. So, what we're going to do now is, we're going to, head off to lunch and when we come back, we're going to do some, test review exercises. Okay. If you if you have this test driven, goodbye.