Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

return handler in reply(...) mocks in order to allow removing handler and monitoring calls #159

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

burgalon
Copy link

Similarly to WebMock it would be nice if mocking replies will return the handler so that later we can:

  1. Assert that mock has been called
  2. Remove handler if necessary

@ctimmerm
Copy link
Owner

Thanks for your pull request!
I agree that this would be a useful feature, but if the handler is returned from reply it would mean quite a large breaking change is introduced (any existing code that uses chaining would no longer work). Maybe it could instead be done in a way that is backwards compatible by adding an extra method? Something like:

var handler = mock.onGet(...).reply(...).handler();

@burgalon
Copy link
Author

Hey Colin,
The suggestion .handler() doesn't seem quite right: That would mean that we we would need to save the last handler in _this and then .handler() would return it?
More generally, I think it would be nice if we take axios-mock-adapter further in the direction of not jus mocking, but also allowing matchers and assertions for requests which have occured like WebMock. Thus I think letting go of chaining, is for a good cause :)

@ctimmerm
Copy link
Owner

What if we could have the best of both worlds and instead of returning either the instance or the handler, return a proxy that proxies the methods/properties to either the instance or the handler.

A contrived example:

function createProxy(instance, handler) {
  var proxy = {
    handler: handler,
    instance: instance,

    get: function() {
      return instance.get.apply(instance, arguments);
    }
  };

  Object.defineProperty(proxy, 'called', {
    get: function() {
      return handler.called;
    },

    set: function(value) {
      handler.called = value;
    }
  });

  return proxy;
}

@burgalon
Copy link
Author

personally I like when things are straight forward, and the object type at hand is of clear type and interface. The suggested proxy makes it harder to follow the code and understand

@burgalon
Copy link
Author

we could have a flag at the mock level determining what's the returned object... but that will create communities forking usage instead of forked code :)

@burgalon burgalon force-pushed the return-handler-and-called branch 2 times, most recently from 1cc8672 to 13c43f9 Compare April 28, 2019 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants