Skip to content

Instantly share code, notes, and snippets.

@patmaddox
Created June 22, 2017 03:23
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save patmaddox/96900b89c73c01d19a73da064b6075a3 to your computer and use it in GitHub Desktop.
Save patmaddox/96900b89c73c01d19a73da064b6075a3 to your computer and use it in GitHub Desktop.
# Collaboration test...
describe Client do
it 'calls a_method on a server' do
client = Client.new
server = double 'server'
expect(server).to receive(:a_method).with(1, 2).and_return 3
client.do_it(server)
end
end
# For a server contract test, do you need to write:
# - one test for server.a_method(1, 2)
# - one test that server.a_method(something, something_else) returns 3
# and are they the same test, or separate tests?
@patmaddox
Copy link
Author

@jbrains my understanding is that because of the expectation expect(server).to receive(:a_method).with(1, 2) there needs to be a test that does: server.a_method(1, 2); and because of the stub .and_return 3, there needs to be a test that does expect(server.a_method(something, something_else)).to eq(3).

Is that correct? And do they need to be the same test, or separate tests? Does this expectation imply expect(server.a_method(1, 2)).to eq(3) ?

You say I need to match an expectation (receive(:a_method).with(1,2)) to an action, and a return value (and_return 3) to an assertion... but I'm not sure how that would look in this example.

@jbrains
Copy link

jbrains commented Jun 22, 2017

Short answer: Yes, no (they don't need to be the same test), yes.

Longer Answer

Given this collaboration test, I'd expect to add to the contract of Server the property a_method(1, 2) => 3 given whatever state I might need to care about. (None, I hope.) This would intuitively lead me to check this with a contract test: expect(server_in_the_necessary_state.a_method(1, 2)).to eq(3). Here, we would be combining two tests:

  1. A test for Server with action server.a_method(1, 2). (What would it expect? Always 3? Always? Hm... maybe we can purify this function...)
  2. A test for Server that shows when server.a_method() returns 3. (Which combination of parameters and state would make that happen? Could we kill the state maybe?)

Depending on the meaning of server.a_method() I might prefer to separate these two tests, but without more context, I'd combine them until I saw evidence that I should separate them. Many people find separating these tests pedantic, so I take a more pragmatic approach.

About the collaboration test...

Generally, in a single collaboration test, I don't like to set an expectation on a method and stub a return value. (Stub queries; expect actions.) I do this even if I'm not violating command/query separation, although I interpret as a sign to examine my query for command-ish behavior, just to be safe. Typically, those two doubles help me check two different aspects of the client's behavior:

  • The stub helps me check how Client reacts when server.a_method() returns the significant value 3. (I'll add more stubs to check for other significant return values.)
  • The expectations helps me check that Client invokes server.a_method() at the right time, and possibly specifically with parameters 1, 2. (For example: since the clock reads "2017-06-22 00:39 -0300", Client invokes server.find_orders_open_as_of() with Date:2017-06-22. Here, I'm checking Client's behavior of handling "now" and not infecting server with it. In checking this behavior, I don't yet care what Client does with those open orders.)

Taking my more specific example, which gives me enough details to judge whether to separate the stubs from the expectations, and thinking of the case where a controller asks for all the orders open as of date:

  • when I check that the controller handles "now" correctly, I don't care how the controller handles the open orders, nor, indeed, which open orders the server returns.
  • when I check how the controller handles 0, 1, many, lots of open orders, I don't care what date they correspond to, nor, indeed, that the query even requires a date.

In this situation, it feels pretty obviously "good" to me to have separate collaboration tests for the stub(s) and expectation(s), in order to avoid putting irrelevant details into the corresponding tests.

The corresponding Contract Tests

This turns out not to affect how I write the contract tests. All the contract tests will boil down to: "given a server with these open orders matching these dates, find_orders_open_as_of() only matches the right ones". The different cases will check the significant return value cases of 0, 1, many, and lots of open orders and every test will have the action "server.find_orders_open_as_of(some_valid_date)". In a language with compile-time type checking, the type checker even runs that last test for me when I type implements Server. We can then decide whether to formally check what happens to find_orders_open_as_of(nil) or merely note the usual contract "if you give me nil, I do what I want".

@patmaddox
Copy link
Author

Okay so taking your more specific orders example, it might look something like:

describe Controller, 'count_open_orders' do
  let(:database) { double 'database', find_orders_open_as_of: [] }
  let(:controller) { Controller.new database }

  it 'fetches open orders as of now' do
    expect(database).to receive(:find_orders_open_as_of).with('now') # returns default [] which we don't care about for this test
    controller.count_open_orders
  end

  it 'returns 0 for no open orders' do
    expect(controller.count_open_orders).to eq(0)
  end

  it 'returns 1 for one open order' do
    allow(database).to receive(:find_orders_open_as_of).and_return [double('order')]
    expect(controller.count_open_orders).to eq(1)
  end
end

"fetches open orders as of now" checks that controller correctly passes 'now' to database.find_orders_open_as_of, and the (super boring) "returns X for N open orders" tests check that controller correctly counts the records returned.

Splitting them up communicates a contract of: controller expects to pass a single argument to database.find_orders_open_as_of and get an array back, but doesn't expect a specific return value when 'now' is passed in. I like that.

Let's make it a tiny bit more interesting... now the controller will sum the total of open orders:

describe Controller, 'sum_open_order_totals' do
  let(:database) { double 'database', find_orders_open_as_of: [] }
  let(:controller) { Controller.new database }

  it 'fetches open orders as of now' do
    expect(database).to receive(:find_orders_open_as_of).with('now') # returns default [] which we don't care about
    controller.sum_open_order_totals
  end

  it 'returns 0 for no open orders' do
    expect(controller.sum_open_order_totals).to eq(0)
  end

  it 'sums order totals of multiple open orders' do
    allow(database).to receive(:find_orders_open_as_of).and_return [
      double('order 1', total: 1),
      double('order 2', total: 2)
    ]
    expect(controller.sum_open_order_totals).to eq(3)
  end
end

Here I'm mainly interested in the fact that 'database.find_orders_open_as_ofreturns a list of objects that respond tototal– and presumably for thattotal` to respond to '+' based on the summing logic. How do you suggest I write a contract test for that? Something like...

describe 'database contract: find_orders_open_as_of("now")', shared: true do
  it 'returns objects that respond to #total' do
    results = database.find_orders_open_as_of 'now'
    expect(results).to have_at_least(1).item
    results.each do |r|
      expect(r).to respond_to(:total)
      expect(r.total).to respond_to(:+)
    end
  end
end

By writing my collaboration test, I've discovered that:

  • I need a database object that responds to find_orders_open_as_of('now')
  • it should return a list of things
  • those things should all have a method called total on them
  • the return value of total should respond to +

It feels a bit off to me that I'd be confirming all that in a single contract test.

It may be that the contract test isn't specific enough, because it doesn't follow your rules:

  • A stub in a collaboration test must correspond to an expected result in a contract test
  • An expectation in a collaboration test must correspond to an action in a contract test

which would produce something closer to:

describe 'database contract: find_orders_open_as_of("now")', shared: true do
  it 'returns an empty list for no orders' do
    expect(database.find_orders_open_as_of('now')).to eq([])
  end

  it 'returns open orders' do
    order_1 = create_open_order total: 1
    order_2 = create_open_order total: 2

    create_closed_order total: 5

    expect(database.find_orders_open_as_of('now'))
      .to match_array([order_1, order_2])
  end
end

BUT my controller probably doesn't care that total is an integer... just that it can be summed. Would you represent that in the contract test, and if so, how? Using the respond_to stuff from above?

@patmaddox
Copy link
Author

Here's a bit of context on the original problem that prompted these questions...

I'm looking at a React component which uses some struct data. It expects the data in a specific structure. Imagine something like contact.address.city. The struct data is populated by a service object which basically delegates to a REST client.

My challenge then is to:

  • check that the service object provides the structure the React component expects
  • check that the REST client provides the structure that the service object expects

Here's what I'm thinking...

Testing the React component

  1. Provide it with stub data in the structure it expects
  2. Assert it doesn't blow up
  3. Maybe assert some other stuff

This stub data represents the "answer" from the service object, which means I need a test on the service object that asserts it can return that data...

Testing the service object

  1. Assert the service object returns data matching the stubbed data from the react component
  2. Provide the service object with a mock REST client that returns necessary data structure
  3. Expect the service object to call the mock REST client with correct arguments

Now I have an expectation on and returned data from the REST client...

Testing the REST client

  1. Call the REST client with the same arguments as service object step #3, and assert that it returns the correct data

Am I on the right track?

@jbrains
Copy link

jbrains commented Jun 27, 2017

In the case where Controller uses Database.sum_open_order_totals(), I imagine that the Controller doesn't actually care about the meaning of "sum", except perhaps for the implied property that sum >= 0. In that case, Controller probably doesn't care about the difference between count_open_orders() and sum_open_order_totals(), so I would let the Controller make the same assumptions about both: return value is a non-negative number. The Database cares about computing the sum correctly, the feature cares about using the sum instead of a count of rows, but the Controller probably doesn't care.

Accordingly, the Controller stubs Database.sum_open_order_totals() to return 0, another non-negative number, to raise an error (if that makes sense) and that's it. If sum_open_order_totals() were really sum_open_order_totals_as_of(date), then I would treat it the same way we did before.

It's an open question whether Database.sum_open_order_totals() needs "sum" to be part of its contract. I would treat this as an implementation detail until I encountered a situation where I felt doubt. What matters most is that sum_open_order_totals() returns a value that is plausible as a sum (>= 0) and raises errors or not as the situation warrants. When we implement Database, we'll write at least one test for sum = reduce (+) 0 (map orders quantity) that puts in 3 orders with quantities 1, 3, 5 and expects a sum of 9. We can leave that as an implementation detail test until the contract test becomes helpful. For example, do we want to make it an explicit part of the contract that Orders have a quantity property? or do we just want to leave implicit the notion that Orders can be "summed" somehow? I prefer the latter until we judge that that's no longer good enough. Doubt, a failing test, or a customer-reported mistake would prompt me to action.

I agree that it feels off to check in a Contract Test that the orders inside the Database are summable (even worse, that they respond to :+! Implementation detail much?!).

Does the Controller care how the Database calculates the sum of the open orders? Probably not. In that case, let Database own the notion of "sum". The Contract Tests for Database might include a few examples of computing the sum of the open orders, but that's useful more for documentation for future clients, rather than for the current Controller.

On To Your React Example

React Component

Add a test for what happens if the stubbed data has the wrong structure. Default values? Blow up?

For a find() method returning at most one item, in general, I want to check this:

  • returns nothing
  • returns something, good format
  • returns something, bad format
    • returns something missing an important property (default value? blow up?)
    • returns something missing an unimportant property (default value? nothing? empty string?)
  • raises error

Service object

This appears to wrap a REST endpoint in order to decouple the React component from the data source. In that case:

  • data source returns something valid
  • data source returns something invalid
  • data source returns nothing
  • data source blows up

REST Endpoint

Integrated tests with server and/or VCR-style "integrated tests" with real data + fake transport

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment