3

I was watching a recording of a conference by Sandi Metz on testing. One of the things that I struggled with was that she said not to test outputs of the object under test (her example was query messages since she uses rails.) Here is a gist summarizing what she was saying and here is the full talk.

Let's say I have some object A that I want to test and A is on the client-side of my web application. It takes to some API and I want to make sure it is requesting data from the right portion of the API (e.g. a PATCH to /some/url with some body payload.)

That seems to me like part of the contract of the object? Am I testing my code the correct way? Am I misinterpreting the point Sandi was trying to get across? I really am focusing on not writing brittle tests but that still test what need to be tested.

Christophe
  • 74,672
  • 10
  • 115
  • 187
Adam Thompson
  • 1,243
  • 1
  • 9
  • 14

4 Answers4

2

We should strive to test features, not implementation details. Very simply put, features are requested by the customer/product owner. Implementation details are choices made by developers that the customer/product owner is unaware of or doesn't care about.

To simplify your scenario, consider testing whether the error message returned by A is the correct message.

Usually, you shouldn't be testing this. The specific error message is an implementation detail. Whether it is "Error 001 occurred!" or "Cannot find user in database!" or "Kan de gebruiker niet vinden!" (Dutch translation) is irrelevant. What we seek to test is its behavior. Therefore, we can test if an exception is thrown, and if that thrown exception was of the correct type, but we should not be testing the exception's message.

There is an exception to this:
When the functional (not just the technical) analysis explicitly defines the error message. For example, consider that we're building an API for a customer who expects to receive an exception with message "USER_NOT_FOUND" when an unknown user is requested.
In this case, the error message is no longer an implementation detail, it is a actual feature of the application. Therefore, we must test it.

Applying the same principle to your example:

Let's say I have some object A that I want to test and A is on the client-side of my web application. It takes to some API and I want to make sure it is requesting data from the right portion of the API (e.g. a PATCH to /some/url with some body payload.)

You can always test if A sends out a web request. But the specific URL it uses should not always be tested.

If the API and client application are considered one solution/business project, then their communication (e.g. specific URL names) is an implementation detail and should not be tested.

If the API already exists and you cannot change it, then using it correctly is part of the requirements of your client application, and therefore it can be meaningfully tested.

This is also reflected in the gist you linked:

What NOT to test?

  • Messages that are sent from within the object itself (e.g. private methods).
  • Outgoing query messages (as they have no public side effects)
  • Outgoing command messages (use mocks and set expectations on behaviour to ensure rest of your code pass without error)
  • Incoming messages that have no dependants (just remove those tests)

Note: there is no point in testing outgoing messages because they should be tested as incoming messages on another object

Flater
  • 44,596
  • 8
  • 88
  • 122
  • Interesting point about considering the fetcher/API as one component and testing the behaviour of that rather than each piece individually. I think that is the key I was missing. Probably it's better to test them cohesively through an some integration/end-to-end and leave out the implementation details of their exact communication in order to reduce test brittleness. – Adam Thompson Aug 29 '18 at 17:19
  • 1
    @AdamThompson: Note that you can test the internal component communication, but that's a different kind of testing. Testing the external behavior takes precedence. Testing the internal behavior can be a separate step if you want it to be, but it's usually only useful in cases where different parts of the application are developed by completely different teams (at which point you can actually start to argue that these are two separate projects that have the same project name). – Flater Aug 30 '18 at 07:51
  • Got it, so I need some integration/end-to-end glue testing to make sure everything is tied together nicely, but at the unit level is the wrong place for it because it's tied too closely to implementation details and creates brittleness? – Adam Thompson Aug 30 '18 at 16:01
  • 1
    @AdamThompson Yes. When unit testing, you don't test the dependencies of the object you're testing. You _mock_ the dependencies. This does actually give you _some_ indirect proof that the tested object handles the mocked dependency as you expect it to, but it's not the focus of the test. – Flater Aug 30 '18 at 17:35
  • right, but it would be better just to respond to ANY request (not just the one I expect?) since that would tie it less to implementation details. – Adam Thompson Aug 31 '18 at 02:44
2

I think you are misinterpreting the talk.

What to test?

  • Test incoming query messages by making assertions about what they send back
  • Test incoming command messages by making assertions about direct public side effects

So you do check what the 'output' of the object is. What you don't test is how it achieves that output, even if its using 'Output Messages' to other objects.

So in your example, no you don't care what API your client talks to and how. All you care about is that when you call the client, it gives you the right result.

Community
  • 1
Ewan
  • 70,664
  • 5
  • 76
  • 161
  • If you don't care about what API your object talks to, then this means you must have some sort of integration/end-to-end test to make sure everything is glued together correctly right? – Adam Thompson Aug 29 '18 at 17:15
  • unless you have more than one identical working api that wont be a problem – Ewan Aug 29 '18 at 19:23
  • But then how do you know your object under test is communicating w/ the API correctly since for a unit test you are stubbing the response? – Adam Thompson Aug 30 '18 at 04:36
  • you mean, what if your mock returns the correct response to the wrong request? Well 1. test your mock works before you use it. and 2. have integration tests as well which use the real api – Ewan Aug 30 '18 at 06:21
  • But if the mock only responds to a specific API call with specific parameters I'm back to square one (e.g. I am now testing that the object calls the specific API.) – Adam Thompson Aug 30 '18 at 16:00
  • 1
    @AdamThompson you are testing the object works, not how it works. If you mock a dependency incorrectly, then yes you might get false positives but your integration test (as you say) will fail. Your integration test is still just testing that the object works, not what api it calls. – Ewan Aug 30 '18 at 16:02
1

I really am focusing on not writing brittle tests but that still test what need to be tested.

OK, if that's your goal (and it is a good one...)

One of the things that makes tests brittle is that the test becomes coupled to implementation details. Classes are like module, in that part of the motivation for creating a new class is that we've made some decision about how the solution should be implemented, and we want to limit the amount of code that knows the decision that we made. See On the criteria to be used in decomposing systems into modules (Parnas, 1982).

In particular, coupling your test to what a command should be doing is, from this perspective, risky. Instead, look for the query that you can invoke to confirm that the command did the expected thing.

Now, you are absolutely right that somewhere there is a bit of code that, given inputs x,y,z is supposed to produce a particular http request. And you will want to write a test for that code (easy - it's a query). But writing a test to confirm that A calls this function? Yikes.

A talk that may help for your specific example is Gary Bernhardt's Boundaries. Part of the point is to keep the boundary thin; if the code isn't doing anything surprising, then it is easy to write it in a way that is too simple to hide a mistake. When we need to write tests of complicated code that talk to the boundary, we can easily replace a real boundary with test doubles (think ports and adapters).

There are, of course, some strategies for testing interactions across a process boundary. Contract testing, using tools like Pact, is one possibility. My experience has been that these strategies are more expensive, and so you want to limit them to the thin end of the testing pyramid.

Note: HTTP Patch is not safe, which is another way of saying that it is a "command". So in Sandi's grid, it falls into the expect to send bucket, with the big letter warnings about the implications of API drift.

HTTP "commands" are a little bit confusing, because they return messages, and Sandi says that commands are return nothing? And that's true, in an environment where the CQS pattern makes sense, and you have strong message delivery guarantees. But the network is not reliable, and you need some sort of acknowledgement to know that the command message got through.

So you might end up with two different sorts of tests - one where the test double for the http client is a mock, and you "expect" the correct http request to be sent, and then another sort where the test double of the http client is a "stub" that returns some fixed response to the test subject.

Both of these sorts of tests depend on the test doubles being a faithful representation of the api contract between the client and server, so you'll want to be careful there -- in particular, being willing to discard the tests and start over if somebody introduces a backwards incompatible change to that API.

Community
  • 1
VoiceOfUnreason
  • 32,131
  • 2
  • 42
  • 79
  • I think you are correct - it should be treated like a getter/setter method of a class (tested indirectly but not directly!) – Adam Thompson Aug 29 '18 at 17:20
0

First, make sure you and Sandi have the same priorities. Outside of academia and research, software eng isn't a science, since it's done at the behest of businesspeople and business isn't a science. By declaring severe limits on her testing strategy, Sandi is aiming for speed of run and speed of refactor by deduplicating her test code. You, on the other hand, may be writing embedded software for a pacemaker - in that case, you'd be much less worried about duplicate coverage than by not enough.

Second, this testing limitation she proposes probably works great along with some other practice she isn't mentioning, like maybe dependency inversion, or one that she kinda touches on, like good contract tests separate from the unit tests. By itself, your instincts are correct; no it's not valid to say that you've unit tested an object without checking the state changes it makes on collaborators. I think she's just advocating for a certain way of checking that, not that you should fail to!

Sapphon
  • 19
  • 2