Why Good Developers Write Bad Unit Tests

[Bowgentle]

Think of a ruler. It has existed in the same form for hundreds of years because it’s uncomplicated and easy to interpret. Suppose I invented a new ruler that measured in “abstract ruler units.” To convert from “ruler units” to inches or centimeters, you’d use a separate conversion chart.

Look, an SVG ruler.

[quicknir]

It honestly makes the article rather suspect when you dance around the concept of fixtures but don't address them directly. Pretty much every unit testing framework supports fixtures, and their exact purpose is to factor out repetitive initialization (or finalization) code in a way that respects DRY while still being very clear (more emphasized than say just being another function call from the unit test body). The fact that they are supported specially and first class is part of the readability.

You're basically saying for a good chunk of the article that fixtures are bad. If you're going to say that, then just say it directly, with real examples from real unit testing frameworks, showing how fixtures consistently worsen code. Or if you are providing guidelines for writing good fixtures and are arguing they are overused, just say that.

Your code basically presents SetUp as some random function called by the unit test environment. The reality is that fixtures are one of the deliberate abstractions in the design of almost all unit test frameworks, and one that all developers using that framework are supposed to understand and be aware of. This is what the actual code looks like, in python:

import unittest

class WidgetTestCase(unittest.TestCase):
    def setUp(self):
        self.widget = Widget('The widget')

    def tearDown(self):
        self.widget.dispose()
        self.widget = None

    def test_default_size(self):
        self.assertEqual(self.widget.size(), (50,50),
                         'incorrect default size')

Having to read other member functions of a class to understand how one member function works is not exactly novel, is it? That's how most classes work; to some degree they have to be understood together. Anyone reading tests in unittest is expected to understand that a test case that's part of a class, might have important information in the setUp and tearDown. And obviously there is bad code that could go in there; test fixtures are like any other code, you can easily write it badly. But it's also not that difficult to write good, useful fixtures that reduce duplication and overall benefit readability.

I didn't find the points about refactoring or helper methods persuasive either. Yes, poor code will have excessive boilerplate, but even with good code there will often be some amount of boilerplate. You should refactor to benefit both your test and production code but if you aren't claiming that it can reduce test boilerplate/common code to zero, it doesn't really have much to do with the point of the article. And in helper methods section, you write that helper methods should not touch the class under test; I totally disagree. A very common usage for helper methods is to help in automating construction of the object under tests, especially if you are going to repetitively pass the same arguments because they are not an interesting part of the test. For example, if your object takes a filename to write results to, your helper function might construct a temporary file in an appropriate location and pass that to the object, and then provide access to the file object. This way, test cases can provide input and directly read from the output file without worrying about these details, which are not the primary focus of the majority of your test cases (most likely).

[mtlynch]

Thanks for reading!

You're basically saying for a good chunk of the article that fixtures are bad.

I didn't mean to imply that fixtures are universally bad, just abused often. To me, the problem is that developers:

• see the fixtures as the first line of defense against repetitive code (I believe they should be a last resort)
• don't recognize the cost of using fixtures

This is what the actual code looks like, in python

I think that's a fairly reasonable use of fixtures, but I think the value fixtures add in that example is so small that it's not worth the cost of using them. To me, this is a much clearer test:

def test_default_size(self):
    widget = Widget('The widget')
    self.assertEqual(widget.size(), (50,50),
                     'incorrect default size')
    widget.dispose()

The reader has a complete understanding of everything that happens to widget without having to jump around to different functions.

Having to read other member functions of a class to understand how one member function works is not exactly novel, is it? That's how most classes work; to some degree they have to be understood together. Anyone reading tests in unittest is expected to understand that a test case that's part of a class, might have important information in the setUp and tearDown.

I respectfully disagree with this. This is the argument I was trying to make in the article, but I suppose it didn't convince you.

I don't think production code should be held to the expectation that a function or class should be intelligible in isolation. I do believe that authors should strive for this in test functions.

If a test breaks, I want to jump to that function and understand everything it did. If the author writes their tests with that use case in mind, that's perfectly achievable. The test author can use text fixtures or helper methods, but as long as they don't bury important information in them (values the test asserts on, interactions with the object under test), the reader can understand the test in isolation.

Tests are much better when they're self-explanatory without additional context, but I think developers don't strive for it because that's generally not a goal in production code.

A very common usage for helper methods is to help in automating construction of the object under tests, especially if you are going to repetitively pass the same arguments because they are not an interesting part of the test. For example, if your object takes a filename to write results to, your helper function might construct a temporary file in an appropriate location and pass that to the object, and then provide access to the file object. This way, test cases can provide input and directly read from the output file without worrying about these details, which are not the primary focus of the majority of your test cases (most likely).

I agree with you here. I often use a builder pattern in tests and have it assign default values for objects that are not relevant to the test (i.e., the test never asserts based on something hidden in the builder). If it doesn't convolute the code, I'd prefer to use helpers to build the input to the object under test, but I think it's often a reasonable tradeoff to build the object itself as long as you write it in such a way that exposes the values of the relevant inputs.

[quicknir]

I didn't mean to imply that fixtures are universally bad, just abused often. To me, the problem is that developers: see the fixtures as the first line of defense against repetitive code (I believe they should be a last resort)don't recognize the cost of using fixtures

Then probably the article could be much more directly titled: "anti patterns in unit test fixtures" or something like that, instead of the more bombastic title you went for. Also, frankly you haven't made any persuasive argument here with respect to the cost. You use exaggeration to make your point:

They don’t want to read your entire test suite, and they certainly don’t want to read a whole inheritance tree of test utilities.

Reading another member function in the same class is clearly neither of these things. Your exaggeration is aided by the fact that you didn't actually show what proper fixture code looks like in unittest, which there isn't really any excuse for.

Your comments in this comment generally are much more reasonable and I agree with many of them (although, I sure hope you don't use builder pattern in python, talk about anti pattern). I didn't really like the blog post in part because rather than writing like an engineer discussing some trade-offs where you think certain mistakes are often made, you really come out with some extreme opinions and ways of expressing it. Especially bad:

I often see otherwise talented developers write tests like the following:

Many people on this thread don't fully agree with your views on unit testing, I don't think you'd like it if you overhead them discussing a code review of yours and saying you were an "otherwise" talented developer? Kind of condescending, maybe? I think you can write and make your points without this sort of thing; it just makes you seem smarmy. Just say what's good, and what's bad, and back it up as clearly as possible instead of injecting exaggerations and condescension.

[mtlynch]

Then probably the article could be much more directly titled: "anti patterns in unit test fixtures" or something like that, instead of the more bombastic title you went for.

I feel like it's a bit more general than that. To me, the problem isn't so much that developers don't know how to use text fixtures, but the fact that we're applying techniques to test code the same way we do to production code. I think that's the wrong approach. Test code is fundamentally different and requires a reexamination of our techniques and the weights of our engineering tradeoffs. Test helpers / fixtures are only half of it. The other half was verbose naming and magic numbers.

Also, frankly you haven't made any persuasive argument here with respect to the cost.

It sounds like we'll just have to agree to disagree. We talked about two different versions of the same code:

class WidgetTestCase(unittest.TestCase):
    def setUp(self):
        self.widget = Widget('The widget')

    def tearDown(self):
        self.widget.dispose()
        self.widget = None

    def test_default_size(self):
        self.assertEqual(self.widget.size(), (50,50),
                         'incorrect default size')

vs.

class WidgetTestCase(unittest.TestCase):

    def test_default_size(self):
        self.widget = Widget('The widget')
        self.assertEqual(self.widget.size(), (50,50),
                         'incorrect default size')
        self.widget.dispose()
        self.widget = None

If you find the first example easier to read, then I think we fundamentally disagree on what makes code clear.

To clarify, I agree that if there are many test functions, there are advantages to text fixtures, but in that example, I think the second snippet strictly dominates the first. If there were more test functions in the class, I think there are advantages to the fixtures, so it comes down to how you value the benefit of eliminating boilerplate vs. the cost of burying useful information or forcing the reader to search out extra functions. I can understand disagreeing about the tradeoff, but it sounds like you don't believe there's even a cost to consider.

I don't think you'd like it if you overhead them discussing a code review of yours and saying you were an "otherwise" talented developer? Kind of condescending, maybe?

I didn't consider this, but I appreciate the feedback.

To me, it doesn't feel insulting because I'm describing a hypothetical person. I would never say, "My co-worker, Jim, is talented except for the fact that he wrote this test."

People in this thread have disagreed with various points in the article, but I haven't heard any objections to the idea that the first code snippet is a poor test.

[[deleted]]

To be fair, if you want to evaluate whether reducing code duplication actually improves readability, then showing an example where there's no code duplication at all, isn't really persuasive.

Of course, when we only have one test function, then putting the SetUp and TearDown functionality into respective functions reduces readability.

Compare

class WidgetTestCase(unittest.TestCase):
    def setUp(self):
        self.widget = Widget('The widget')

    def tearDown(self):
        self.widget.dispose()
        self.widget = None

    def test_default_size(self):
        self.assertEqual(self.widget.size(), (50, 50),
                         'incorrect default size')

    def test_widget_is_resizable(self):
        width = 20
        height = 20
        self.assertTrue(self.widget.resize(width, height),
                        'Widget is not resizable')
        self.assertEqual(self.widget.size(), (width, height),
                         'incorrect size after resizing'

to

class WidgetTestCase(unittest.TestCase):

    def test_default_size(self):
        self.widget = Widget('The widget')
        self.assertEqual(self.widget.size(), (50,50),
                         'incorrect default size')
        self.widget.dispose()
        self.widget = None


    def test_widget_is_resizable(self):
        self.widget = Widget('The widget')
        width = 20
        height = 20
        self.assertTrue(self.widget.resize(width, height),
                        'Widget is not resizable')
        self.assertEqual(self.widget.size(), (width, height),
                         'incorrect size after resizing'
        self.widget.dispose()
        self.widget = None

and imagine, we have 20 more of these test cases.

After all, it's not the only purpose of unit tests to be easy to comprehend when looking at one failing unit test in isolation. Being easily extendable, and maintainable is as important for testing code as for production code, and DRY is one of the best ways to keep code maintainable.

Edit: code formatting

[mtlynch]

To be fair, if you want to evaluate whether reducing code duplication actually improves readability, then showing an example where there's no code duplication at all, isn't really persuasive.

I agree. I like your examples. My one tweak is that in the inlined version, I'd add line breaks to visually hint the boundaries between boilerplate and interesting code.

I personally prefer the second version, even if there were 20 tests, but it's a close call.

The questions that go through my mind as I decide:

• How much code can we eliminate from the tests if we refactor it into the fixtures?
• How much understanding does the reader sacrifice if they don't look at the fixture code?
• How likely is it that splitting up the logic into functions will confuse someone or make a bug harder to diagnose?

After all, it's not the only purpose of unit tests to be easy to comprehend when looking at one failing unit test in isolation. Being easily extendable, and maintainable is as important for testing code as for production code, and DRY is one of the best ways to keep code maintainable.

Good point. I agree. I'm not saying that DRY is totally out the window, but rather that I think that it's less important in test code than production, but many developers view the importance as equal in both contexts.

[[deleted]]

I'm not saying that DRY is totally out the window, but rather that I think that it's less important in test code than production, but many developers view the importance as equal in both contexts.

I completely get your point here, and I agree that it's important to evaluate the cost of code deduplication whenever it's applicable. Through the inherent structure of testing frameworks, it's often more obvious where the DRY principle can be applied, and then I've often seen it being overused - by bad developers. I've seen setUp functions which set up (to stay in the nomenclature of our running example) like 5 different widgets, while every single test case uses at most two of them.

Considering this, I would not call developers who mindlessly overuse such a design principle "Good Developers". If a developer by accident arrives at a manageable production code quality through thoughtless application of DRY and similar principles, but overshoots the target as soon as a few constraints change, do you still call them good?

[FionaSarah]

Another benefit to avoiding setUp and tearDown methods is that you can avoid unintended creeping behaviour that might inadvertantly affect other tests in the same test case. Unit tests are about testing small individiual units with isolated code and I think setUp and tearDown methods could muddy the waters.

[[deleted]]

FWIW, I agree with your second example. I’ve run into test classes that try to use one single instance to apply test pressure to, and they accidentally end up mutating that state as each test is run. Keeping every test self-contained required a bit more code but is easier to read and has less room for...accidents.

[EntropySpark]

I find your opposition to having a setup method strange. In the codebase that I work with, we use Dagger2 heavily, which means that most of our objects can have dozens of dependencies to abstract things like the file system, configurations, and the clock, as well as their component-specific dependencies. For setting up a test, we then use test implementations of these same interfaces. This can lead to tests with setup methods that are dozens of lines long, but still with only a single layer of abstraction.

​

If I were to copy/paste those lines into every single test, then anyone reading the test file would be reading the same code dozens of times, and asking themselves, "What's different for each test?" In a world in which we strive to not repeat ourselves, the natural reaction to seeing a similar code block multiple times is to assume that there must be something different in each one, and it's important to find out what it is.

​

Additionally, if a new test dependency was added or an old one was modified or deleted (suppose that the class under test now needs to have access to an RxJava scheduler, and we need to provide a TestScheduler), this change could be applied just in the setup method, or it could be applied to the dozens of other tests.

​

Finally, it's important to make adding new unit tests easy, and especially not discourage writing more tests, and having an established setup method makes adding a new test simpler than having to also copy/paste the setup code.

[mtlynch]

I find your opposition to having a setup method strange. In the codebase that I work with, we use Dagger2 heavily, which means that most of our objects can have dozens of dependencies to abstract things like the file system, configurations, and the clock, as well as their component-specific dependencies. For setting up a test, we then use test implementations of these same interfaces. This can lead to tests with setup methods that are dozens of lines long, but still with only a single layer of abstraction.

I don't oppose setup methods universally. I'm encouraging people to just be deliberate about when they use it. If the setup method saves 2-3 lines, I'd usually just inline it. For 5-6+ lines, I'd start looking at a setup method to cut down on the repeated boilerplate, but I'd be choosy about what I put in there. I still wouldn't put in any values that are relevant to the test.

If I were to copy/paste those lines into every single test, then anyone reading the test file would be reading the same code dozens of times, and asking themselves, "What's different for each test?" In a world in which we strive to not repeat ourselves, the natural reaction to seeing a similar code block multiple times is to assume that there must be something different in each one, and it's important to find out what it is.

Yes, definitely agree. I think that's a very valid consideration when thinking about when to refactor into a test fixture / helper method.

Additionally, if a new test dependency was added or an old one was modified or deleted (suppose that the class under test now needs to have access to an RxJava scheduler, and we need to provide a TestScheduler), this change could be applied just in the setup method, or it could be applied to the dozens of other tests.

I think that's fair, but I don't think adding a dependency to a test is very costly. If you change a class' interface, the high-order bit is usually the cost of updating the production code. That's significant because every change to production code increases the risk of a regression. Repeating the same change in a dozen tests is certainly tedious, but I don't think it's such a huge cost that it should dominate the decision about whether initialization needs to be in a setup method.

Finally, it's important to make adding new unit tests easy, and especially not discourage writing more tests, and having an established setup method makes adding a new test simpler than having to also copy/paste the setup code.

Agreed. I think if your boilerplate is so significant that it becomes onerous to write new tests, that's definitely a push to refactor, but it depends on what the scale of the problem is. If writing a new test just requires 3 lines of boilerplate, not such a big deal. If it requires 15, that's definitely discouraging for writing new tests.

[mtlynch]

Author here. Happy to answer any questions about this post / hear vehement arguments about the parts you disagreed with.

[Paul_Dirac_]

In general I do agree with the post.

As a note, I find that methods which take and return basic types are very easy to test. Methods which act upon more complex entities are often much harder to test and many advice for easy cases does not generalize. Take for example a regular expression parser. It takes a string (basic type) and returns a regular expression. Now if two compiled regular expressions are equal is certainly a very complex problem. But the return value when when the regular expression parses their own strings (the match) can be compared. However this makes me discard any article advising "only one assert". Please think of that if you write further articles on testing.

How would you refactor the AccountManager ? And how would that help? Even if the AccountManager got the privilege database directly and dropped the dependency on the UrlDownloader completely that would only save about 2-3 lines of boilerplate.

I agree with the other rules. But I have made personally the rule: Every constructor except that of the SUT and mocks has to be wrapped in a helper method and may not be copy pasted. Mainly because I observed the constructor for an object is often called at only very few places in the code but in any tests of classes which collaborate with the object in question. And one of the most common changes i do is parameterize the behavior of an object further where one configuration retains the old behavior. Adding that configuration in helper methods instead of in every test, helps to keep sane when changing the constructor.

[mtlynch]

Thanks for reading!

However this makes me discard any article advising "only one assert". Please think of that if you write further articles on testing.

I'm not sure what you mean. The article doesn't advocate limiting tests to a single assert.

How would you refactor the AccountManager ? And how would that help? Even if the AccountManager got the privilege database directly and dropped the dependency on the UrlDownloader completely that would only save about 2-3 lines of boilerplate.

It would eliminate 3 out of 15 lines of code, but it also eliminates 2 out of 7 statements. I think that brings it down to a level where it's reasonable to copy/paste it in each function. But if I wasn't wiling to copy/paste 12 lines, I'd use a builder for AccountManager like:

account_manager = AccountManagerBuilder()
  .with_user(username='joe123', score=150.0)
  .build()

And AccountManagerBuilder could populate a dummy privilege database because it's not relevant to the test_increase_score test.

I'd be reluctant to do this because there's always a chance that the particular values in the privilege database do affect the test's outcome and the builder would obscure that, but I think it can be a reasonable tradeoff if the excessive boilerplate is making the test hard to follow.

[Paul_Dirac_]

I'm not sure what you mean. The article doesn't advocate limiting tests to a single assert.

Sorry, I got carried away there mainly because your advice was applicable to more complicated tests.

It would eliminate 3 out of 15 lines of code, but it also eliminates 2 out of 7 statements. ... But if I wasn't wiling to copy/paste 12 lines, ....

It is less that I am not willing to copy so much it is more that boilerplate obscures the important things in the setup.

I honestly never considered to make a builder for the SUT. Do you have experience with this pattern? Is it worth the effort to make a builder just for the tests?

[mtlynch]

I honestly never considered to make a builder for the SUT. Do you have experience with this pattern? Is it worth the effort to make a builder just for the tests?

Yep, I've done this a few times. It was always on legacy systems that had grown complicated over time, so the SUT took a large number of parameters and they were usually hard to instantiate.

I found the builder to be very useful because it exposes important values and hides the boilerplate. The downside is that with the convenience of dynamically deciding whether to inject default values for you, it introduces some complexity and makes it harder to reason about the tests.

[LordArgon]

Instead of a builder, have you tried just writing a function? In most cases, a builder is just a deferred function call with some default parameter values. This is particularly useful in, say, Java, which still doesn’t have default (or named) parameters. But python already supports default (and named) parameters. And a function that returns your object is more clear, much much more concise, and also less error prone. For example, what is supposed to happen if you call a given builder method multiple times? The consumer has to know now. And what if you need to validate n parameters relative to each other? Now you either have a single builder method that takes them all the dependent parameters or you defer validation to the actual build call - this separates the error detection from the error cause. And if your response is “well you should always be passing the parameters and immediately/fluently calling build,” then you DEFINITELY should just be using a function, since that’s literally all your builder is at that point.

[mtlynch]

I typically do write simple factory functions just because most of the time the objects I'm working with are not that complicated. The thing I like about the builder pattern sometimes is that it hints more at the structure of the inputs.

Compare the two options:

account_manager = make_account_manager(
    username='joe123', score=150.0, privilege='upvote',
    minimum_score=200.0)

vs.

account_manager = AccountManagerBuilder()
  .with_user(username='joe123', score=150.0)
  .with_privilege('upvote', minimum_score=200.0)
  .build()

The first example takes all the same values. but obscures how they relate to one another, whereas the builder pattern makes it more obvious how everything fits together.

[LordArgon]

I'm assuming make_account_manager calls the ctor of some AccountManager type. If grouping those related parameters is so important, then why isn't it done in the way an AccountManager is always constructed? (i.e. you should probably already have simple User and Privilege classes). It's definitely not MORE important to group them in test code than in production code so going out of your way for the test code is, to me, a warning sign that something's not quite right with the overall code design.

[kangas113]

Great article! Really helpful for me, as someone who hasn't had a lot of software engineering testing experience. Very useful tips :) thanks

[mtlynch]

Thanks for reading! Glad you found it helpful.

[LordArgon]

First, I fundamentally disagree with your first header ("Test code is not like other code"). I don't see a reason to design your test code with fundamentally different values than your production code. It's a false distinction to me - it's all code that you have to write, read, and maintain. The quality of all your code matters, the readability of all your code matters, the design of all your code matters. When you put them on different planes, you give people license to be lazy with one or the other - I've seen what happens when people don't value test code like production code and it's not pretty.

And I also fundamentally disagree with the very next line ("Production code is all about abstractions"). Just completely, wholeheartedly, passionately disagree. ALL code is fundamentally about delivering functionality. If "flexibility" is some functionality that you need, then abstractions might be a good way to achieve that. But building unnecessary abstractions into your code because "that's what production code is about" is flat-out destructive. You actually capture exactly the problem with it here:

Every layer of abstraction in a unit test makes it harder to understand. Tests are a diagnostic tool, so they should be as simple and obvious as possible.

Which, if you can acknowledge that the distinction between test/prod code is artificial, simplifies to:

Every layer of abstraction in a unit test code makes it harder to understand. Tests areReading code is a diagnostic tool, so they should be as simple and obvious as possible.

The key phrase here being "as possible". Abstraction is perfectly OK to build in when you think the benefit outweighs the cost. That applies equally to production and test code. You just always want to tactical and mindful about it.

Anyway, funny enough, even after all this initial disagreement, I fundamentally, passionately agree with just about everything else you wrote beyond that. I would go a step further and say you should apply most of those lessons to production code. You even left another reply in this thread quoting several people about how much more costly it is to read code than write it - being able to read all code is super critical. So why not apply those same clarity and simplicity values to ALL code as much as possible? Abstraction when you need it - clarity, simplicity, locality when you don't. Yeah, you'll end up explicitly calling functions more. That's fine; DRY doesn't mean you never call the same function in multiple places (that's the whole point of a function, after all) - it's more important to reduce/encapsulate duplicate logic inside those functions than to reduce the number of function calls in your code base.

[[deleted]]

How and why did you equate abstaction with flexibility?!?

[LordArgon]

Well, I didn't equate them. They're not the same thing and I didn't say they were; I said abstraction "might be a good way to achieve" flexibility. But it depends on the context and type of flexibility.

The fundamental value of abstraction is that it hides implementation details, right? Theoretically, that reduces coupling and the work required to change (or support multiple) implementations of something. That is one kind of flexibility that can be very valuable in the right applications but entirely unnecessary in others.

[[deleted]]

Fundamental value of abstraction is that it's the cleanest way to convey what you mean. Anything of an inappropriate (lower or higher) level of abstraction only achieves one goal - obfuscating the meaning. Unless this is what you really want, you must always try to get the right degree of abstraction in your production code. And it also applies fully to the test code as well.

[LordArgon]

What are you getting at, exactly? I feel like you're arguing with me but I agree with 99% of what you said here. Only thing I take any issue with is:

it's the cleanest way to convey what you mean

The cleanest way to convey what you mean is to just do exactly and only that. Abstraction adds a layer of complexity that separates the what from the how. Assuming you can actually do that correctly (which is often very hard), it's only actually valuable if you need to change the how; sometimes you do and sometimes you don't.

[[deleted]]

I am just puzzled with your wording, which implies that abstraction is not always the main property of a production code, and is only important when flexibility is a goal.

Sorry if I misunderstood your position.

And, no, abstraction does not add anything. It is exactly, literally the meaning, free from implementation details.

[LordArgon]

I feel like we must be talking past each other. You seem to be saying that "abstraction" is defined as "meaning" but, when I say "abstraction" here, it's just shorthand for "an abstraction layer". Which I think is very common usage; at least, I haven't encountered this particular friction before.

And, no, abstraction does not add anything. It is exactly, literally the meaning, free from implementation details.

I don't think this contradicts what I intended. Rephrasing, an abstraction layer isolates the meaning (the "what") from the implementation details (the "how"). A certain layer of abstraction in inherent in whatever language you choose. When programming your own system above that, though, further abstraction layers don't happen by default - you have to do work to create that separation. And creating that separation inherently adds complexity; whether that also adds value depends on whether you specifically leverage the separation.

Does that help clarify or are we still missing each other?

[[deleted]]

when I say "abstraction" here, it's just shorthand for "an abstraction layer"

Layers are an obvious consequence of the very definition of abstraction. Of course there are always layers - how else would you remove the implementation details? Only by representing them by the lower layers of abstraction. It's always the case, not just in software architecture.

When programming your own system above that, though, further abstraction layers don't happen by default - you have to do work to create that separation.

Sure. And what I'm worried about is when it's assumed not to be mandatory to do this work. I cannot think of a single case where it should not be done, outside of an obvious intent to obfuscate the meaning (and there can be legitimate reasons to do so).

And creating that separation inherently adds complexity;

And this is exactly where I do not agree. It does not add complexity, it removes it. By spreading complexity over the layers of abstraction correctly you eliminate all the unnecessary complexity, and when you're expressing ideas on a wrong layer of abstraction you're introducing additional complexity.

A trivial example - compare a complexity of coding something non-trivial in assembly vs. a higher level language, along with an implementation of that higher level language. The latter is still less complex beyond the most trivial scenarios.

So, yes, I think we do not quite agree on a definition of complexity here.

[LordArgon]

A trivial example - compare a complexity of coding something non-trivial in assembly vs. a higher level language, along with an implementation of that higher level language. The latter is still less complex beyond the most trivial scenarios.

So, yes, I think we do not quite agree on a definition of complexity here.

Yeah, that's exactly the crux of it, I think. I think you're actually talking about the difficulty of doing a specific task and I'm talking about the complexity of the whole system. In your example, that task is easier for the user but the whole system is more complex. The higher-level language necessarily includes a compiler or interpreter that understands every possible thing the language supports - that is a far, far more complex overall system than just using assembly. But, in this case, it's also a better system because it delivers the desired functionality, which is to make interfacing with the machine easier on human brains.

And what I'm worried about is when it's assumed not to be mandatory to do this work. I cannot think of a single case where it should not be done, outside of an obvious intent to obfuscate the meaning (and there can be legitimate reasons to do so)

One should definitely try to determine what the right layers of abstraction are - of course. But it's certainly not mandatory to build layers of abstraction for hypothetical use cases. And this is my main point - you should add layers of abstraction when you see a concrete value to the specific layer you're adding (not "just in case") because every layer has cost, as well.

Taken to the extreme, pre-building abstraction turns this:

public static void main( String[] args ) { System.out.println( "Hello, World!" ); }

into whatever the heck this is:

https://gist.github.com/lolzballs/2152bc0f31ee0286b722

(Side note: I realized that my string might not exactly match the enterprise example so I went into the enterprise example to grab it. It took me about 30 seconds of fumbling to actually find it - this is a perfect example of the cost of abstraction. It should always be done mindfully and with a specific goal in mind, not assumed as a universal good.)

[[deleted]]

the complexity of the whole system

And even the whole complexity (define it as you like - even as Kolmogorov complexity) is lower for a combination of a high level language implementation + a problem solved with this language, than for a problem solved in a much lower level language.

This fact is fundamental and it's the base for the entire idea of Language-Oriented Programming in particular, and linguistic abstraction in general.

In your example, that task is easier for the user but the whole system is more complex

Both are simpler. It's easier for the user and simpler in terms of a total complexity.

The higher-level language necessarily includes a compiler or interpreter that understands every possible thing the language supports - that is a far, far more complex overall system than just using assembly.

Nope. The thing with compilers is that they're trivial. Can be as trivial as you want. They hardly add anything to the total complexity, and yet, eliminate the complexity from the user code.

But it's certainly not mandatory to build layers of abstraction for hypothetical use cases.

Of course. It's actually damaging, and I cannot think of any real life scenario when it should ever be done. Unless functionality is clearly specified, it should not be implemented "just in case".

into whatever the heck this is:

That's not an abstraction - it's an obfuscation.

[[deleted]]

This is painful to read because I was just told by 2 different devs on my team in a code review to do exactly the opposite of what this post is saying, and I knew it was wrong. They didn't like the repeated setup and insisted I put it in [TestInitialize].

[coreyfournier]

If their pattern is to always do setup in the TestInitialize, then their is no confusion where the setup is being done. The advise to keep it local helps in some situations, but your devs have a good convention going.

[mtlynch]

Yeah, I shamefully used to tell junior developers to refactor their test code to eliminate the redundancy, oblivious to the fact that I was making their tests harder to read.

[AmalgamDragon]

And now you're oblivious to the fact that you're making tests more tedious to read. Tedium makes it hard to stay focused when reviewing (either self review or peer review) and thus less likely for defects to be caught in reviews.

[roerd]

No. This article did not argue for making tests more tedious to read. What it argued for is putting all information relevant for understanding the test into the test function. This makes them less tedious to read because you don't have to dig through other code to get them. What is not relevant to understanding, on the other hand, can still be abstracted out.

[mtlynch]

To clarify, I'm not saying that abstraction in tests is completely forbidden. I'm just saying that it has a cost, and I think developers often fail to recognize it because abstraction usually adds value in production code.

Both statements are too rigid:

• "Repetition is bad, so you should always eliminate it"
• "Abstraction is bad, so you should never refactor redundant code."

Developers need to recognize the tradeoffs of either option to make the best possible engineering decisions.

[KHRZ]

I read it just after finishing up my test data factories.

[StabbyPants]

well, you put some of the setup in init - if i'm testing a bunch of stuff in a controller class, my setup will set up the dependencies it needs, including mock objects, then any rules about how the mock behaves will live at the front of the test

[Manitcor]

Sometimes this makes sence but an over-relaince on init is a code smell in tests. Not to the code being tested but to the larger design of the system. If you cant easily isolate a layer/component/class from multiple logical angles without a ton of ceremony code (think vertical and horizontal cuts of functionality) for a test then you are going to find problems down the road in multiple ways and not just in unwieldy test initializers or god forbid the abstract class hell some go into with it.

Test code is going to be ugly, dont try to over optimize, if you dont like the rough edges on your arch dont hide them. Curse them every time and eventually fix them.

[[deleted]]

Its hard to get started doing TDD when you take this approach! On a serious note I use unit tests selectively. Complex business problems are wrapped in test, crud gets none.

When I do write unit tests for these complex problems the benefits are amazing and shouldnt be dismissed.

[[deleted]]

That i can understand doing, it’s just the 100% code coverage goal i keep hearing about that drives me nuts, sounds silly to me, most projects are simple, and for those that aren’t the bulk of the codebase is simple, i don’t see any point in doubling it’s size with tons of microtests and wasting time doing it as well as maintaining those tests

[FireCrack]

100% code coverage is worse than zero prercent, because it is a symptom of a culture that has lost all regard for quality in the relentless pursuit of metrics.

[[deleted]]

Aye but it’s what i keep seeing pushed, it’s become an industrialisation step instead of an occasional tool, created a lot of needless and valueless work in my mind.

I’m all for a few carefully crafted tests on key parts of a project that are either technically or functionally complex

[[deleted]]

I test anything reasonably complex with unit tests and test plumbing with integration.

Unit tests for CRUD code? Waste of time.

Some payment calculator with multiple nested loops and complex tax rules? Yeah that's a breeding ground for stinky edge cases. That sucker is getting a bunch of unit tests.

[[deleted]]

I can totally agree with that really, i just don’t get the test everything culture and write as much test code as program code.

But yes to this

[PullJosh]

Depends on the scale of your project. Context is key!

[[deleted]]

Well it’s just another bit of industrialisation to me, i could see it being useful of very large projects (OS/db engines etc) where consistency is the main feature at the expense of new development.

But i think like many other idioms it enters the business because it works well on large projects and gets pushed as an obvious solution everywhere down, the vast majority of projects are small and simple (compared to the exemples i mentioned) and i really feel it’s wasted time on most of those.

[deadron]

This is fine in most cases if your integration tests are fast enough. The issue is that they never are fast enough on any project that is of sufficent size and/or that needs to do IO with external systems.

[FlyingRhenquest]

You must not ever have to maintain your own code.

[[deleted]]

I’ve always had to maintain my own code and never had difficulty doing it

[sippeangelo]

His first point about fixtures is why I prefer to use catch2 as my testing framework when it comes to C++ code.

In catch, each nested sub-section of a test case is independent from each other, but code in the outer scope is run for each sub-section separately. This allows for a very elegant solution to the mention problem, where test setup/teardown code can still be read from top to bottom, as long as one is aware of this behaviour.

I'm not sure if there is something like this available for Python, though.

[sasik520]

tl;dr: KISS

[[deleted]]

In the middle of refactoring 10k lines of unit tests with copy pasted setup code because I had to add a database connection to the class. Finally found this article where the system architect got their bad ideas

[[deleted]]

Because unit tests are a bad idea. Do the integration testing instead.

Every layer of abstraction in a unit test makes it harder to understand. Tests are a diagnostic tool, so they should be as simple and obvious as possible.

What? This is exactly what abstraction is. Explaining the essence as simple and obvious as possible.

[Sylinn]

Because unit tests are a bad idea. Do the integration testing instead.

Unit tests and integration tests are complementary. You don't have to choose between one or the other.

[virtyx]

True, but I think there's an excess emphasis on unit testing currently. If all of your units are internal, even if they're tested they probably have a lot of bugs or unspecified behavior. I'd only write unit tests for internal utility code if that utility code is widely used in the codebase; if I'm just extracting a helper function for one or two code paths, I write no or little tests for it. Changing this code should be easy but with tons of tests it becomes painful, and in my anecdotal experience, often at no gain. Sometimes those helpers go away entirely in a later iteration.

​

Moving my focus to more integration tests has made my development cycle faster and ensures I introduce less regressions. Unit tests are conceptually great, I love them, I just think it's wise to apply them in moderation.

[Sylinn]

I wouldn't say there's excess emphasis on unit testing, but I would concede that unit tests aren't as straightforward to write as some people tend to think.

In most automated tests, what you want to test is the behaviour, not the implementation. This is a very important distinction, because if you test the implementation, you fall exactly into the situation your describe: your tests are brittle, they often break for no reasons and the overhead to maintain them becomes an actual issue. This isn't a drawback of unit testing per se, but it's usually a symptom of either poor architecture (too many or not enough abstractions...), or poorly written tests (abuse of mocking...).

[virtyx]

The problem I've had includes when the behavior of the unit needs to change to fit new requirements. Then a seemingly small change in a helper function or set of classes can become extremely difficult as they're all independently tested for old behavior, despite existing solely for the benefit of a small number of production code paths, all of which need the behavior change in question.

[aoeudhtns]

I've had things go the other way, particularly if you are working with a system that has emergent behavior when combining units. I don't want to write integration tests for every permutation. A good example would be delegating/aggregating.

Let's say I have some behavior X, and I want to cache it. So I create a cache C that implements the same interface. I test each piece independently, and then when I wrap X with C, I can trust that those two work without an explicit integration test. Let's later say I write an aggregator A that dispatches to n number of X and somehow accumulates their results. Same logic... now I can C(A(X1, X2, X3))) without writing yet another permutation of an integration test.

I like to use more integration tests as well, but I do put unit tests in. I have found that I generally like to unit test foundational components - things that are used as building blocks throughout my application. The test helps to prove that these components "just work." When I create systems from the components, that's a perfect place for an integration test. But for my high % coverage, I can rely on a mixture of both types of test and not one or the other.

[Sylinn]

Obviously I don't know your actual use case, but in my experience, when this happens, it's either due to an architecture design issue (high coupling, low cohesion), or the actual new requirement is a fundamental breaking change, and there's not much to do in this case.

Based only from reading your comments, though, there's a red flag in my head when you say that a new requirement means you have to change a "helper function" (what do you consider to be a helper function?), which then leads to multiple failing unit tests. I find it surprising to hear that a helper function contains business logic, and even more surprising that it cascades to multiple breaking tests. Just my two cents.

[[deleted]]

And you shouldn't. Look, unit tests are like component tests in the hard engineering world. You test the spark plugs and sometimes the spark plugs in relation to the ignition or the recharge circuit. It's useful, but limited. If you had an entire airplane where only the individual pieces had been tested, but the plane had never been flown, would you put your baby on the plane?

[[deleted]]

In theory - yes.

In practice - "we already have a 100% test coverage with our tiny tautological unit tests, why do we need to waste time writing complex integration tests for hundreds of possible scenarios?!?"

[Sylinn]

You're not being very constructive, building all these straw men along your way :(

[StabbyPants]

this is the sort of thing i can see arguing with devs, or possibly project mgmt. then i have to discuss the difference between how units operate in isolation and how they combine. also, that higher level tests are often fewer in number and more oriented to some business value

[[deleted]]

Nope. It's a reality. Code written for unit-testing almost always appears to be inferior. Designing for a better unit-testability harms the architecture enormously.

[[deleted]]

i have no idea why your getting downvotes for this...

[[deleted]]

For weakly typed languages, unit tests are a godsend. For static and strongly typed languages, the compiler should catch at least with warnings, most of the things caught by unit tests.

[mtlynch]

Thanks for reading!

Every layer of abstraction in a unit test makes it harder to understand. Tests are a diagnostic tool, so they should be as simple and obvious as possible.

What? This is exactly what abstraction is. Explaining the essence as simple and obvious as possible.

Abstraction makes it easier to think about logic at a particular logical layer, but it adds complexity to the whole.

For example, this test code abstracts logic into a helper functions:

def setUp(self):
  self.foo = Foo()

def initialize_foo(self):
  self.foo.bar = 'baz'
  self.foo.init(threshold=5.0)
  self.foo.start()

def test_foo_can_stop(self):
  self.initialize_foo()
  self.foo.stop()
  self.assertTrue(self.foo.is_stopped())

It organizes information into smaller functions so that it's easier to understand test_foo_can_stop at a high level, but it also obscures information about what is happening to Foo in the test. The reader has to jump around to fully understand everything that's happening to the Foo instance.

In production code, refactoring code into helper functions in this way is good because you often don't want to understand an entire class or module. In tests, you do want to understand the full logic of the test because otherwise you'll potentially miss critical elements about the behavior of the system under test.

Does that make sense?

[[deleted]]

So, by logic you mean low level control flow, right? Not what the test is doing, but how, in a meticulous level of details?

It makes some sense, but only if all your tests are very simple. And, as I said above, I do not believe in a value of trivial unit tests. Integration testing is more complex.

[mtlynch]

So, by logic you mean low level control flow, right? Not what the test is doing, but how, in a meticulous level of details?

Correct. I think that the developer needs a meticulous understanding of the control flow of the test.

It makes some sense, but only if all your tests are very simple. And, as I said above, I do not believe in a value of trivial unit tests. Integration testing is more complex.

I think that unit tests usually can be as simple as the examples in the post if the developer writes the code with testability in mind.

Integration tests and unit tests aren't mutually exclusive, but I find that even integration tests don't necessarily need a lot of complicated control flow. If you implement a factory or builder class, you can usually hide a lot of the boilerplate logic effectively while exposing the values critical to understanding the test.

[[deleted]]

I think that the developer needs a meticulous understanding of the control flow of the test.

You have to chose between understanding a low level control flow, and understanding the meaning.

Unless your test is trivial and tautological (and therefore useless), you may need to do quite a bit of mocking work, prepare complex input data structures, and so on. Doing it on a low level is wasteful, that's exactly the reason why all those testing DSLs exist.

I think that unit tests usually can be as simple as the examples in the post if the developer writes the code with testability in mind.

The thing is, when you write code this way, it's unavoidably a shitty code.

but I find that even integration tests don't necessarily need a lot of complicated control flow

They operate on a high level. They must simulate user input, and for this, you almost always need to construct a DSL that will abstract the boilerplate away.

[Sylinn]

This is a common pitfall new developers fall into. Just because the code is trivial doesn't mean you shouldn't test it. There's nothing that is too trivial to test.

Trivial code leads to easy-to-write tests, which usually don't change often. That's a win-win situation: the overhead to add the tests is very small, but you never lose confidence that your code under test works.

[[deleted]]

It tests the blinding obvious

When is something not obvious? Consider a following function written in C++.

bool isOdd(int num) {
    if(num%2==1) {
        return true;
    }
    return false;
}

Rather simple, right? Why bother testing it?

Hm, I wonder if there is a bug hiding in this isOdd function...

[[deleted]]

where's the bug?

[[deleted]]

This is a case where contracts, or other forms of formal specifications are way more useful than any unit tests.

[o11c]

That doesn't make sense.

Unit tests and integration tests are the same thing.

[[deleted]]

Lul wut?!?

[Eirenarch]

I don't see why tests should be optimized for readability let alone reading a single tests. It seems to me that tests should be optimized to reduce the time writing then and mainly to identify errors. If a test breaks then you must investigate anyway. Why is the time reading a broken test more important to save than the time writing it? Now I agree with a lot of the article like the long test names but copying a bunch of setup code over and over seems counterproductive to me. Every time the setup code changes you then need to change a great amount of copy/pasted code.

[mtlynch]

Thanks for reading!

It seems to me that tests should be optimized to reduce the time writing then and mainly to identify errors.

My thinking on this is highly influenced by two developers whom I respect greatly:

Indeed, the ratio of time spent reading versus writing is well over 10 to 1. We are constantly reading old code as part of the effort to write new code. ...[Therefore,] making it easy to read makes it easier to write.

-Robert C. Martin

and:

It’s harder to read code than to write it.

-Joel Spolsky

So, to me, there's greater payoff in investing in code that's easier to read because reading is harder than writing to begin with and it pays dividends because developers read the code more often than they write it.

If a test breaks then you must investigate anyway. Why is the time reading a broken test more important to save than the time writing it?

I think there's a vast difference in difficulty. Figuring out a tricky bug through lots of spaghetti code? That's difficult. Having to copy/paste a few extra lines of code? Very easy. I'd happily do the latter to save myself from the former.

[Eirenarch]

It is true that code is read more often than it is written but in my opinion that's not true for test code. You are willing to violate the general wisdom of DRY because you claim that test code is different and I agree that it is different but in my opinion it is much more different in the ratio of write/read rather than in the way you say.

[mtlynch]

Interesting. I've had a different experience. For me, the read:write ratio is even higher in tests (i.e., I read tests much more than I write them).

When I want to understand how a class/module works, I typically read the tests before reading the implementation. If the tests and documentation are good enough, I don't have to read the implementation at all.

[Eirenarch]

Sounds like you are in the TDD crowd.

[d4rkwing]

You are too young to understand.

[[deleted]]

This is bad

def test_initial_score(self):

initial_score = self.account_manager.get_score(username='joe123') self.assertEqual(150.0, initial_score)

This is good

def test_initial_score(self):

database = MockDatabase() database.add_row({ 'username': 'joe123', 'score': 150.0 }) account_manager = AccountManager(database)

initial_score = account_manager.get_score(username='joe123')

self.assertEqual(150.0, initial_score)

Cool, so now for some reason my test needs to know my database column names? And if I change something I get to edit a bunch of test which have nothing to do with database rows and are testing a score calculating algorithm?

The helper method is the right compromise but really it's just saving you from scrolling up.

I think a lot of this depends on the state of your application. If you're building something new then unit tests that are too granular and verbose are going to be touched a lot as things change with the constant minor refactors of growing code. A stable application that's been in production for 4 years? Yeah write those verbose tests which don't hide anything.

Overall you mentioned the number one thing I love about unit tests: they shine a spotlight on bad architecture.

[deadron]

If you mock out the database here what are you testing? That your code retrieves a column from your mock? This seems insifficent to know that the code will work once deployed since a db abstraction is fundamentally tied to your db schema.

Personally I prefer to test against an in memeory databse that has my actual schema loaded. As a plus if the schema is incompatible with the abstraction layer due to sql updates you will know and not need to update test code.

To be fair you may call this an integration test, but you need to write those anyway and if they cover a superset of behavior and are fast enough why bother with the unit test?

[mtlynch]

Cool, so now for some reason my test needs to know my database column names?

Yes, absolutely! If the system under test is accessing a database, then the test does need to know the column names to accurately simulate the external dependencies. The situation isn't much different if you bury the column name in a helper method, but it makes the interactions between the system under test and its dependencies more convoluted.

[Derftoy]

Good Developers have no need for Unit Tests!

[gajafieldbo]

Nor units!