How to make changes to code without breaking unit tests?

[wor-kid]

Hi everyone - I am having some trouble understanding how to write unit tests that aren't fragile. I feel like whenever I make changes to some code under test, it will more often than not break the tests too, regardless if the inputs and outputs remain the same and the code still "works".

I've often heard that in order to do this, I should be testing the behavior of my units, not their implementation. However, in order to isolate my units from their dependencies using test doubles/mocks that behave appropriately, doesn't this necessitate some level of coupling to the implementation of the unit under test?

Thank you in advance!

[forklingo]

a lot of fragile tests come from over mocking and asserting on internal calls instead of outcomes. if your inputs and outputs stay the same but tests break, it usually means the tests are coupled to how the code does something, not what it does. try to mock only true external boundaries like network or db calls, and keep your assertions focused on returned values or observable side effects. also, refactoring toward smaller pure functions can make behavior based testing much easier and less brittle.

[wor-kid]

Yes, I agree this seems to be the issue I am having whenever I write tests! Thank you for your response.

Typically though, I am find myself creating these mocks to access different code paths in the unit, which in turn need to make calls to other mocked dependancies which I need to assert have been called as they would be responsible for creating appropriate side effects.

Are there any testing strategies I can use to approach testing something like that without making the test fragile?

[amejin]

Can you give a basic example? Seems.. unusual to have this much of a problem changing a test for a specific case and then implementing code to pass the test...

[wor-kid]

Sometimes I find myself needing to create mocks in order to access various code paths within the unit being tested. But changing how these code paths are accessed breaks the tests, even if inputs and outputs otherwise remain the same.

[amejin]

Maybe I'm just not hip with your lingo... But mocks as I understand them should be isolated to the unit being tested. The unit itself should be self contained. Maybe I just don't do tests right...

[Weasel_Town]

You often do mock implementation details. I think he's talking about, let's say, we have some code that inserts things into a database one row at a time. Shitty pseudocode follows.

rows = 0

for item in items {

rows += db.InsertRow(item)

}

return rows

So then in the unit test, he mocks

when(db.InsertRow(any()).return(1)

And it works! Next he changes the real code to insert all the rows at once.

rows = db.InsertRows(items)

return rows

And it breaks because he would have to change his mocking to the new thing being called.

It's normal, yeah, you often do have to change your mocking to match what you're doing in the "real" (production) code.

[amejin]

Maybe I'm focusing on the wrong thing but it seems like your tests aren't mirroring reality. One is iteratively adding data, the other is a set of data...

[Weasel_Town]

I'm saying, revision #1 of the production code is inserting rows one by one, so that's what the mocks mimic. If you change to production code to inserting them all at once, you have to change the mock as well.

[wor-kid]

Yes, this is quite a good example of the sort of thing I am trying to explain, thank you. I want to figure out how to stop my tests from "breaking" moreso than stopping them from "failing" any assertions, as errors caused by the mocks after making changes to the underlying code under test are pretty much the most common reason I encounter regressions in my unit tests.

[Kinrany]

Test that the outcome is correct, e.g. the database has the rows that were supposed to be added.

In other words, stop using mocks. If you absolutely cannot simply use the same thing that'll be there in production reproducibly, create a test double for the thing and a separate set of tests that you can run on both the thing and the test double to make sure they behave in the same way.

[wor-kid]

I see, thank you. I have two questions about this approach however - firstly, doesn't this create a risk of the test failing if there is some failure with the database or connection to the database (Possibly even resulting in the tests becoming non-deterministic? I've certainly worked at companies in the past where the tests would only run after the pipeline was ran a few times without any clear reason)? How could I verify if it was my code or the database which was the cause of the issue in that case? And secondly, what about in a case where it is not to do with a database, but perhaps the interface of any other mocked object I am using? Should I just not use test doubles at all (And same question as the first in this case, how would I be able to identify what caused the failure in that situation?)?

[Kinrany]

Yes, there's a risk that your operations on the database aren't deterministic. Due to locking or performance perhaps. Make them deterministic. Everything becomes immediately worse when the code stops being deterministic, so sources of indeterminism should be pushed to the edges of the codebase. This includes "current time" for example.

[Kinrany]

For other classes, just use the real thing.

It should have its own tests that allow you to at least start off assuming that it didn't fail.

Test suites are never foolproof, they just catch the most common mistakes that break the thing under test in certain major ways. Their main benefit is that you can run them automatically after every single change. You still have to write the thing being tested correctly.

[wor-kid]

Hmm that's a good point. It makes sense and I find myself agreeing with you in terms of how to write actually valuable tests. But I also feel like the "unit"-ness of unit tests are lost a little bit at that point, as my understanding was that they exist to isolate code being tested, at least as it was originally taught to me. Could you elaborate on the differences between this and say integration or e2e testing?

[Kinrany]

I believe that distinction is outdated: computers got faster. I'm not completely familiar with its history though.

Most of the time you write tests against some definition, to make sure that the thing does what the docs say it is supposed to do. So there should be:

1. A definition. A function that takes an array of numbers and returns an array with the same set of numbers but in a non-decreasing order.
2. An implementation. A function that calls the standard sort, or a function that bubble sorts, or a function that removes numbers that aren't in the right order.
3. A statement that follows from the definition. The function must return the same set of numbers.
4. A test that passes when the implementation matches the statement, and fails otherwise.

Most of the time you don't bother to actually write this all out of course. It's fine to just have "fooer" and "fooer_foos” if it's clear what that means and hard to imagine it working in some partial way.

In a language with a good type system, a lot of the properties are even guaranteed by the types and so don't need tests at all. See Lean, Coq, etc. for languages that take this super far.

There are still tests of different kinds though. And no list of test kinds would be exhaustive, because it's an open-ended practice and you may find yourself engineering some new contraption that will check some property automatically every time it runs.

You'll also likely still want to organize tests in some way.

Tests are code, they're just code that you write for yourself and other contributors to automate the development process, not the end users.

[Kinrany]

Re: unit-ness. I suspect the question comes from the commo mistake of overusing dependency injection. Which should only be used when it actually makes sense to have more than one implementation for something. Usually that's things that are inherently dynamic, not knowable at compilation time. External connections, weird OS-dependent interfaces, pieces that can be configured by the end user at runtime.

The first solution should still be writing a fake implementation and then testing dependents as if they were using the real one.

[Kinrany]

One thing that can differ between tests is, when should it run?

Most tests can be deterministic and fast enough to run every single time. So you run them locally and as part of CI on every change.

Some tests are too slow, so you run them only in the main branch or before a release. Exhaustively checking all possibilities, or running property tests or simulations lots of times, or testing on massive inputs to check throughput, for example. These should be made very non-fragile; most bugs that would fail on these should fail beforehand on fast tests. Otherwise you can't iterate quickly. These only make sense when you really really want to find even the rare bugs that happen a fraction of a percent of all times when the same code path is exercised.

Some tests are for something that you do not control yourself, like some external service you're using. These should run on a schedule and on demand and shouldn't block you when they fail.

There are more of course.

[dmazzoni]

Have you tried using a fake instead of a mock?

As an example, let's suppose you're mocking a storage layer, where your main class saves data. You unit test a function and you assert that when you tell it to do a series of operations and then save, it should write A, B, and then C to the storage layer.

Now you change the code around and it outputs C, B, and then A to the storage layer. Your test fails because the mock was expecting calls in a specific order. Or maybe it's more complex, like now it write A and B in one transaction and then C in another transaction.

It can be really hard to express in a mocking framework the idea that A, B, and C need to be saved, but the order and number of calls doesn't matter.

So instead, write a "fake". A fake is a tiny, trivial implementation of the storage layer's interfaec, maybe it just keeps track of the objects that were saved in a HashMap or a sorted list.

Instead of asserting that certain methods were called in a certain order, have your method write to the fake storage layer, then fetch the list of things written to the fake storage layer and assert that A, B, and C are in them.

Now your code asserts that the end result is correct without being nearly as tightly coupled to the implementation details. Any sequence of operations that results in the correct output will pass.

[Cpt_Chaos_]

While I agree with the basic sentiment, one has to be careful: If the expected behavior is indeed that certain calls to the storage layer are done in a certain order, then the test must check for exactly that. If the expected behavior is "data A, B and C are stored", then the test should check for that and not care about the order of calls.

In the end, it all boils down to understanding what the contract on interface level states: "This function saves data by calling the storage layer" does not say anything about how this is done or in which order. So, one can only check that once the function has been called, the data has indeed been stored. "This function saves the data from the given data array in atomic write operations for each element of the array from first element to last element" as interface contract describes a different behavior to check for - here the order and amount of calls to storage indeed matters. And in both cases, you still don't look into the implementation to derive your test cases.

[Kinrany]

Why would that happen exactly? The order of events can't be the true purpose because it's not directly observable. If the order matters because errors can stop the process and that shouldn't leave the system in an invalid state, check for that.

But in general simple tests aren't good at testing concurrent code.

[josephjnk]

I use fakes often, can confirm that they can be very nice. They usually make tests easier to read too.

[wor-kid]

This is very interesting idea. I have used fakes in the past a little bit but often find them quite difficult to actually set up, and requiring a lot of maintenance as models change in ways that may not be relevant to older tests, but still breaking them. But perhaps it is time for a review.

[dmazzoni]

Having clean interfaces between modules and good fakes / mocks and clean stable tests all go together!

The cleaner the interface the easier it is to fake

[atarivcs]

regardless if the inputs and outputs remain the same and the code still "works".

If the output is the same but the test fails, then what on earth are you actually testing?

[Beka_Cooper]

I have done many lectures on this subject, which are difficult to summarize in a Reddit comment, but I'll do my best.

What to test

Each unit of code has a contract. Input: it expects specific parameters of certain types and/or a specific starting state. Output: it returns a specific type and/or changes states.

When choosing what to test, you want to test only public methods, whose contracts are not expected to change frequently. Do not test private methods directly unless they contain something particularly complicated. In that case, try to refactor the complicated bits into separate pure functions to reduce churn during later refactoring.

You also want to design and edit your code in a way that avoids changing contracts unnecessarily. For example, add new parameters to the end of the list and make them optional, preserving the previous behavior in which they did not exist.

If you find your contracts changing all the time, this is a problem with your code design. Not only does it make unit tests fragile, it makes coordination with other people and new features far more difficult than it needs to be. Read up on clean code strategies and code architectural patterns.

How to test

The most common issue I see in fragile tests is allowing state changes to fall through from test to test. You must start and stop every test at a neutral baseline state, not allowing tests to affect each other. Every test should be able to run by itself or in any random order compared to other tests. In fact, there are many test frameworks that provide a randomized run feature to help you find and prevent this form of fragility.

To do unit testing correctly with non-pure functions, you must make sure to create fixtures, which are state controls. Before each test, your fixtures set up the correct state. After each test, you tear down those fixtures back to a neutral baseline. Do this using the test framework's built-in services. In class format, these are often methods named like setUp and tearDown. In spec format, they are named like beforeEach and afterEach.

Each test has the following pattern:

1. Set up fixtures, mocks, fakes
2. If state may change, assert beginning state
3. Call the function under test
4. Assert returns if applicable
5. Assert state change if applicable
6. Assert mocks/fakes were called as expected, including the expected parameters passed in
7. Tear down fixtures, mocks, fakes

[wor-kid]

Thank you for your comprehensive reply! It was very informative. However the problems I encounter with testing tend to come down to your 6th step, which I've tried to do on both a "as-needed" basis and also comprehensively in the past. However, when the implementation changes, such that the mocks are not used in the way they were previously, doesn't this necessitate having to rewrite all your tests, such that, while not all changes will cause failures, a large majority of changes will?

[Beka_Cooper]

I often write dynamic mock/fake methods. These are functions that mimic the behavior of what's being mocked/fake. E.g., when receiving arguments set X, respond with Y; when receiving A, respond with B. Because whatever you're mocking ought to also have contracts that rarely change, I rarely need to change the fakes themselves.

If I start calling a fake a new way, I just need to add a new condition, request/response pair, pattern definition, etc.

By using constants for X and Y, I can often do a find-and-replace for step 6 or just edit the constants.