 This talk is about Anti-patterns. I might I might talk about test smells. So what I mean by by test smell is that it's It's some kind of a hint It might be this the shape of something or the name or some kind of a pain or Feeling I get from from code in this case test code That suggests there there might be an issue So it's not an absolute. Yes, that's a problem. It's more like There's something funky about this. I should like Look at it and and See if there is something behind Not necessarily, but that might be the case so I'm a programmer by background also trainer coach I Work at a consultancy And basically I spent half of my time working with code half of my time with Methodology, so this is clearly based on the the the other half where I work with code in a bunch of different organizations and Sort of lucky to get Exposure to all kinds of code bases mostly Java and the example search Java But I think that this these anti-patterns apply to many languages, not just object-oriented languages many others as well, so here's Here's a piece of the abstract if you read the abstract for the session Just like the point out the the really important parts of the abstract tests need to be maintained and that's you know, you hear a lot about When you start talking about Thorough unit test or other kinds of automated tests one of the main concerns That seems to raise is isn't that a lot of code to maintain? Isn't it a burden if we write so many tests? that's that's actually exactly what we're trying to avoid by being able to identify and fix test smells the fact that tests are Unmaintainable that's that is the ultimate test smell So we're we're going to look at a few Particular sort of names for things that I I've given or somebody else has given and some Heuristics around how to how to think about these things now We're talking about smells which which means it's not an absolute this is good and this is bad Sometimes what's good in the one context you go to the next source file and You know there the good the so-called good structure or good design is actually a really bad idea And you do the exact opposite Refactoring in a different source file and it that's that's a good idea in that context So this is going to be a kind of a walkthrough of a few samples I've categorized them into four different groups. We'll start from the top left corner talk about smells related to how you check things so assertions In in a test framework language Then we'll talk about how we organize code test code Between classes methods and so forth then we'll we'll go through the bottom half and I've got examples of each there again their Java, but I think you'll you'll follow even though you're not maybe a Java programmer So let's start with the first assertion. It's called The smell is called primitive assertion. Now. Here's the sample of it There's something Something I think is wrong about this code. I call it primitive assertion. How would you? describe it What's the problem here? Has something to do with the assertion? anyone Sorry It'll be always false So there's nothing wrong with the I mean this test will pass. Let's just assume that There's something about how we're making the assertion. That's that's the problem Now I call it primitive assertion and I call it primitive because what we're asserting is that Evaluating certain type of script Should produce certain type of output Now when I described what what we're testing none of none of those words Included numbers like one minus one zero anything like that What we talk about? strings Conceptually not numbers. So the whole Assertion about what's the index of a certain string? That's too primitive for our purposes So it's it might not be a problem at this this scale, but if you have thousands of tests and Everywhere you use slightly too primitive Concepts to express your intent that might become a really big burden over time Because any code you look at is too primitive You always have to first scan it through and then oh, yeah, that's what it's doing. It's not immediately obvious So I changed it a bit Here's an example of a slightly better way to do the same assertion using a slightly different API so these are two units, but basically expressing the same thing using Words that are closer to the concept domain We're asserting that output contains a certain string. It's much much better in my opinion So that was a primitive assertion. There's another assertion smell that I'd like to point out I think this might be something you've also seen It's called broad assertion and this requires a bit of explanation so The following example is from a from a system I worked on Well a decade ago This was for a medical company So a drugs development company. So they what their business is It's basically coming up with a drug and then trying to pitch the drug to doctors Because it's the doctors who prescribe and basically market the drug to consumers. So The company was concerned that they're They're sort of sales force the people who go and talk to the doctors Give out samples and so forth that they're not actually selling the product. They're just recording hours logging hours and They're not actually talking to doctors. They're home playing PlayStation and you know cashing in the paycheck so, you know slightly Paranoid, but you know, I was young and foolish and wanted to help the evil corporation So I set out to implement a desktop application that was running on the sales reps computers and So we embedded in all of the sales presentations the drug presentations we embedded a small small sort of active component that continually sent information to a small piece of server software running on the Windows operating system and Basically logging what what slide are we displaying of which presentation and then what the next time the computer had a network access They would upload that information to the server so In short, we are collecting a log file over here and then we're pushing it to the server when we have internet Now the one of the challenges was that we were the the log file produced by the the component we embedded into the presentations and The the file format expected by the server was slightly different So we had we were doing a small trans translation or transformation on the client side just before uploading and That's what the next code is code is about So here's a test that I wrote some some time ago it that was You know not using JUnit for so I've translated into an API that you know people generally recognize but the Approach was the same So we we have a couple of Strings that we deal with here in the test input file and expected output So these are the actual the actual content not File paths or anything like that We had one setup method that said create input file and here you can see how the the string builder is sort of Accumulates the the sort of stuff that we have in the input file the log file produced by the presentation and then We had another setup method which set up the the expected output. So after the transformation What should we have and this is what we were supposed to upload the server? So similar Similar information or actually the same information, but in a slightly similar but also slightly different format so for example, we didn't put timestamps over here, but screen durations and The overall file structure was a bit different So those were the setup things set up input and output and Then we had a test the test said transformation generates right stuff into the right file and You can read from this test what it's doing and I'm I'm saying that this has a smell This test has a smell that I call broad assertion I'll give you a moment to Try and imagine why you might I call it broad assertion? Yes, it's testing so many things That if if the test fails I just know that there were some kind of a difference between the input and output and now this might be manageable It's it's only 10 to 15 lines of data on Each like expected and actual so it's not impossible to figure out what the difference was but it does take a lot of effort to to dig in and Figure out what's going on There's another another problem with this it relates to the the breadth of the the data we're comparing sorry The sorry the hum is so loud I It doesn't have to do with the files no Has to do with the contents of the files being fairly big chunks so It's not just that when something goes wrong. It's it's sort of hard to figure out what went wrong But it is that it goes wrong very easily So anything small that changes anywhere in the the log file formats this test will fail So Contrast this to having more specific tests Multiple of them that each would only test one particular aspect and not fail if some other aspect fit is changed So here's a like one step Towards what I think well at least away from this particular smell having separate tests for the screen durations being Produced correctly so in this test we're sorry. I shouldn't touch that in this test. We're basically creating a very simple Log file Using a small utility So in the setup we're actually setting up like the surroundings for the log file and then when we say transform This little helper method. He'll here will take our Take this content and wrap it inside the overall structure for the log file So in our test we only see the relevant bits in the test method and also we're we're simply Sort of fuzzily comparing that you know there should be the word started towards the beginning of the file and Finished in towards the end and somewhere in between there should be screen one Suggest that the structure is sort of correct Very fuzzy. It's not very Not very specific. So there there are things or changes that might be sort of falling between the cracks with this approach and Then some of those those other things might be caught by a different test So for example here, this is just the screen durations being calculated and rendered appropriately, so you have three Time stamps and then based on those time stamps you should calculate certain numbers over here Again specific to some parts of the transformation output Something else might be changed. This test wouldn't notice as Long as those other things are covered by a different test That should be okay So those were a couple of assertion related smells there. There are a bunch of others But I thought I sort of pick a couple that I find fairly common doing two detailed primitive things and doing two broad So universal comparisons usually it's assert equals When it comes to broad assertions So let's look at a couple of organization patterns smells This one I called the split personality and Here's an example Can you identify what's split in This example Yes, this this is testing many aspects So the test has a split personality on one hand. It's testing this and on the other hand. It's also testing that So that's the that's where the name comes from but there's also another slight maybe even more common Test smell in some code bases at least the white space And that's a small hint that there might be something wrong about this test because the programmer felt like you should insert white space in between to Know separate the two concepts Yeah, that's that's a small detail inside this bigger smell But so yeah, we're we're testing a whole bunch of command line arguments passing those to the command the configuration object And then asserting like the result of or the effect of all of those flags in a single test so here's a One way to so start pulling those apart Deal with the default options separately and then basically figure out What what's the default for default effect for default configuration? Because in this case, it's kind of hard to say. I mean we have no idea what the default is for the others But if you if you pull out default separately that tends to be a good idea and then Verify the effect of certain flags with separate tests Like for example here explicit options are set correctly now. I would still go further It wouldn't fit on the slide, but I would still go further and and Verify the effect of each individual flag separate with with a separate test What's still missing conceptually in this case is the the combined effect of flags So what if I have Minus F and minus V is it significant? Well, I might make it the risk judgment that is they shouldn't have any effect on each other I might not test that but It might be that minus D and minus F have an effect on each other You know in that case, I might actually want to test that combination explicitly as well The same this sort of splitting Splitting this split personality test into each personality having a separate test That also helps with in the same way as with protossertions when something fails It pinpoints it more more specifically. This is what's wrong It might mean that you have in absolute terms more lines of code in your Test suite on the other hand each each individual block is easier to grasp and it's easier to reason about Another organization smell I added here was split logic This tends to be an issue only in some certain types of code basis most web applications don't seem to have this but I've seen a few all kinds of various things where this isn't is an issue so This this particular smell I found from the J Ruby Project J Ruby is an open-source implementation of the Ruby language and interpreter on on the JVM and If you're looking for Samples for bad code, I'd say J Ruby unfortunately is a fairly good source It's not very it's not very good code in my opinion Although I love the the product as such the code code is awful so oops one of the one of the classes in in this J Ruby project is a class called Ruby which is kind of a High-level object in the system which represents the runtime so you can tell the Ruby object to Evaluate certain code or interpret certain code. So that's what we're testing here the test was called variables and methods and It's a one-liner that says eval this piece of Ruby code and then That should load some kind of a source file and then a bunch of assertions that compared The result of a different eval to some expected output Can you tell what the split logic issue here is? What's the logic that's being split? Loading of the file the file the external file does relate What is it about the file? Well, yeah, the file might not exist It's not quite that that that would be an issue Luckily we'd find out probably Through some kind of an exception. Let me ask you another question Why should this piece of Ruby code Evaluate to hello world Why should this evaluate to hello world in reverse? So it's not quite obvious from the test because well because of you know, we're testing something that's very like non-domain related to us But because the half of the logic of the test is inside an external source file So we've split the tests It's logic into two source files, so we have to switch back and forth well, and what's even worse is Probably when this happens And you for example you double-click into Into this source file in your ID The odds are that your ID actually opens as a separate text editor unless you've added all kinds of plugins in there But that's specific to the JRuby project So we whenever we do something with this test if it fails We have to look at the source the source file and the other source file If we change the test the same thing we need to go back and forth So clearly not so good This is what what's inside the variables method variables and methods Ruby file and A bunch of calculations is being done there So one way to solve this would be what? Yeah, just put them into the same source file Here's one way to do it. There's several of course You might create some kind of a utility. I've called it a pender ball file and I had I wanted this This utility because I didn't want to deal with line feed characters So I thought that this might be useful because there was a lot of things that were going on there and That made that made the source listing too long to fit on the slide using a decent size font Simplified it a bit At least this would bring this the logic into the same same test method Can you see a problem with this this test method Too many string constants Technically there aren't any constants We're testing too many things so so what's this smell called? Split personality. Yeah So that and this tends to happen It's just like refactoring when you refactor one shape to another the new shape might have something to refactor and You know the same with test smells once you get rid of one smell. You might uncover another smell or you might basically Exchange one smell to the to another And this is why I sometimes end up reversing or reverting a refactoring because I realize that actually This isn't much better. It's actually slightly worse So I if I can't find a way to refactor further. I might actually backtrack a bit Same with test smells. So here's how you might go about it separate the concepts separate tests for variable assignments and separate tests for method implications and you might go even further but That's that's generally the idea So let's talk about transparency When I say transparency, I'm referring to transparency as in the the reader of the code sees What's going on and why it's going on? The first the first sample I called setup sermon as in You know you go to I'm not sure. I'm not very religious But in Finland we have a tradition to go to the church once or twice a year and You know, especially if you look at small kids, you know, it's like it's a tradition so you bring the kids and They don't understand what's going on and then the the priest goes on and starts my speaking at them They couldn't care less and the priest is using awkward words. They're totally foreign You don't understand what he's saying So they dose off and the same happens with programmers with certain kinds of code Here's an example. It's slightly small font size, but I'd like you to Try and figure out what this setup method is doing Once you can put that into words, please Say it out loud What what is this setup method? setting up So so far we have reads XML populates some objects. What else? So uses the file system to set up stuff over there Has three Three present. Okay. So now we're getting close. Can you repeat the three aspects? You said three aspects Reading an XML file. In this case, it's called a catalog or manifest. Yeah, it's It's getting a list of missing resources. Yes, and third And it gets a list of downloads based on the manifest that a List of missing packages so Conceptually there are three things going on, but that's you know, none of those concepts are very obvious At least not very visible in this this setup. So if we're conceptually talking about three steps Why why isn't that a three line method? And that's that's exactly the the setup sermon thing this happens a lot with setup methods particularly With test with test methods, you know, we already saw us the split personality thing which is Kind of a similar problem, but for a different reason. We're trying to test too many things in in one test But even when you split those into separate tests a lot of the times you still have Similar problem in your setup, which is you're setting up a lot of things in a single method It's not that you shouldn't set them up in a single method necessarily Especially if you need to do these these things in a certain order It might be very good to have just a single setup method rather than three separate setup methods But you you do want to Have the method be Transparent so that the intent the purpose is visible So here's one possible way to refactor that Extract a couple of private methods You know, we can create the package fetch or whatever it is That's not necessarily you don't necessarily have to do this in the next a private method It's a it's a one-liner fairly high level operation and then we create a temp directory and Then we extract missing downloads from a certain XML file that's more or less the this sort of same conceptual abstraction level and Then all of the details of how how we create the temp there where temp directory is physically How do we extract the missing downloads? That's that might be relevant sometimes, but you can easily find it By control clicking over there when necessary Another form of missing transparency or not having enough transparency is that It's the big string and this is again XML. So I apologize for making your head hurt The the previous example The the exit parsing the XML file Getting a list of missing resources and so forth that came from the same drug company exit the code base that I made for the evil drug company This is also from the same the same code base. This is the actual contents of the XML file So you can't read this, but that's a setup method. It starts over here and continues over here Basically appending to a string buff or building up a The XML contents of the the manifest file and it doesn't stop there That the method doesn't stop there. There's another piece of XML. That's actually one of we've managed to create the The like first of two Versions of the manifest and then we start setting creating a second version a variation of the the same manifest file and Continues all the way until here So basically what these are is the version of the manifest on the client the version of the manifest on the server then There's a difference. Therefore, we need to do an update and download missing resources so Here's an alternative that I Think if you're using XML that you might might want to consider Using some kind of a builder pattern For example, this is a hand-built Builder object called product catalog XML builder Now product catalog is a synonym for manifest so We create this builder and then we use somewhat fluent API for adding products with presentations Instead of actually dictating the exact XML format. Now, this has a few benefits Whenever we change the structure of the the manifest file, we don't have to go through all of these tests Because it's all Encapsulated inside the builder object. We only need to update the builder to generate the proper XML Second it's more visible. What does the manifest contain? from here, it's next next impossible to to sort of Figure out what products are in there and what are the reason the relationships between them the more Levels you have the indentation levels the more chances that you you know your Your eyes don't quite align things correctly and you think that Product a contains not just two presentations actually contains three because one of the lines was So far away that you kind of misaligned it It's quite difficult to keep track of but this is fairly simple There is something that you we probably should do further What the heck does false me? The you might want to take this even further But again, this is just for to fit on the slide easily At least you probably get the idea Now in this case, I have also two separate builder instances and and and and The infamous white space in between which might suggest that I actually might want to do two setups Now these these things also don't have a dependency. I can create this before this Or the other way around it doesn't matter. So what I'd probably do next is split this setup into two and call them maybe Create local catalog and create server catalog or something more descriptive Does anyone have an have experience with builder objects or? Factory objects Yeah, are you by the way? Are you using Ruby by any chance? No, okay? Because this is it's fairly established in the Ruby community. You have a whole bunch of third-party libraries that for now Creating objects like this in a use in a sort of concise manner for testing purposes But it's less common with Java and she sharp and so forth So that's actually I'm very happy that it's not just Ruby people who do this Let's look at maintainability things The first one is called absolute file path and I think you've seen this What's wrong with this test? It depends on the the location of your workspace And so some of the ways this impacts your your developers or fellow developers is that Everybody has to have the the project checked out into the same location. Otherwise, you know just doesn't work this test will fail Another thing that a lot of people don't realize is that You look you can only have one Version of the product the project checked out Now if you're doing if you're working on The like development branch and then some kind of us a maintenance branch Now what do you do when you need to fix a bug? It's it gets awkward So it's it's not a very small issue yet. It's actually a big issue Now luckily the the solutions are also trivial So the first thing in Java would be to consider a relative path Yeah now This would no longer depend on first of all windows. I can also use this on a Linux and a Mac. I knew it would work Also, it doesn't any more depend on the location of the project Directory so I can have several Versions checked out or several branches checked out Can you think of any other way to get rid of the problem? What else could you do then use a relative path? Sorry mark mock the XML file So well, we could we could build the XML in in source code in with Java code around here That might be an option. Yes the the trick is if we if we're testing something that that we want To use the file API then we might want to actually have the file in in place But that's a very good option anything else any other ideas Sorry properties Sorry, I still can't hear with that configuration properties that might be that might give you Another way to get rid of the absolute part The added complication would be that everybody has to maintain their property file might be a Reasonable way to get rid of a situation if for some reason for example if you're using an API that for some reason doesn't accept relative paths Even it requires an absolute path That might be a way out of it So Another another thing that you might consider is using the class loader so if if your test data is part of the source packages or Source test resources, whatever. It's actually in the in your class path in Java. So you might also use Something like this asking the class loader to to give the resource and read it from the from a stream or something This would have the the relative benefit that you can actually move the class around as long as you move the The the test the catalog XML as well as you move the class You can move it to a different package a different source tree all together and they'll still work You don't have to go and edit the text edit edit the literal string minor difference, but Might be useful So the last sample I wanted to show is what I call pixel perfection this is also crap that I wrote ten years ago and To give you some background this was a like a graphical editor so Think of boxes and lines boxes are connected with lines It's like a trivial graph editor in a sense now. I was I was I was concerned with our Graphs being rendered properly on the screen. So it's visually okay and The one of the tests that I wrote I look like this There's there there are two boxes at certain coordinates. Oh, sorry with certain sizes Then we create a diagram add the two boxes there with at certain coordinates and Then we render the diagram on a on a buffered image And then we assert all kinds of things. So what's wrong with this? It's very fragile so in this particular case we're talking about fairly simple Square not square rectangular shapes So it's not like it's so sensitive to the the width or the height of the box But for example It's very sensitive to unrelated changes if we're changing something about the size of the box obviously we We need to change this but it's not just that if we change the the thickness of the border or The padding of the elements which kind of shouldn't relate to whether a box is connected or not Still this test was break. So it's it's it's very quiet fragile indeed now Can you figure out another way to test this? Test that the two boxes connected with a lot or connected are indeed rendered with a connecting line How else could we test this you compare the top and bottom of the boxes So that would work if they're always Adjacent so sort of touching each other But if we're talking about boxes that are separated there should be a line drawn between them So What else could we do then check certain pixels that this picture should be white that should be black? so that's very good, I have given this presentation a few times and Like you're the first person to actually suggest this there's a there's a downside to it as well So and that's actually it's better than this It does retain that some of the same problems as this so to Let tell me if I got it wrong, but so the idea was that we we might use constants first of all and and if we place the The boxes have certain locations And then we maybe by trying giving it a shot and seeing how it renders adjusting the numbers a bit at some point We might get a completely diagonal Line or maybe a completely straight line between the two boxes and Therefore we could we could just point out the starting point and ending point and say Like loop through all of the coordinates in between and that should be black anything else should be white So that that that would get rid of a lot of these numbers So the the way it might be still vulnerable to or fragile to change unrelated change would be that if if the Again the shapes change a bit or if we change the place where the the line is attached to the box for example It's it's no longer the lines are not drawn from the center of the box to the towards the center of the other box but it's the That's a nearest corner of the rectangle and from there straight to the lead the nearest corner of the other rectangle the the line would be at different place and It's conceptually. It's not about the boxes being connected. This test shouldn't break If we changed the sort of connection points True true. Yeah, so we could we could also Get rid of this Deplication somewhere we need to like tell the computer how to recognize a line from you know non-line stuff. That's true so looking at this so this was code that I wrote is actually more than 10 years ago and Once I sort of it's scavenged my Hard drive for all kinds of crap I could find This hurt my ego so much that I wanted to fix it even though that the software is no longer used It's probably been Not unused for 10 years already So I set out to fix this in all kinds of ways. So I did try to apply simple refactorings like Getting rid of duplication But I could never really get rid of the coordinates There's always a bit of the coordinates involved and I wanted to get rid of those I call this pixel perfection because it's so specific about the the coordinates and This is what I came up with Instead of a certain pixels I Decided to start by saying okay. I don't care about the pixels. What I want to do is box one and box two are connected So I just wrote that into into the test and deleted all of the coordinates and Then I set out to implement this matcher the connects and it took a while Here's the complete implementation for the matcher. So it's it wasn't trivial But I was determined to fix fix a mistake in the world that I had introduced and Sorry Yeah, in a sense, yeah Yes, so once once you start having so much logic in your tests, it's kind of like you you also need to test your tests The the thing is you might test your test with the production code Intentionally changing the production code on saying that test should fail run the test if it doesn't fail something's wrong Yes entertain myself so you can't see this and That's okay the the reason why I picked this example into a presentation is that this represents and a style of a style of testing that I Think not too many people consider. I'm not saying it's a good idea even most of the time, but I Think it's a good idea sometimes now What this stuff does is basically walk through the the image starting from one corner point of box one and Like like looking at the neighboring pixels just like brute force walking through The connected pixels that happen to be black until either it's Covered all of the possible neighboring pixels Locate the boxes then we follow the line and make sure that there's no gaps You know we we might be able to tell the computer how to do that Whether it makes sense or if it's worth the the effort, you know, maybe most of the time not but it's still an option Yeah Right Yes So if he if you have a test model, it's it's it can be a problem with how you wrote your Test code or it can be a problem with the how you wrote your Production code is true True, yeah, we shouldn't test the other people's code And in this case, there was no third-party API. We're directly using the Java awt something But you're very much correct Well Yeah, so one more solution alternative solution to this would be sort of the golden master approach that we could we could get rid of all of the the sort of Algorithm stuff we could get rid of all of the oops the pixel stuff If we just render it once and say Yeah, that looks correct and then make that a golden master save that into a file and then in your test You know render it again Like here, but just compare it to the whatever the golden master is on this if you change Something that changes the appearance of the boxes or the line or the shape or size of things you'd have to probably Re-evaluate the the correctness of the render But it might be okay if if you if you manage to isolate that that The diagram you're drawing To a small enough that you can you can trust that it it works and it doesn't break too often or it doesn't change too often So that that's also another alternative that we could do But personally I found this very entertaining so I wanted to do this so Does that was the the set of samples This is just eight samples eight smells Some of them are very much related. There's a bunch of more in the book that's Apparently, it's not available yet. I mean not in in print, but you can get an e-book very easily already today if you want to Follow me on on Twitter. I'm Lasso Koskala I Don't have an email here, but I'd be happy to discuss this over email. I I won't respond quickly, but I will So I'd be happy to continue discussions. We still have a few minutes left if you have questions about the smells Or the other smells in the book Yeah It's the I Have no idea so the the publisher is Manning publications and all of their book covers are from this set of old pictures that the the publisher that the owner of the company bought at some point they bought the rights to these pictures and I guess he keeps them in the safe somewhere and whenever they publish a new book The biggest decision he makes is really which image will I use? Personally I kind of wish that they run out of pictures, but you know, I like the I like the mustache Not really no All right. Thank you. I hope this was useful and I I hope that I sparked an interest in you in you looking at your test code a bit Maybe with slightly different eyes And if you'd like help, I'm sure you can find a another person with the same passion very close to you. Thank you