Sandi Metz advice for writing tests

rules for good testing.md

Rules for good testing

Look at the following image...

...it shows an object being tested.

You can't see inside the object. All you can do is send it messages. This is an important point to make because we should be "testing the interface, and NOT the implementation" - doing so will allow us to change the implementation without causing our tests to break.

Messages can go 'into' an object and can be sent 'out' from an object (as you can see from the image above, there are messages going in as well as messages going out). That's fine, that's how objects communicate.

Now there are two types of messages: 'query' and 'command'...

Queries

Queries are messages that "return something" and "change nothing".

In programming terms they are "getters" and not "setters".

Commands

Commands are messages that "return nothing" and "change something".

In programming terms they are "setters" and not "getters".

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

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

What to Mock/Stub

Command messages should be mocked, while query messages should be stubbed

Contract Tests

Contract tests exist to ensure a specific 'role' (or 'interface' by another - stricter - name) actually presents an API that we expect.

These types of tests can be useful to ensure third party APIs do (or don't) cause our code to break when we update the version of the software.

Note: if the libraries we use follow Semantic Versioning then this should only happen when we do a major version upgrade. But it's still good to have contract/role/interface tests in place to catch any problems.

The following is a modified example (written in Ruby) borrowed from the book "Practical Object-Oriented Design in Ruby":

# Following test asserts that SomeObject (@some_object) 
# implements the method `some_x_interface_method`
module SomeObjectInterfaceTest
  def test_object_implements_the_x_interface
    assert_respond_to(@some_object, :some_x_interface_method)
  end
end

# Following test proves that Foobar implements the SomeObject role correctly
# i.e. Foobar implements the SomeObject interface
class FoobarTest < MiniTest::Unit::TestCase
  include SomeObjectInterfaceTest

  def setup
    @foobar = @some_object = Foobar.new
  end

  # ...other tests...
end

Thanks for your thorough response.

The test has to specify the Stub, so the test has to define what it returns when asked
....
So the information ... is part of the assertion and should be right next to it in the same test.

In Ruby we can (and do, with the ubiquitous testing framework RSpec) define such things inline in the test - in this case as

instance_double(WeatherApi, weather: :rain)

I was only writing it as typescript class for the sake of mutual intelligibility, but I see that this was misleading in another way.

Regardless: reading between the lines I interpret your main point as this:

To be a useful stub, it should only return the relevant result in the scenario that we're testing.

If so then that makes sense, and we have a syntax for this in RSpec:

api_stub = instance_double(WeatherApi)
allow(api_stub).to receive(:weather).with("London").and_return(:rain)

This, like WeatherStub:

• doesn't assert that the message was called
• will raise an error if "New York" or "london" is passed

This way we avoid the arbitrary values ("Sunny", "Manchester" etc.): I prefer to avoid arbitrary values, because it's not 100% clear to the reader whether it's significant that this specific value is used, or whether any other value would fulfil the same purpose.

Thanks for your thorough response.

The test has to specify the Stub, so the test has to define what it returns when asked
....
So the information ... is part of the assertion and should be right next to it in the same test.

In Ruby we can (and do, with the ubiquitous testing framework RSpec) define such things inline in the test - in this case as

instance_double(WeatherApi, weather: :rain)

I was only writing it as typescript class for the sake of mutual intelligibility, but I see that this was misleading in another way.

Regardless: reading between the lines I interpret your main point as this:

To be a useful stub, it should only return the relevant result in the scenario that we're testing.

If so then that makes sense, and we have a syntax for this in RSpec:

api_stub = instance_double(WeatherApi)
allow(api_stub).to receive(:weather).with("London").and_return(:rain)

This, like WeatherStub:

• doesn't assert that the message was called
• will raise an error if "New York" or "london" is passed

This way we avoid the arbitrary values ("Sunny", "Manchester" etc.): I prefer to avoid arbitrary values, because it's not 100% clear to the reader whether it's significant that this specific value is used, or whether any other value would fulfil the same purpose.

Yes, to really shorten the conversation down:
The test should contain all details that matter for the particular scenario being tested, and it should not contain any details about the current implementation.

Let's connect that with Sandi's talk from the original idea way up in the thread:

• Why not test outgoing queries?
• Because an outgoing query will definitely be tested as a scenario-fixture for one of the command tests or incoming queries

I can see how this can be confusing if you do not pin down tests to each scenario, and use broad-spectrum mocks instead.

OK thanks again for all the discussion - all the best of luck with your projects.

OK thanks again for all the discussion - all the best of luck with your projects.

Thank you too, nice conversation!