Unit tests for methods with trivial logic?

Benjamin Hall

So since I can't do real work against tickets (for various bottleneck-related reasons) right now, I'm looking at the unit tests for the product I'm responsible for and trying to decide what we need tests for and what tests should change/go away. The idea being that as I touch other things going forward I can add a few unit tests along the way.

As I understand it, the point of unit tests (when not doing TDD, which we're not) is to provide regression-detection capabilities. So that during the dev process you can know if you broke something existing. At which point you either have to update the test (because a requirement changed and now the test is outdated) or unbreak the thing (if the change was unintended). The goal being reducing the number and scope for defects before it hits QA.

Context: This is a node.js/typescript backend API server. I'm building tests for the controller layer--the API unwraps the request and handles authentication, then forwards to the appropriate controller layer which should have the business logic. The controller uses an access layer to make the HTTP/database calls it needs to do its work. That layer is relatively thin. In theory. So in principle, all the business requirements and business logic (invariants and suchlike) are in the controller layer.

Which brings me to my question: what about methods that are just pretty simple? Where they do a couple of tests for malformed data (ie "hey, you can't create this thing without a name" or "hey, you can't have more than one of these with the same name") and that's about it? Do those need unit tests? What do people think?

ObjectMike

@Benjamin-Hall said in Unit tests for methods with trivial logic?:

So since I can't do real work against tickets (for various bottleneck-related reasons) right now, I'm looking at the unit tests for the product I'm responsible for and trying to decide what we need tests for and what tests should change/go away. The idea being that as I touch other things going forward I can add a few unit tests along the way.

As I understand it, the point of unit tests (when not doing TDD, which we're not) is to provide regression-detection capabilities. So that during the dev process you can know if you broke something existing. At which point you either have to update the test (because a requirement changed and now the test is outdated) or unbreak the thing (if the change was unintended). The goal being reducing the number and scope for defects before it hits QA.

Context: This is a node.js/typescript backend API server. I'm building tests for the controller layer--the API unwraps the request and handles authentication, then forwards to the appropriate controller layer which should have the business logic. The controller uses an access layer to make the HTTP/database calls it needs to do its work. That layer is relatively thin. In theory. So in principle, all the business requirements and business logic (invariants and suchlike) are in the controller layer.

Which brings me to my question: what about methods that are just pretty simple? Where they do a couple of tests for malformed data (ie "hey, you can't create this thing without a name" or "hey, you can't have more than one of these with the same name") and that's about it? Do those need unit tests? What do people think?

I wouldn't consider input validation trivial. When I think of a trivial piece of code I think something like "generates a name by appending x to y". Or trivial getters/setters against a private field in languages where you need to write that yourself. I would think there is a high likelihood that input validation will evolve or have bugs in it so tests would be good around that.

Another decent high-level reason given for unit tests is it acts as compiler-and-assert enforced "documentation" of what the code does. Written documentation is a lot more likely to rust in place compared to test code.

izzion

To paraphrase my company’s Slack recently: “[One] goal of unit tests is to provide a deadman switch to help the next guy know he dun broked sumtin”

The most useful unit tests I’ve worked with are ones that test important business logic within the code and that are named & organized in a way that clearly indicates what the business logic is. So if the frobnicator module should take 2 foo and output 4 bar, a test should capture that and indicate as much of the “why” of the logic as can be reasonably displayed in the naming / organization. But if the business doesn’t care what color the bar are, testing the method that sets a default color is less useful — though if the program dies a fiery death if there is NO color set, then your test should at least verify a default is set, without necessarily enforcing a specific default (maybe via asserting the color matches a constant static value defined in the code under test)

I certainly would never test basic language features (e.g. the getter/setter on a .NET POCO auto property).

dkf

@Benjamin-Hall said in Unit tests for methods with trivial logic?:

Which brings me to my question: what about methods that are just pretty simple?

Some simple methods absolutely should be tested. For example, if a class is meant to act as a key in a map, you absolutely should test that that works. Why? To stop some idiot in the future from fucking that up without at least getting stuff flagged up that they're breaking something subtle like equality. (I heard tell recently of a bug in the Rust compiler that was caused by this category of fuck up; the hash keys they were using were mutable, and “hilarity” ensued. )

Gribnit

@Benjamin-Hall said in Unit tests for methods with trivial logic?:

So since I can't do real work against tickets (for various bottleneck-related reasons) right now, I'm looking at the unit tests for the product I'm responsible for and trying to decide what we need tests for and what tests should change/go away. The idea being that as I touch other things going forward I can add a few unit tests along the way.

As I understand it, the point of unit tests (when not doing TDD, which we're not) is to provide regression-detection capabilities. So that during the dev process you can know if you broke something existing. At which point you either have to update the test (because a requirement changed and now the test is outdated) or unbreak the thing (if the change was unintended). The goal being reducing the number and scope for defects before it hits QA.

Context: This is a node.js/typescript backend API server. I'm building tests for the controller layer--the API unwraps the request and handles authentication, then forwards to the appropriate controller layer which should have the business logic. The controller uses an access layer to make the HTTP/database calls it needs to do its work. That layer is relatively thin. In theory. So in principle, all the business requirements and business logic (invariants and suchlike) are in the controller layer.

Which brings me to my question: what about methods that are just pretty simple? Where they do a couple of tests for malformed data (ie "hey, you can't create this thing without a name" or "hey, you can't have more than one of these with the same name") and that's about it? Do those need unit tests? What do people think?

Yes. Those need them. Those are the negative constraints. Having those solid is important. It's a form of proof.

BernieTheBernie

@Benjamin-Hall said in Unit tests for methods with trivial logic?:

Do those need unit tests? What do people think?

Just remember that you can f*ck up "simple" boolean logic quite easily:

@BernieTheBernie said in CodeSOD collection:

if (!rawImage.IsIncomplete || rawImage.ImageStatus != ImageStatus.IMAGE_NO_ERROR)

I do not know the quality of your cow-orkers, but be prepared that you might find such gems while writing the tests.

Benjamin Hall

@BernieTheBernie Not just cow-orkers.

I was writing a test earlier. It was failing--the wrong exception was being thrown. Why? Because I'd set up the mock to return the appropriate piece of data if passed id 0. But then passed id = 1.

BernieTheBernie

@Benjamin-Hall You see: it's trivial. Really trivial. Like everything software related.

Gribnit

@BernieTheBernie said in Unit tests for methods with trivial logic?:

@Benjamin-Hall You see: it's trivial. Really trivial. Like everything software related.

It's deeply trivial though, make no mistake.

izzion

@Benjamin-Hall said in Unit tests for methods with trivial logic?:

@BernieTheBernie Not just cow-orkers.

I was writing a test earlier. It was failing--the wrong exception was being thrown. Why? Because I'd set up the mock to return the appropriate piece of data if passed id 0. But then passed id = 1.

Some days you’re the worker, some days you’re the cow.

Benjamin Hall

@izzion said in Unit tests for methods with trivial logic?:

@Benjamin-Hall said in Unit tests for methods with trivial logic?:

@BernieTheBernie Not just cow-orkers.

I was writing a test earlier. It was failing--the wrong exception was being thrown. Why? Because I'd set up the mock to return the appropriate piece of data if passed id 0. But then passed id = 1.

Some days you’re the worker, some days you’re the cow.

Yeah, today is definitely a bovine kind of day. Especially since all of my tickets have involved dates and times. And timezones. And expected behavior when things are changed in one particular way (involving time just for extra troubles). Brain is hurting.

Oh, and the product owner and the designer (who acts as the backup when the PO's gone) are out today. So yeah. Ugh.

Gąska

All tests have positive value. Unless the test takes such a long time it's infeasible to run it on regular basis, you're always better off having one than not having one.
Trivial tests provide the least value but also take the least time to write, so why the hell not.
The difference between most code being tested and all code being tested is huge. It's the difference between being pretty sure you have no regressions and willing to bet your salary that you have no regressions. I worked in one project where we've had ~95% unit test coverage all the time (the remaining 5% were mostly false negatives because C++ dev tools suck), and it was like day and night compared to what I have to deal with now (we also have a lot of tests, but mostly high-level ones, and nobody really checks for coverage. People generally avoid refactoring anything unless they cannot frankenstein new features on top of the current architecture anymore.)
You can't get 100% coverage without doing all those trivial tests too. So fucking get them done, it'll take you like one minute each at most.

TwelveBaud

@Gąska

For bonus points, after more substantial and significant development occurs, turn the trivial tests off and you find out how much of the application is not being exercised as part of non-trivial testing. Which can drive conversations about reducing code (complexity, API surface, compile times) or possible bugs ("it's supposed to check for the thing! now that we're not artificially checking for the thing, why isn't it doing so itself?") or future enhancements.

dkf

@Gąska said in Unit tests for methods with trivial logic?:

You can't get 100% coverage without doing all those trivial tests too.

If you need to inject new mock values into closed enumerations in order to get 100% coverage, perhaps you shouldn't go after 100% coverage after all.

TwelveBaud

@dkf The enumeration may be closed in this version, but what if the software suddenly finds itself working with data from the future :spoopy_ghost:? i.e. the software is upgraded and then rolled back but not before new data with a new enum value is recorded.

Either that case is supported (potentially by throwing up a "this is not supported" message, but that's support nonetheless) and needs testing, or it's not and the untested code shouldn't be there.

Gąska

@dkf said in Unit tests for methods with trivial logic?:

@Gąska said in Unit tests for methods with trivial logic?:

You can't get 100% coverage without doing all those trivial tests too.

If you need to inject new mock values into closed enumerations in order to get 100% coverage, perhaps you shouldn't go after 100% coverage after all.

The real question is why do you have dead code in the first place.

PotatoEngineer

@Gąska said in Unit tests for methods with trivial logic?:

@dkf said in Unit tests for methods with trivial logic?:

@Gąska said in Unit tests for methods with trivial logic?:

You can't get 100% coverage without doing all those trivial tests too.

If you need to inject new mock values into closed enumerations in order to get 100% coverage, perhaps you shouldn't go after 100% coverage after all.

The real question is why do you have dead code in the first place.

It's usually a sign of unhinged paranoia. I wrote some code like that not too long ago. The function executes this or that logic based on a PM-editable JSON file. There's an array of logic-bits in the JSON file, with a priority and conditions for each, so it acts like a giant switch statement. I stuck a fallback value at priority 0 in the JSON file for that far-future case when nothing goes right.

Which means, of course, that when it fails, it's going to fail silently, but with a default value that's not too bad.

Gribnit

@PotatoEngineer said in Unit tests for methods with trivial logic?:

It's usually a sign of unhinged paranoia.

Yeah you need to unit test to oil the hinges.

Zenith

@Benjamin-Hall Simpler (as in static utility) functions/procedures are about all I'd write unit tests for. The rest is just too many moving parts or something you've outsourced to a framework anyway.

Zenith

@Benjamin-Hall Simpler (as in static utility) functions/procedures are about all that I'd write unit tests for. The rest is going to have too many moving parts or already be outsourced to a framework anyway.

Gribnit

@Zenith said in Unit tests for methods with trivial logic?:

The rest is just too many moving parts

You're writing fucked code everywhere but your utilities?

Grunnen

I'd suggest putting effort into sensible unittests rather than focusing on 100%.

In one project I have worked on, there are unittests like this:

x = convert_current_time();
sleep(1 sec);
y = convert_current_time();
assert(y - x == 1 sec);

(Failed sometimes randomly, probably when the system was under high load and the OS scheduler interrupted the unittest at the 'right' moment for a few ms.)

Or like this:

my_json = create_some_json();
my_xml = process_request(my_json);
assert(my_xml.length() == 513);

It's surely an easy way to increase the coverage a lot, but it won't fail on many kinds of regressions but it will fail on many kinds of changes that are not regressions.

Gribnit

@Grunnen said in Unit tests for methods with trivial logic?:

I'd suggest putting effort into sensible unittests rather than focusing on 100%.

In one project I have worked on, there are unittests like this:
x = convert_current_time();
sleep(1 sec);
y = convert_current_time();
assert(y - x == 1 sec);
(Failed sometimes randomly, probably when the system was under high load and the OS scheduler interrupted the unittest at the 'right' moment for a few ms.)

Or like this:
my_json = create_some_json();
my_xml = process_request(my_json);
assert(my_xml.length() == 513);
It's surely an easy way to increase the coverage a lot, but it won't fail on many kinds of regressions but it will fail on many kinds of changes that are not regressions.

yeah anything concurrency wants multithread random access testing to get some fuzz against the timing. But it seems optimism blinded me and these are just brittle and weak tests.

Benjamin Hall

@Grunnen Yeah. That's my experience--when the tests have started failing, it's 99.99% of the time due to an intended change to the underlying method under test that didn't get reflected in the test (something new to mock). Or the effects of randomness.

And there are a bunch of methods I can't test in a reliable, because they rely on fundamentally random things I don't have control over (ie crypto-related stuff with one-time encoding).

Gribnit

@Benjamin-Hall said in Unit tests for methods with trivial logic?:

And there are a bunch of methods I can't test in a reliable,

You can behavior test them AND if you break out the randomness sources you can fix values and hard I/O test them.

Zenith

@Gribnit No. My code is great. But I'm not using JSON to pass parameters through an AJAX object into a web service through a SOAP layer into a stored procedure that calls seven other stored procedures to put a record into a queue table for some SQL agent job that runs every 10 minutes to process. Writing unit tests for code that doesn't end in a write operation somewhere is a thousand times easier. So I try to write code so the write operations are straightforward and far away from everything else.

Seriously, why can't I scroll on mobile anymore?

Grunnen

@Zenith In my experience, in such cases it can be rewarding to refactor the codebase so that somewhere in this process, you can swap something with a mock object that doesn't really write the data but lets you intercept them.

Zenith

@Grunnen Refactor? May as well ask for a pony too. We live in very different worlds. Only place I ever worked that even mentioned unit testing demanded it be done on JavaScript UIs (the type generated in the browser at runtime).

Benjamin Hall

@Zenith said in Unit tests for methods with trivial logic?:

Seriously, why can't I scroll on mobile anymore?

Yeah, this is really annoying and makes me not post from mobile at all. @julianlam Consider this a bug report--the mobile composer (on Android Chrome at least) does not scroll with the keyboard up, so if there's any text at all, it starts getting hidden by the soft keyboard real quickly.

Gribnit

@Zenith said in Unit tests for methods with trivial logic?:

@Grunnen Refactor? May as well ask for a pony too. We live in very different worlds. Only place I ever worked that even mentioned unit testing demanded it be done on JavaScript UIs (the type generated in the browser at runtime).

In my experience, it can be rewarding to send me a pony.

Jaloopa

@Gribnit As in a small horse or £20?

Gribnit

@Jaloopa said in Unit tests for methods with trivial logic?:

@Gribnit As in a small horse or £20?

Sure. Also beer.

Gąska

@Zenith said in Unit tests for methods with trivial logic?:

@Benjamin-Hall Simpler (as in static utility) functions/procedures are about all I'd write unit tests for. The rest is just too many moving parts or something you've outsourced to a framework anyway.

You really hate your job, don't you.

Gąska

@Grunnen said in Unit tests for methods with trivial logic?:

I'd suggest putting effort into sensible unittests rather than focusing on 100%.

Porque no los dos?

@Grunnen said in Unit tests for methods with trivial logic?:

Or like this:

my_json = create_some_json();
my_xml = process_request(my_json);
assert(my_xml.length() == 513);

There's a lot that can be improved but at least it tests something. Meaning you get a failure when the behavior changes at least most of the time.

Gąska

@Benjamin-Hall said in Unit tests for methods with trivial logic?:

@Grunnen Yeah. That's my experience--when the tests have started failing, it's 99.99% of the time due to an intended change to the underlying method under test that didn't get reflected in the test (something new to mock).

We haven't been in war for ages so why do we keep paying for the army?

Zenith

@Gąska said in Unit tests for methods with trivial logic?:

@Zenith said in Unit tests for methods with trivial logic?:

@Benjamin-Hall Simpler (as in static utility) functions/procedures are about all I'd write unit tests for. The rest is just too many moving parts or something you've outsourced to a framework anyway.

You really hate your job, don't you.

You have no idea.

Gribnit

@Zenith said in Unit tests for methods with trivial logic?:

@Gąska said in Unit tests for methods with trivial logic?:

@Zenith said in Unit tests for methods with trivial logic?:

@Benjamin-Hall Simpler (as in static utility) functions/procedures are about all I'd write unit tests for. The rest is just too many moving parts or something you've outsourced to a framework anyway.

You really hate your job, don't you.

You have no idea.

We have at least a local minimum.

Zerosquare

@Gąska said in Unit tests for methods with trivial logic?:

You really hate your job, don't you.

Actually, there's a unit test that checks @Zenith's level of hate for his job is equal to INT_MAX.

Gribnit

@Zerosquare said in Unit tests for methods with trivial logic?:

@Gąska said in Unit tests for methods with trivial logic?:

You really hate your job, don't you.

Actually, there's a unit test that checks @Zenith's level of hate for his job is equal to INT_MAX.

That's supposed to be LONG_MAX, whoops! Holdover from the 64-bit conversion.

Was wondering why he'd started blinking again.

HardwareGeek

@Gąska said in Unit tests for methods with trivial logic?:

Porque no los dos?

¿Porqué no los dos?

Filed under: No, autocarrot, I don't mean DOS, did or does

Gąska

@HardwareGeek if I wąńtęd tó put fuńńy śquiggłes iń my póśt, I wóułd.

error

@Zenith said in Unit tests for methods with trivial logic?:

My code is great.

Zenith

@error said in Unit tests for methods with trivial logic?:

@Zenith said in Unit tests for methods with trivial logic?:

My code is great.

Part of me wants to say "yeah, well, what would a Java developer know about code quality?" Part of me wants to remind you of dozens of data breaches and service outages (just those that hit the news or we'd be here all night) and how none of them were anywhere near systems that I was responsible for.

kazitor

@Gąska said in Unit tests for methods with trivial logic?:

@HardwareGeek if I wąńtęd tó put fuńńy śquiggłes iń my póśt, I wóułd.

Step one: Reply to @Gąska

Grunnen

@Gąska The aim of unittests is to get failures when you break a functionality, not to randomly sometimes get failures when you change something.

Gąska

@Grunnen how do you know if the change is something you wanted or broken functionality? How is the test suite supposed to know that?

Jaloopa

@Grunnen unit tests serve as documentation for maintainers too. If you change something that results in externally visible changes then it's perfectly valid for that to break a test, alerting you to the fact that you either need to update the test/documentation for the new behaviour, or that the change in behaviour was unwanted and you need to change your change

topspin

Since the verdict seems to be yes, test even trivial things, I'd like to chime in with a question myself on what I should test for these mostly trivial, slightly anonymized examples. (I have much more problematic things where I don't have tests, but they're on the opposite end where I neither know what nor how to test it)

Case 1: some simple "material law" that's basically a formula from Wikipedia / some paper / whatever:

double fooMaterial(double deltaT, double width, double lambda) {
    return exp(lambda) * exp(-4. * deltaT * deltaT / (width * width));
}

The code is obviously the same as the formula I want to implement, by inspection. I could test that some arbitrary values give the "expected" result, but that'd would just be me doing the same calculation again and then hard-coding them into the test. I could test that deltaT == 0 && lambda == 0 results in 1, but that's basically just a case that I could compute without a calculator. I have no idea what else I could test.
There is no reason to ever change this function (if you wanted to change the formula used, you'd rather change which function gets called than change this function) and if it did change, I have no idea what invariants it would still fulfill. Would every value I hard-coded into the test fail? Would the special-case I mentioned still hold?
What's the value in computing a few values and hard-coding them into the test?.

Case 2: a simple function that implements a "compute numerical residuals of boundary-value problem" interface.

class SomeModel : public IModel
{
public:
    SomeModel(double v, double T /*, ...*/) : velocityIn(v), tempIn(T) { /*, ...*/ }

    // types and indices for exposition only
    void computeBoundaryResiduals(const vector<double>& vars, vector<double>& residuals) override {
        residuals[0] = vars[0] - velocityIn;    // Solve for velocity = velocityIn
        residuals[1] = vars[1] - tempIn;        // Solve for temperature = tempIn
        residuals[2] = vars[2];                 // Solve for density = 0
    }

    // other stuff ...

private:
    double velocityIn, tempIn;
}

The function just basically sets fixed values into the residuals. The interpretation of that, however, is completely determined by this concrete SomeModelimplementation, the interface IModel has no notion if those values are "correct". I could write a test for this concrete class that checks that the values I provide when constructing it are actually set when I call the function.
But that is 1) trivial to see and 2) testing (and reproducing) complete implementation details, since I could swap the residuals[0] and residuals[1] indices around internally and still get the correct answers out, while breaking the test.

What I do instead is run a much larger test that exercises the complete solver for some example problems and check that those results don't change, but that is much larger than a unit and also, well, sub-optimal. But at least it gives me test coverage.

homoBalkanus

@Zenith said in Unit tests for methods with trivial logic?:

@error said in Unit tests for methods with trivial logic?:

@Zenith said in Unit tests for methods with trivial logic?:

My code is great.

Part of me wants to remind you of dozens of data breaches and service outages (just those that hit the news or we'd be here all night) and how none of them were anywhere near systems that I was responsible for.

I have never been hit by a bullet. Therfore I'm bulletproof.

homoBalkanus

@topspin
Case 1: are there boundaries to those values? It would be worth validating that values fall in correct ranges. Maybe checking if the double precision is what you aim for?

Case 2: some is better than none. Ideally you would be testing against the IModel interface only and for some tests you would provide your test implementation of IModel to test other parts of the solver under more controlled conditions.