Delicious Controversy: Docs & Tests

Talks

Thomas Meeks

@thomas

#documentation

#testing

#test-driven-development

Delicious Controversy: Docs & Tests

by Thomas Meeks

In the talk "Delicious Controversy: Docs & Tests" by Thomas Meeks at Rails Conf 2013, the speaker challenges the conventional wisdom around self-documenting code and test-driven development (TDD), advocating for a practical approach to documentation and testing in software development. Meeks reflects on experiences gained while working on a project called Code School, which involved managing a codebase of over 27,000 lines across multiple developers.

Key Points:
- Documentation Myths: Meeks begins by criticizing the notion of self-documenting code, arguing that it is unrealistic. He explains that while clear method naming can indicate functionality, it often fails to convey crucial context that new team members may require to understand the system's workings.
- Spec Documents vs. Specs: The speaker shares his viewpoint that specs cannot substitute for proper documentation. He highlights that while integration tests (specs) serve a purpose from a user perspective, they do not effectively communicate programmatic details necessary for understanding the code's functionality without exhausting effort.
- Approach to Documentation: He advocates for documenting code as it is written to avoid the burden of recalling array logic and other details complacently at a later date. Meeks suggests documenting public methods succinctly, focusing on what parameters the method requires, what it does, and its potential side effects.
- Critique of Unit Testing and TDD: Meeks critiques unit tests, categorizing them as often resulting in over-engineering and unnecessary maintenance. He emphasizes integration tests as a solution to ensure that features work as intended and encourages adoption of a holistic approach to testing that allows developers to consider the full stack.
- Real-World Case Studies: Meeks utilizes practical examples, such as subscription management, to underscore how improved documentation and integration testing can lead to smoother development processes. He illustrates how using service classes and presenters can create cleaner, more maintainable architectures while improving the clarity and robustness of tests.

Conclusions: The primary takeaway of Meeks' talk is the importance of prioritizing practical strategies over strict adherence to TDD and the myths of self-documenting code. By fostering an understanding of core principles and iterating on documentation practices, teams can enhance developer happiness, reduce technical debt, and make significant strides in productivity. Meeks emphasizes that happy programmers lead to more effective team performance, arguing that operational efficiency should come from streamlined code management, not overwhelming documentation or testing burdens. He concludes by encouraging developers to determine what techniques work best for them, promoting a culture of flexibility and critical thinking in software engineering.

00:00:15.980 Hello everyone! I'd like to start off by getting a quick count. Raise your hand if you went to Sandy's talk. All right, very cool! This one is kind of related to it, and I mostly agree with everything she said.

00:00:23.630 I would like to say, too, that if you welcome this talk while disagreeing with everything I say here, please, for the love of God, follow what she said. So I wanted to start with something very delicious: black claw, and the talk is delicious controversy.

00:00:36.910 I'm going to talk about documentation and tests. So, why am I giving this talk? It is actually kind of scary to get up in front of the best and brightest and give a talk. You might think it's pretty controversial, but I'm doing it because Code School is a pretty interesting project, and we've learned a lot while doing it.

00:00:47.780 We have over 27,000 lines of code, including aspects, 29 committers, and lots of rotation. You know, NV Labs also does client work, so we have people coming on to Code School and moving back off onto client work sometimes once or twice a month. There are a lot of lessons learned from doing this.

00:01:04.220 When we started Code School, we drank a lot of the kool-aid about test-driven development being the golden path to good tests and good design, that code can be self-documenting, and things like that. We learned that not all that kool-aid is necessarily practical.

00:01:18.160 So I'd like to spend the next 30 minutes or so being kind of a champion of practicality and see where that takes us. I want to start off by talking about documentation. Documentation kind of has a bad reputation.

00:01:26.650 Personally, one of the last classes I took in college was software engineering, and in that class, they asked me to write a spec document for a vending machine. That spec document ended up being well in excess of 30 pages, and we had two people working on it. I was not nearly as long as the teacher wanted, which was pretty ridiculous.

00:01:41.220 Spec documents are one extreme, and self-documenting code is the other extreme. This is kind of the premise of self-documenting code: you have a class called Subscription, you have a method called cancel, you write your code in a concise and clear manner, and people reading the code can understand it.

00:01:58.000 Then you don't need to write documentation, right? Because it's obvious what cancel does; it cancels the subscription. Well, that might not tell you a few things; maybe it sends an email to the client telling them that their subscription has been cancelled or maybe it refunds them for their partial month.

00:02:12.739 Does this happen at a job? Does it not happen in a job? The term 'cancel' doesn't tell you these things. So then the first thing we programmers do is add a bang to it. 'This is a dangerous method.' Oh yeah, that doesn't really tell me anything. Does that mean it's going to throw an exception or that it's going to wipe the server? I'm not really sure.

00:02:34.000 Self-documenting code is a total pipe dream, and that is not necessarily intuitive. When you're writing code, you have the benefit of all the context at the time you were writing it, and when you come back to look at it six months later, your brain does this amazing job of recalling the context as you're reading the code again.

00:02:43.890 But when one of your peers comes and looks at your code, that’s just not true. They have to search around to get the same context that you had, especially if they’ve never worked on the project before and especially if it’s something that’s complex like subscriptions.

00:03:01.030 It's kind of like a puzzle piece. Every method you write has a well-defined API like a puzzle piece. You can kind of tell that this puzzle piece is probably a leaf or something like that—it's very easy to understand just by looking at it for a few seconds. But when you go into an application like Code School, it's like being thrown into a sea of puzzle pieces and being asked to figure it out without the benefit of a picture of what that puzzle is going to look like.

00:03:20.610 You can do that; you can start from the corner and figure out each puzzle piece individually and put the pieces all together. So it can be done, but it takes way too long, especially if we have people who are only going to work on Code School for just a couple of weeks and then move back onto a client project.

00:03:34.320 They'll spend the next week figuring out what they need to do, and it's also really soul-crushing. Everybody at NV Labs has a very healthy respect for each other, and when I read code written by one of my peers, I respect them, and if I can't figure it out, that makes me feel stupid.

00:03:45.480 It doesn't matter how long I've been programming; I've been programming for 10 years, but that doesn't necessarily matter. Then the next part is when people say, 'All right, in addition to self-documenting code, we're going to use specs. We're going to use Cucumber; these things are our documentation! They tell us what our features are.'

00:04:01.820 I would like everyone to imagine for a moment if the Rails core team deleted all of their API docs and told you to go look at the specs. It doesn’t work that way! Tests are terrible documentation. It’s not the right mindset when you’re writing the features. That’s something that is from the user perspective; it doesn’t tell you what you need in order to use the class.

00:04:17.190 Plus, we are essentially asking people to understand 11,000 lines of code by reading 16,000 lines of our spec. You're putting your peers on their back and stretching them to their limits on the very first day. It doesn’t work.

00:04:31.290 Documents exist so others can use classes without understanding them. It would be impossible for any of us to get very far in using Rails or even Sinatra if we had to deeply understand how the entire system worked. We don’t have enough hours in the day to do that.

00:04:45.420 So how do you make documentation useful? It’s really not that hard. You want to document all of your public methods and you want to document as you go. It’s really important to document as you go.

00:05:04.300 You're going to have a lot of pain writing documents up front if you don’t have it in your codebase. I sat down and documented an 800-line class and it took me the better part of a day because when you document something, you're not just looking at that 800-line class; you also have to look at all of the other classes that it uses.

00:05:22.850 You want to keep it very short and you never want to discuss implementation. As soon as you discuss implementation, you've created maintenance overhead. Instead, it just sorts—it doesn't implement bubble sort or quicksort. It just sorts.

00:05:34.620 You want to answer three questions: What does the method need? That goes beyond just parameters; that also includes the state of the class. Maybe you should only really call a subscription's cancel method if the subscription isn’t already canceled.

00:05:47.310 What does the method do? This is the obvious part; it cancels the subscription. And you need to say: What are the far-reaching consequences? It emails the user, refunds their money immediately, or creates a job that refunds their money at some later point.

00:06:00.090 These are the things that people need to know so that they can use the subscription class without emailing 10,000 customers or deleting all of the data in the database.

00:06:13.040 In short, you need to treat every single thing you write like a public API. There are a lot of amazing documentation resources for many of the open source projects that we use out there, but in our internal applications, we usually do a very poor job of emulating those docs, and generally, I think that should change.

00:06:25.780 Ideally, you're going to write your docs so that you could put it up on GitHub and other people who have never seen your code can use it without understanding it. This makes all of your peers around you much faster; you’re lifting up the entire team by doing this.

00:06:42.910 Maybe somebody needs to implement something really quick using subscriptions and instead of spending all day figuring out what exactly is going to happen in subscriptions, they can let go of that fear because you've documented it and they know that calling cancel isn't going to destroy the user's data.

00:06:56.630 You want to give them the puzzle; you want to give them a picture of the puzzle so that when they start putting the pieces together, they know if they're on the right track.

00:07:07.550 So the next subject is testing. Testing is a minefield of opinions; it is always a very interesting subject. We have 16,000 lines of specs—that's 5,000 more lines than application code. I feel like the testing culture is such that I could get up on stage and feel proud about that.

00:07:19.190 I've written a 500-line application with a million lines of specs, and I’d get a standing ovation because 2,000 percent code coverage is a good thing, right? Well, we found that those 16,000 lines come with maintenance.

00:07:32.640 Just because they're tests doesn't mean that they come for free. So why don't we write tests in the first place? There’s really only one reason: to prevent bugs. You want to know six months from now if the feature you wrote today is still working—that's it.

00:07:47.620 If you have them for another reason, please get rid of them. I want to talk about two broad categories of tests here: one is unit tests, and the other is integration tests. Unit tests, defined very broadly, are tests that test the method without needing the class.

00:08:01.610 For example, if you are testing something on the subscription, you would probably mock out the user. I have a very strong opinion about unit tests—they are bad tests, just categorically bad tests. There are way too many lines of test code for each line covered in your application.

00:08:16.830 If you have a one or two-line method, when you write a unit test, you're going to instantiate it, mock something out, run that method, and assert something, maybe with two assertions. That can lead to five lines for a one or two-line method.

00:08:30.380 You can toss that out over an 11,000-line application, and you're in trouble. It gets really ridiculous really fast. They’re too coupled to the implementation, precisely because you're mocking things out, especially if you've made the mistake of stubbing internal methods.

00:08:44.930 That's a horrible, horrible test. I have a suggestion for that later on. They create too much maintenance, and they fail to ensure that you're using a class correctly. This goes beyond just making sure that you are treating the API in the class correctly.

00:09:01.070 It's about whether you're changing some internal state on the class that you are using and that internal state might break when you call another method on it later. These are the kind of things that only integration testing can capture.

00:09:13.750 Sometimes we hear the argument that you want to mock out things like queries because you don’t want to test Postgres directly, but that’s not the point. You’re not testing Postgres; you’re testing that you’re using it correctly. These are all very complex systems, and even your classes are complex in their own way.

00:09:30.740 Now I get to have a rant and have a little fun. So if you don't test or if unit tests are bad tests, that means test-driven development creates bad tests, and that's okay. Test-driven development is about good design, not good tests.

00:09:42.520 The reason you use test-driven development is that it forces you to use your code while you're writing it to make sure that you like your own API as you're developing it, which is useful. You can tell how useful it is by how passionate people get about test-driven development.

00:09:55.740 I remember reading a blog post while I was writing this talk that paraphrased: ‘If you don’t practice test-driven development on a day-to-day basis, you should reconsider your career as a programmer.’ That is utter nonsense.

00:10:09.230 I mean, can you imagine Linus Torvalds would not be a programmer if that were true? He did not create Linux with test-driven development. Plus, test-driven development does not guarantee good design, and we need to stop acting like it does.

00:10:25.240 I shed a tear every time I see a job posting that says we practice TDD all the time, expecting you to be an ultra-uber master at TDD to get this job. Because there are plenty of people that like TDD, and that's great. But you're also ensuring that you don't have a diverse kind of programmer at your office, and eventually, that may hurt you.

00:10:37.850 We've kind of gotten to the point for some where we’re worshiping our hammer and burning people at the stake because they do not practice TDD. I’m personally just kind of tired of meeting really great developers who are carrying around a lot of guilt due to the fact that they don’t practice TDD.

00:10:51.380 If you don’t do TDD, that’s fine. Use it if it works for you, that’s great! But please delete your unit tests after you replace them with integration tests. An integration test has the full stack, and they check the result.

00:11:05.380 Integration tests are never concerned with implementation, and they are beautiful. I love them. My favorite tests I've ever seen are in Code School. For example, when we create a course like jQuery or try jQuery, we have an API that allows us to send correct and incorrect answers through.

00:11:21.780 We have a very complete set of specs that send good answers, bad answers, even answers that try to hack the server or crash it. We don’t mock the executor. The executor is a whole other app completely separate from jQuery.

00:11:36.020 In fact, our executor is Node and our application is Rails, and we allow those requests to pass through to the executor that runs locally. So now we’re testing two apps with one set of tests. Do our executors have tests? Yes, they do. But the most important tests are the tests that we have in the course.

00:11:50.800 It's an amazingly efficient use of our time and resources because the tests are very easy to write. It’s testing our complex interactions, and we’re in the right mindset. We’re focusing on whether this feature works, which is what we really care about at the end of the day.

00:12:05.420 If somebody creates an account, is that account created? Can they add a course to their account? Can they play the course? These are the things we care about—not whether or not something calls another method.

00:12:20.090 In the Rails community, the first thing that comes to mind is Capybara. Capybara is the be-all and end-all of integration testing! You're going in through HTML; you test literally the full stack and you're checking everything back in HTML, seeing it as a user would see it.

00:12:32.670 No, no, no, no! Capybara takes way too long to implement. It’s an amazing piece of software—I have a lot of respect for the people who wrote it—but I don’t think you should use it for all of your integration testing.

00:12:42.850 If you have all of your tests in Capybara, you’re setting yourself up for major pain. There’s way too much maintenance; there are too many gotchas. It doesn’t encourage good code! If Code School were written, God forbid, with all 11,000 lines inside of the controllers, Capybara would test that just fine.

00:12:56.780 It wouldn’t care, and even worse, few changes tend to break tests. We have this problem with our designers where they change something in the view, and because we were a little too eager in our scoping, all of our integration tests fail, and Travis CI screams at everybody.

00:13:11.660 Nobody's happy. So, if you're not going to use Capybara, what are you going to do? I would like to suggest using same tests by testing your models, using services, and using presenters.

00:13:27.780 In order to get same tests, you have to architect your application in a certain way, right? If you have all of your code shoved into a controller, that’s difficult to test. I am generally not a fan of changing my code just to make it easier to test unless it's obviously poorly architected.

00:13:43.700 But I’m okay with this approach personally because it feels like the right way to do things, and you get a lot of other benefits from writing Rails apps this way. So a model—we're all pretty familiar with it; it's your core application logic.

00:14:01.940 It does one thing, and it does it well. Your subscription should not be handling credit cards directly; your users should not be handling subscriptions. Your courses should not be handling challenges; it's one thing and one thing only.

00:14:18.520 They’re not necessarily active record, and I think the community has mostly moved beyond that, but it's worth mentioning. So, let’s take a few examples. Consider a subscription model that has a method called charge, and all it does is grab a billing subscription and call charge on it.

00:14:34.310 If it works, it sets paid to true and increments the next bill due date. This is pretty easy to read, but it still doesn't tell you a lot. Does the charge happen immediately? What does the paid attribute mean? Does it stay true, or does it turn false the next day?

00:14:50.820 What is this billing subscription thing? I'm going to have to go read billing subscription now to find out if that’s a remote call or a local call—what does it do? Instead, document it! I tend to prefer YARD for documentation; it makes things really easy, but it really doesn’t matter what you use—TomDoc, RDoc, or YARD.

00:15:05.680 As long as you have some text above your classes and methods, this tells us a lot. Now we know ‘paid’ is true if the payment is current—so they’re paid up. That answers a question. It immediately attempts to charge—that's pretty important.

00:15:22.900 And it also updates the next bill due date. Now we don’t even need to read that code; we just read the docs. We also know, as a bonus, that it returns the date that the next charge should be attempted and that it throws an error if billing happens to be down. We wouldn't know that without some documentation.

00:15:36.020 Plus, we’re making billing objects private, so who cares what it does or how it doesn’t unless you are changing the subscription? You don’t need to care because the documents are telling you everything you need to know.

00:15:53.490 So how do you test something like that? I tend to like RSpec; you could also use mocks. This is one case where mocks are great because you don’t want to be talking to credit card companies when you run your tests, but it's very easy.

00:16:06.050 You create a subscription, you charge it, you assert that it's been paid, and you assert that the next billing date is what you expect it to be. Done! You've tested that method, and if you're using VCR like this, you've also indirectly ensured that it's using billing subscription correctly.

00:16:18.400 VCR is going to pretend with a request-response cycle and will return all the data you need. Services orchestrate models and handle logic that doesn't fit into a model, and they help your controllers become paper-thin, which is really important.

00:16:32.820 So imagine we have a controller: the create action. We're creating a new subscription, and we want to see if that subscription gets a discount or maybe everything gets a discount. It’s not that bad, but controllers have enough to do.

00:16:50.000 They’re handling your filters, security, and parameters. There’s enough going on there; they don’t need to know about your business logic. Plus, what if I want to be able to create subscriptions through a CSV file and run a rake task? I can’t do it like this, or I’d have to duplicate code.

00:17:05.640 Instead, create a service and make your controller ridiculously simple. We create the subscription service, ask it to create it, redirect, and render. Now the controller is handling the bare minimum.

00:17:14.880 What is the service like? When you document it, what does the implementation look like? Well, it tells us that we're creating a new service; we're applying a discount if it's appropriate, and it immediately charges.

00:17:28.840 It's carrying that documentation along with it. And when you test a service, it is equally easy. You create the service, pass in some good parameters, assert that the service created successfully, and check that the subscription was marked paid.

00:17:41.150 Normally, if you're doing a unit test, you would never, ever do service.subscription.due. But this is an integration test; we want to test the result of the operation. We want to ensure that the subscription is marked paid. If that means you have to do a database query inside of your integration test, that’s fine. That’s what they are there for.

00:17:55.740 We’re also making sure that the discount was applied. Presenters have been around for a while, and I'm seeing them used a lot more—it's great! They help display complex views, and they are template and framework agnostic.

00:18:06.920 Ideally, if you can—you can bend on that a little bit, that's okay—but ideally, your presenters don’t necessarily know about HTML, and they help controllers and views become paper-thin, which is again really important.

00:18:17.460 Before you have a presenter, you get something like this, where the logic you've got in your view is not that bad. This is pretty easy stuff, but compounded across 11,000 lines of code, it doesn't work because now you really need to test that with something like Capybara.

00:18:29.200 Instead, use a presenter. There’s really nothing to test there. The bug that you could have would be misspelling a method name. If you really feel like you need to test to make sure that you're not misspelling method names, that’s fine. But personally, I don’t think that's a useful test.

00:18:39.720 You don’t get a lot of value out of the lines that you have to write and maintain when you write a test like that. Presenters themselves are really easy to write; it’s very quick to write presenters usually. And again, documentation does not take that long.

00:18:51.380 It’s usually one line, and you don’t have to debug documentation, which is kind of nice. Finally, testing a presenter—you create one and you just assert that it works. Technically, we actually use RSpec for all our stuff, but these make for shorter examples.

00:19:05.080 So what about controllers and views? They should be so simple that testing them is a complete waste of your time. Controllers and views are configuration. We’ve already got plenty of templating languages out there that do not allow logic inside the template because it’s just generally not a good idea.

00:19:20.000 Controllers are barely complex enough now that you cannot express them in YAML, but you almost could. That’s how every controller should be written because if you’re using services and presenters instead and you decide, ‘Hey, I would like to write a Ruby Motion app,’ then you've got something to start with.

00:19:35.480 You can use the services and presenters inside of something like a Ruby Motion app or a command-line application. Maybe you don't, but it doesn’t matter; it still makes your test much easier because you're not having to deal with HTML.

00:19:51.440 Even if you have to pull parameter processing and redirects into a service, that's fine. I am next week going to write the service that handles flash notifications because they were just complex enough that I didn’t feel like they belonged to the controller anymore.

00:20:09.500 Whatever it takes so that you can keep your application fast, easy to understand, and easy to test. We don’t want skinny controllers; we want skeletal controllers. We want just the bag of bones walking around—that’s it.

00:20:23.890 Some guidelines: These are just guidelines; they’re not religion. Every class should be treated as an API defined by its public methods. The method should only be public if it absolutely must be. There is a lot of cost to having a public method.

00:20:38.440 If you're doing anything like this, you have to document it, and you need to write an integration test for it. You need to document your public methods, feature an integration test for them, and remember as you're writing your tests and writing your application that every line of code you write has a maintenance cost.

00:20:50.650 Even if it's tests, in a perfect world, you would have fewer lines of spec code than application code. I'm not sure that's possible, but I would like to try it one day. I’d like to shoot for that, but it’s hard to get there when you're starting from 16,000 lines.

00:21:09.360 You want to refactor slowly. The big refactor is the cousin of the big rewrite. We are taking it very slowly. We still have plenty of Capybara tests, and we still will by the end of the year, but we are taking things out of our controllers, creating services, and testing those services.

00:21:25.470 After that, we get two sets of integration tests passing, delete the Capybara tests, and like I said, these are just guidelines. Spin them freely. If something makes more sense and it doesn’t follow these guidelines, do it: there are plenty of exceptions; there are always exceptions.

00:21:40.610 No rule is set in stone. If you have especially critical code, I don’t think we will ever get rid of Capybara tests on the happy path for a user creating an account, charging their credit card, finishing all their information and sorting the course.

00:21:54.540 If we can’t do that, we’re out of business, so it makes sense to test that and deal with the extra maintenance of a Capybara test for something like that. It does not make sense to test and ensure that the word 'completed' appears on your account page.

00:22:11.310 We are not out of business if that word happens to not appear. You may be annoyed, but we can deal with that; we’ll give you a discount code that breaks frequently, whatever the reason may be.

00:22:30.640 It's a class that a lot of people touch, so go ahead and write a unit test for it. Go ahead and spend the time to make your unit tests really fast. One of the main reasons I really don’t like speed, though, as an argument for unit tests, is that—think to yourself—how long does it take for you to get bored when you’re watching the dots?

00:22:44.010 For me, it’s about six to ten seconds, and unless you’re willing to spend the time and invest all the money required to make your unit tests pass in six seconds for a Rails app, especially if you have to initialize Rails for your tests, which takes three to four seconds on a good machine on its own, you’re wasting your time.

00:22:58.450 Because you're going to check Twitter and come back in five minutes anyway. Use Travis or set it off into another window and keep coding. HTTP APIs: you have to test those from the outside because the headers—the response headers—are an important part of the contract.

00:23:09.970 Right? Does it return 200? Does it return 503? Does it return 404? Whatever—that’s the part of the contract you have to test; you always have to test the contract. A good mindset is an important part of the reason that I'm doing this, at least for myself, is to create the right mindset to write good code.

00:23:23.170 So if I don’t have any controller tests, I'm going to write really simple controllers because I don’t want to get tripped up on some stupid mistake in my controller. If I'm not testing my views, I'm going to have extremely simple views.

00:23:40.110 If I have to write tests and documentation for my public methods, I'm going to make sure that they need to be public. This is an issue in the Ruby community, where we tend to make everything public. Private is a good thing; you want to encapsulate your classes.

00:23:53.950 Make sure that people can’t poke in and call methods they shouldn't be—no private method tests! This means you’re going to have good feature tests. If you run code coverage and you think you have good feature tests and a private method has zero coverage, then that method should not exist.

00:24:06.700 Less time trying to understand code means you're going to be happier programmers. Less time trying to maintain tests means happier programmers too. I mean, who here has had the experience of spending two hours working on a feature and four hours fixing the test for it? I see a lot of hands.

00:24:18.390 It infuriates me. It ruins my day. I know it ruins a lot of other people's days too, and it shouldn’t happen. Working the way that works for you means happier programmers.

00:24:32.930 I think that we programmers are always looking for ways to get more productivity out of ourselves, but we also are really good at forgetting that you need to be happy as a baseline to get any productivity out of yourself at all. Your team needs to be happy.

00:24:45.360 Crushing people over their particular choice of tools is not a good way to get there. And that’s it! If you would like to see the slides, they are at Thomas Meeks dot com slash controversy.

RailsConf 2013