td.when and td.verify warning

[bjarketrux]

Given a td.When followed by a td.verify I can get the following warning:

Warning: testdouble.js - td.verify - test double `myDouble` was both stubbed and verified with arguments (), which is redundant and probably unnecessary. (see: https://github.com/testdouble/testdouble.js/blob/master/docs/B-frequently-asked-questions.md#why-shouldnt-i-call-both-tdwhen-and-tdverify-for-a-single-interaction-with-a-test-double )

But in some cases it is exactly what I want.
Example:

function fetchCached(id) {
    var item = cache[id];

    if (!item) {
        item = server.fetch(id);
        cache[id] = item;
    }

    return item;
}


describe("fetchCached", function() {
    beforeEach(() => {
        td.replace(server, "fetch");
        td.when(server.fetch("123")).thenReturn({ a: 1 });
    });

    it("should only call the server the first time", function() {
        fetchCached("123");
        fetchCached("123");
        td.verify(server.fetch("123"), { times: 1 });
    });
});

I could use td.explain, but that seems like it is for debugging... Any suggestions?
I want to verify that the method is called only once.

[searls]

Good question!

Since what you're trying to assert is the call count and not verify that it was called, I think I'd recommend verifying that by asserting td.explain(server.fetch).callCount, there's no shame in that and it'd make your intention more explicit IMO. (the td.explain API is stable so no worries about it feeling "debuggy")

[guboi72]

What if we wanted to verify that it was called ? That's my problem right now.

[searls]

@guboi72 -- if you delete the stubbing (td.when), does the test fail? If so, then you don't need a td.verify, because the stubbing is indirectly asserting the call was made. If not, then delete the stubbing and verify the call with td.verify.

[connor4312]

if you delete the stubbing (td.when), does the test fail? If so, then you don't need a td.verify, because the stubbing is indirectly asserting the call was made

I disagree with this. Test should clearly demonstrate and be able to require behavior. For instance, I could have some code like this:

function incrementFooGood(db) {
  myModel.foo++;
  return myModel.validate().then(() => db.save(myModel));
}

function incrementFooBad(db) {
  myModel.foo++;
  return myModel.validate();
}

To test any implementation of incrementFoo I would stub the db.save method and td.when(db.save(myModel)).resolves(undefined). We need to verify that db.save is called, we can't just depend on other things randomly failing if the call doesn't happen -- the purpose of testing code is verifying its functionality/behavior is correct, this is not feasible to do if we're unable to verify that calls are made successfully.

[searls]

Wait, I'm confused. I think I need a more complete example for your point to make sense to me.

[carlbennettnz]

If I understand what @bjarketrux was saying, I think I'm running into the same issue. Here's the code I'm trying to test:

function applyFilters(query, filters) {
  for (const key in filters) {
    query = query.where(key, '=', filters[key])
  }

  return query
}

And here's how I'm testing it:

it('applies filters', function() {
  const query = { where: td.function() }

  td.when(query.where(), { ignoreExtraArgs: true, times: 3 }).thenReturn(query)

  const r = applyFilters(query, { a: 1, b: 'x', c: null })

  expect(r).to.exist

  td.verify(query.where('a', '=', 1))
  td.verify(query.where('b', '=', 'x'))
  td.verify(query.where('c', '=', null))
})

This seems to do exactly what I want, except it gives the "test double was both stubbed and verified" warning in the console. I could make the .when stricter by stubbing each call separately, but I can't figure out how to do that while also checking that each was called exactly once. It seems { times: 1 } means at least once.

[bjarketrux]

Hi Carl

A couple of notes:

1. The times can only be used on a td.verify, not on a td.when.
2. times is exactly x times, not at least (as far as i know). So you can put { times: 1 } in each to the td.verify().

Other than that I think this is a valid example of where the warning is only a warning, i.e. a situation where chaining is necessary. You can use the td.explain() here as searls suggests, but it would not look as clean.

Maybe if we can come up with enough compelling cases an ignoreWarning flag could be added, e.g.:

td.verify(query.where('a', '=', 1), { times: 1, ignoreWarning: true });

[searls]

To reply to @bjarketrux:

The times can only be used on a td.verify, not on a td.when.

In fact, td.when(func(), {times: 3}).thenReturn('lol') is supported. It'll do this:

func() // lol
func() // lol
func() // lol
func() // undefined

However I'm not sure it's a useful thing to set in the above case. (See below)

To reply to @carlbennettnz:

If I were testing this function:

function applyFilters(query, filters) {
  for (const key in filters) {
    query = query.where(key, '=', filters[key])
  }

  return query
}

Here is what you could do to test it:

const td = require('testdouble')
const assert = require('assert')

const query1 = td.object(['where'])
const query2 = td.object(['where'])
const query3 = td.object(['where'])
td.when(query1.where('a', '=', 1)).thenReturn(query2)
td.when(query2.where('b', '=', 'x')).thenReturn(query3)
td.when(query3.where('c', '=', null)).thenReturn('huzzah!')

const result = applyFilters(query1, { a: 1, b: 'x', c: null })

assert.equal(result, 'huzzah!')

You can see this in action on runkit

Because the method has no side effect, asserting this with td.verify is insufficient. As you can see in the example you provided, the result r is not actually verified to be correct nor is the order of the invocations validated at any point (though I can't know whether that's meaningful to the function).

So again in this case, I feel like this is the warning doing its job—I don't believe stubbing & verifying the same function was actually appropriate. The key difference is that from the perspective of the caller, the fact that a query is returned for each where() invocation, it doesn't know or need to know whether the returned query is a mutated instance or a different instance, and it's safer to assume it could be either (as the test above does)

[variousauthors]

A workaround for this is:

td.when(theFunctionYouWantToVerify(td.matchers.anything()))
  .thenThrow(new Error('whatever'))

expect(subjectUnderTest).not.toThrow() // or your favourite variation thereon

if you delete the stubbing (td.when), does the test fail? If so, then you don't need a td.verify, because the stubbing is indirectly asserting the call was made. If not, then delete the stubbing and verify the call with td.verify.

This is one of those times where I'm ready to accept being totally wrong, I could 100% be convinced that stub then verify is redundant. Here's my feeling:

You write some code, and then you write a test, and in the test you stub a function. You run the function, it passes. You remove the stub, and it fails. You are confident that your test demonstrates "indirectly" that the call was made.

Are you going to remove that stub manually every time your code changes?

Does that argument make sense? A contrived example: suppose you have this,

const nextNumber = (x) => {
  return addOne(x)
}

describe('nextNumber', () => {
  it('returns 1 more than the given number', () => {
    td.when(addOne(5)).thenReturn(6)
  
    const result = myFunction(5)

    expect(result).toBe(6)
  })
})

and a cosmic ray comes along and strikes your code, replacing the implementation of nextNumber with,

cons nextNumber = (x) => {
  return 6
}

You may not notice right away what has happened. You may release some code onto production that doesn't do what it claims. This is a regression, and some regressions are tolerable. Some aren't. Sometimes I know when I am writing tests that if, by some fluke, a certain call gets skipped because of mutations in the code that I couldn't predict there will be consequences. Consequences... in the database.

This is why whenever I come back to this issue I am always a little confused and a little frustrated. Sometimes I want my test to explicitly verify that some API method was indeed called without hitting the database. For example right now I'm writing validation for nested models and I would like to know that the validate function for model Car calls the validate function for CarDoor. It isn't a big test, didn't take me five minutes to write. Too me 10 or 15 more minutes to fix this warning though. (took me quite a bit longer to write this post ;) it all adds up you know).

I like principles though! I am always happy when a principled person sticks to their principles. And testdouble is a great library and all so I'm fine with having to work around this every so often when things get a little weird.