jasmine / jasmine

Simple JavaScript testing framework for browsers and node.js
http://jasmine.github.io/
MIT License
15.74k stars 2.24k forks source link

Feature request: Matcher that verifies that none of the spies in a SpyObj were called #1568

Closed pcroc closed 2 years ago

pcroc commented 6 years ago

Desired Behavior

A matcher that verifies that none of the spies of the specified object have been called - i.e. there have been zero interactions with an entire mock service.

httpClient = jasmine.createSpyObj('HttpClient',
  ['get', 'post', 'put', 'delete', 'head', 'options', 'patch']);

// Checks that there have been no calls to httpClient.get, httpClient.post, etc.
expect(httpClient).not.toHaveBeenCalled();

Current Behavior

Error: <toHaveBeenCalled> : Expected a spy, but got
  Object({ get: spy on HttpClient.get, post: spy on HttpClient.post, ... })

Thus requiring:

expect(httpClient.get).not.toHaveBeenCalled();
expect(httpClient.post).not.toHaveBeenCalled();
expect(httpClient.put).not.toHaveBeenCalled();
expect(httpClient.delete).not.toHaveBeenCalled();
expect(httpClient.options).not.toHaveBeenCalled();
expect(httpClient.head).not.toHaveBeenCalled();
expect(httpClient.patch).not.toHaveBeenCalled();

Context

If I want to test there have been no HTTP calls for a specific condition, I don't want to have to check every possible method - someone might add a PUT in there but the test doesn't checking for that so still passes.

Other frameworks such as Java's Mockito have this capability - Mockito.verifyZeroInteractions(mock) - to allow us to verify that a particular service was not interacted with at all.

slackersoft commented 6 years ago

I like this idea, but I'm not sure on the right way to enable it in Jasmine. Right now, the object returned by createSpyObj is just a plain old javascript object with each of the methods setup as a spy.

The simplest thing to do would just be a matcher that loops through all properties on the actual object and checks the call counts for any that are spies. This might have some unexpected behavior if you pass an object without any spies, or where only some of the methods/properties are spied on.

We would be happy to review a pull request to add a toHaveNoSpyInteractions matcher that does this.

Thanks for using Jasmine!

nitobuendia commented 6 years ago

How about having something like .calls. but for all .spies. when created with createSpyObj?

I have not checked the code, but I would imagine it is possible that when a spy of a spyObj gets called it would not only update its own counter, but also the spies properties in the same fashion but aggregated for all methods.

This would allow things such as:

expect(mySpyObj.spies).not.toHaveBeenCalled();
expect(mySpyOb.spies.calls.count()).toEqual(5);
nitobuendia commented 6 years ago

I have created #1606 with the previous proposal. It's still a draft, but some discussion are needed around:

slackersoft commented 5 years ago

I think we should stay away from adding an additional property to the returned object with spies on it. This would also potentially reduce the use of a new matcher to only work correctly when used in conjunction with createSpyObj. Given that right now we're just looking for a matcher that verifies no interactions, I would still prefer to see a new matcher (something like toHaveNoSpyInteractions) that looks at all of the properties of the actual and checks the call count for any that are spies.

Hope this helps.

nitobuendia commented 2 years ago

I've started working on this one again. I will be following the recommendation from @slackersoft, but making it a positive statement (toHaveSpyInteractions) as opposed to a negative statement (toHaveNoSpyInteractions). This would be more natural, avoid confusion with double negatives and make it consistent with the toHaveBeenCalled matchers.

At the moment, I got this working as a custom matcher with basic testing (code below). Nonetheless, I am working on integrating this into jasmine core matches with all respective tests. Pull request hopefully coming soon.

const toHaveSpyInteractionsMatcher = function (j$) {
  var getErrorMsg = j$.formatErrorMsg(
    '<toHaveSpyInteractions>',
    'expect(<spyObj>).toHaveSpyInteractions()'
  );

  /**
   * {@link expect} the actual (a {@link SpyObj}) spies to have been called.
   * @function
   * @name matchers#toHaveSpyInteractions
   * @since 4.0.0
   * @example
   * expect(mySpyObj).toHaveSpyInteractions();
   * expect(mySpyObj).not.toHaveSpyInteractions();
   */
  function toHaveSpyInteractions(matchersUtil) {
    return {
      compare: function (actual) {
        var result = {};

        if (!j$.isObject_(actual)) {
          throw new Error(getErrorMsg(
            'Expected a spy object, but got ' + matchersUtil.pp(actual) + '.'
          ));
        }

        if (arguments.length > 1) {
          throw new Error(
            getErrorMsg('Does not take arguments')
          );
        }

        result.pass = false;
        let hasSpy = false;
        const calledSpies = [];
        for (const spy of Object.values(actual)) {
          if (!j$.isSpy(spy)) continue;
          hasSpy = true;

          if (spy.calls.any()) {
            result.pass = true;
            calledSpies.push([spy.and.identity, spy.calls.count()]);
          }
        }

        if (!hasSpy) {
          throw new Error(getErrorMsg(
            'Expected a spy object with spies, but object has no spies.'
          ));
        }

        let resultMessage = 'Expected spy object spies to have been called';
        if (result.pass) {
          resultMessage += ', the following spies were called: ';
          resultMessage += calledSpies.map(([spyName, spyCount]) => {
            return `${spyName} called ${spyCount} time(s)`;
          }).join(', ');
        } else {
          resultMessage += ', but no spies were called.'
        }
        result.message = resultMessage;

        return result;
      }
    };
  }

  return toHaveSpyInteractions;
};

describe('toHaveSpyInteractions', () => {
  const toHaveSpyInteractions = toHaveSpyInteractionsMatcher(jasmine);

  let spyObj;
  beforeEach(() => {
    jasmine.addMatchers({ toHaveSpyInteractions });
    spyObj = jasmine.createSpyObj('NewClass', ['spyA', 'spyB']);
    spyObj.otherMethod = function () { };
  });

  it('detects spy interactions', () => {
    spyObj.spyA();
    expect(spyObj).toHaveSpyInteractions();
  });

  it('detects multiple spy interactions', () => {
    spyObj.spyA();
    spyObj.spyB();
    spyObj.spyA();
    expect(spyObj).toHaveSpyInteractions();
  });

  it('detects no spy interactions', () => {
    expect(spyObj).not.toHaveSpyInteractions();
  });

  it('ignores non-observed spy object interactions', () => {
    spyObj.otherMethod();
    expect(spyObj).not.toHaveSpyInteractions();
  });

  [true, 123, 'string'].forEach((testValue) => {
    it(`throws error if a non-object (${testValue}) is passed`, () => {
      expect(() => {
        expect(true).toHaveSpyInteractions();
      }).toThrowError(Error, /Expected a spy object, but got/);
    });
  });

  [['argument'], [false, 0]].forEach((testValue) => {
    it(`throws error if arguments (${testValue}) are passed`, () => {
      expect(() => {
        expect(spyObj).toHaveSpyInteractions(...testValue);
      }).toThrowError(Error, /Does not take arguments/);
    });
  });

  it('throws error if spy object has no spies', () => {
    const newSpyObj = jasmine.createSpyObj('OtherClass', ['method']);
    // Removing spy since spy objects cannot be created without spies.
    newSpyObj.method = function () { };

    expect(() => {
      expect(newSpyObj).toHaveSpyInteractions();
    }).toThrowError(
      Error, /Expected a spy object with spies, but object has no spies/);
  });

});