Excessive warn "was both stubbed and verified"

[moeriki]

I have the following test.

it('should keep result in memory', function () {
  // setup
  td.replace(testStore, 'get');
  td.when(
    testStore.get('myStore')
  ).thenResolve('[]');
  const manager = storyManager('myStore');
  // test
  return manager.get().then(() =>
    manager.get().then(() => {
      td.verify(testStore.get('myStore'), { times: 1 });
    })
  );
});

Which yields the warning test double 'get' was both stubbed and verified. Yet I cannot remove the stub, because it needs to resolve a Promise. Neither can I remove the verify because I need to check if times is 1 for my test to be correct.

Can we consider not warning when times is used in verification?

[djheru]

I also have a use case that yields this warning.

describe('MyComponent.loadData()', function () {
    beforeEach(function () {
        var myParam = 12345;
        var someCollection;// = some mock data that is operated upon by the method under test
        td.replace(MyInternalService, 'getSomeData');
        td.when(MyInternalService.getSomeData({param: myParam}))
                .thenReturn({
                    $promise: new Promise(function(resolve) { resolve(someCollection); })
                });
    });

    it('should call MyInternalService.getSomeData()', function () {
        MyComponent.loadData(12345);
        //did the code under test call the stubbed method it was supposed to?
        td.verify(MyInternalService.getSomeData({param: 12345}));
    });

    it('should return a collection of widgets', function () {
        //tests to ensure that MyComponent did whatever it was supposed to
        //with the data that I mocked and stored in the someCollection variable
    });
});

In some instances, I'm verifying to see if the stubbed method is called by the method being tested (so I need verify), in others, I'm testing that the mock data returned by the stubbed method has been processed as expected (so I need to stub).

Am I just structuring things incorrectly? Can you add an option or something to turn this off? This creates a lot of noise. My primary use case is testing Angular services that call Resources, which are being mocked by td.

[paulwib]

I had a similar issues and went with the recommendation in #125 to use explain to check the call count e.g.

assert.equal(1, td.explain(testStore.get).callCount);

[mockdeep]

We saw this one as well and came across an explanation in the FAQ:

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

However, I don't actually agree with this explanation for the warning. Personally, I want to know that a stub got called so that I can be sure it's not stubbed unnecessarily. If we change the implementation in such a way that the stub is no longer called, I want a warning from my tests that lets me know to either change or remove the stub. So we want to pair both a .when and then later a .verify.

[searls]

Technical answer

For anyone who reads this reply and still disagrees and wants to do this, td.js includes a configuration option to ignore warnings. All warnings represent a gentle codification of some opinion we thought was important to guide the majority of people using the library (given that the vast majority of test double libraries go on to be largely misunderstood and abused).

Opinions

@mockdeep it sounds like what you really want is a mock object, which Test Double (intentionally) does not provide.

Stubs provide canned responses to the subject to facilitate downstream behavior—if the subject is able to accomplish that without relying on the stub, no harm is done. No more than if you'd passed an extra constructor arg to the subject you didn't strictly need to send, or added any other unnecessary complication to a test you didn't need. A stubbing's specification of the subject's behavior is at most indirect. Your proposal (along with how mock objects work) require every stubbing to be called just-as-intended, which is almost always an example of overspecification. There may be extreme examples where this is preferable, but it's almost definitely not the norm, and as a result I wouldn't want to encourage it with Test Double's API.

[mockdeep]

@searls, so that I understand, here's an example of what I'm talking about:

var fakeAxios = { get: td.function() };
td.replace('axios', fakeAxios);
var myThing = require('path/to/my/thing');

describe('stuff', function () {
  it('does stuff', function () {
    td.when(fakeAxios.get('some_path')).thenReturn(somePromise);
    var response = myThing.doSomething();
    expect(response.success).to.be.true();
  });
});

In my case, if doSomething stops relying on axios, I would want to have a way to have my tests fail and let me know that I can update them. I guess I don't see that as over-specification if it's helping to maintain test quality as well. You're saying you'd be comfortable just letting the test be?

[searls]

If the subject can magically keep working without a stubbing that has been configured for it, then by all means I'm glad to see it pass.

It has never happened to me in seven years of writing isolated unit tests and I doubt t ever will. As a result, adding additional specifications that "I call the thing I just set up because the subject needs to call it" is indeed costly overspecification IME

On Nov 14, 2016, at 11:24, Robert Fletcher notifications@github.com wrote:

@searls, so that I understand, here's an example of what I'm talking about:

var fakeAxios = { get: td.function() };
td.replace('axios', fakeAxios);
var myThing = require('path/to/my/thing');

describe('stuff', function () {
it('does stuff', function () {
td.when(fakeAxios.get('some_path')).thenReturn(somePromise);
var response = myThing.doSomething();
expect(response.success).to.be.true();
});
});
In my case, if doSomething stops relying on axios, I would want to have a way to have my tests fail and let me know that I can update them. I guess I don't see that as over-specification if it's helping to maintain test quality as well. You're saying you'd be comfortable just letting the test be?

―
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[mockdeep]

This has happened to us a number of times. Usually it's because it ends up transparently calling through to another class that we thought we had stubbed and maybe renamed or changed the interface for during a refactor. In particular, this is a strategy we use for tests that perform poorly because the dependency has some labor intensive work to do, so we stub it out and test it individually.

[searls]

You're of course absolutely free to use the library however best suits your needs, but the usage you described is outside the library's chartered purpose (of facilitating fully isolated unit tests for the design benefit of working outside-in). I'd recommend suppressing the warnings in your case. More on the library's purpose here: https://github.com/testdouble/testdouble.js/blob/master/docs/2-howto-purpose.md