Reintroduce / undeprecate method overriding

[movitto]

Feature Description

Thank you for the great library. Previous versions facilitated overriding methods via the methods parameter to mount and/or the setMethods call. Please consider reintroducing this feature for the reasons below.

Problem

Lets say I have a vue component that uses the vue-resource library like so:

{
  name : 'DemoComponent',

  data : function(){
     return {foobar : ''}
  },

  methods : {
     get_data : function(){
        this.$http.get("https://foo.bar")
                       .then(function(response){
                           this.foobar = response.body;
                        }.bind(this))
     }
  },

  created : function(){
     this.get_data()
  }
}

Best practices is to stub out network / http calls in test suites. So as not to couple the test suite to implementation details, previously one could override the get_data method like so:

mount(DemoComponent, {
  methods : {
    get_data : function(){
       this.foobar = '{"foo" : "bar"}'
    }
  }
})

Everyone is happy. The test suite doesn't invoke a network request, nor is it coupled to the implementation details of how the data is retrieved.

The problem is w/ version 1.x of this library the methods parameter to mount was deprecated and it was removed all together in version 2.x.

We are requesting that this functionality is readded to this library for the above reason and so that the workaround described below is not needed.

Expected behaviour

The mount method should accept and use the methods param and the setMethods call should be added to the Wrapper class.

Alternatives

One could use a mixin to override the method:

const override = {
  methods : {
    get_data : function(){
       this.foobar = {foo : 'bar'}
    }
  }
}

mount(DemoComponent, {mixins : [override]})

This achieves the same effect, but is a roundabout way to achieve the result.

Of course in the above example, we could stub/override the $http.get method, but this best practices is to test the interface and expected results and not the implementation

---

Thank you for considering this feature and thank you again for the great library!

[afontcu]

Hi!

It is highly unlikely that we introduce methods or setMethods back for reasons exposed before (link, link, link).

Regarding you example, you could do something like the following:

test('...', () => {
  mount(DemoComponent, {
    mocks: {
      $http: jest.fn().mockImplementation(() => {
        // implement at will
      })
    }
  })
})

hope it helps!

[movitto]

@afontcu thank you for the response. Unfortunately the justification for the removal of setMethod provided in those links do not apply to the scenario presented above, and I suspect it doesn't apply for others (as evident in the comments on those github issues).

The anti-pattern states that to resolve this one should "extract those hard parts away". We infact do this, the example provided above was a simplfied one, our usage looks more like this:

// http_mixin.js:
{
  methods: {
    get_data : function() {
       // http request
    }
  }
}

// Component1:
import HttpMixin from './http_mixin'
{
   name : 'Component1',
   mixins : [HttpMixin]
}

// Component2
import HttpMixin from './http_mixin'
{
   name : 'Component2',
   mixins : [HttpMixin]
}

If we stub $http as you suggested in the Component1 and Component2 tests, we would be stubbing an implementation detail of the HttpMixin as opposed to a common element of the interface. In general the later (being, in the example above, to stub get_data) would be preferable as the implementation details of how get_data could change in the future, without the tests being tightly coupled to that.

Please also consider the following scenario:

{
  name : 'AnotherComponent',
  methods : {
    get_some_data : function(){
       this.$http.get("https://some.data")
                 .then(function(response){
                   // handle response
                 })
    },

    get_other_data : function(){
       this.$http.get("https://other.data")
                 .then(function(response){
                   // handle response
                 })
    }
  },

  created : function(){
     this.get_some_data();
     this.get_other_data();
  }
}

Mocking $http would be more complicated here as we would have to handle both cases in the same same instance:

  this.$http.get = jest.fn.mockImplementationOnce(() => 'some.data')
                          .mockImplementationOnce(() => 'other.data')

As opposed to being able to override get_some_data and get_other_data separately which is a cleaner separation of concerns.

---

Again thank you for considering this pull request. I understand that overriding methods directly isn't ideal from a vuejs perspective, but since the library allows you to do that anyways via mixins, it's not that far fetched.

[afontcu]

Hi!

Unfortunately the justification for the removal of setMethod provided in those links do not apply to the scenario presented above

As mentioned in links above (and in docs, now!) it is hard to define a clear migration path away from overriding methods. This is why we only suggest some guidelines (moving the "code under testing" to its own module being one). It might not apply to your scenario, that's for sure. However, if you're already using a mixin then, as you already mentioned, you could simply override that mixin. This is not the same as patching an internal method – Vue 2 offers mixins as a way to compose functionality in your components (not the most elegant one, but that's beyond the scope!), but it doesn't let you patch/override internal methods. Thus, here VTU is aligning to what Vue has to offer and it's feature-extending mechanisms.

Another alternative is to use tools such as msw that let you stub network requests at the lowest possible level.

[movitto]

OK thank you for the response, in lieu of the method override, we'll continue w/ the mixin override as mentioned.

[afontcu]

Thank you, and thank you for sharing your thoughts! :)

[dumptyd]

If anyone comes across this looking for a way to override methods for reasons in vue-test-utils-next, it can be done like this:

import MyComponent from 'path/to/my-component.vue';

it('test', () => {
  const MyComponentLocal = cloneDeep(MyComponent); // or some other deep copy utility
 const mockMethod = jest.fn();
  MyComponentLocal.methods.methodToTest = mockMethod;
  const wrapper = mount(MyComponentLocal);
  // do things that would result in a call
  expect(mockMethod).toHaveBeenCalledTimes(1);
});