fix #1541: Improve deprecation messages

[afontcu]

What kind of change does this PR introduce? (check at least one)

• Bugfix
• Feature
• Code style update
• Refactor (I guess? xD)
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change? (check one)

• Yes
• No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

• It's submitted to the dev branch.
• When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
• All tests are passing: https://github.com/vuejs/vue-test-utils/blob/dev/.github/CONTRIBUTING.md#development-setup
• New/updated tests are included

If adding a new feature, the PR's description includes:

• A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

I'm open to suggestions with regard the specifics of the deprecation message – not having a clear migration path for setMethods and options.methods isn't ideal, but I don't think we will ever find one.

Closes #1541

[lmiller1990]

excellent, thanks. will see if anyone else has any comments, otherwise merge and do a release in the next day or two.

[JessicaSachs]

I added some rewording for active voice, two-three minor typos, and conciseness. Feel free to take them or leave them.

[dcb99]

Improved deprecation messages is nice, but you're still deprecating a feature that we have implemented in hundreds of tests. If I want to mock a function, and then assert that a button click called that function, it used to be very straightforward, and is extremely useful for verifying the code (and the UI) behaves as expected. Tests are supposed to prove that your code does what it says it does. Now we get hundreds of deprecation warnings.
I get that we don't want tests to rely on implementation details. But the blanket suggest to "...rethink those tests" is both short-sighted and condescending. I don't think all the use cases were properly considered here, just extracting a function and testing it doesn't solve the base test case I mentioned, which is that a button click calls the right function (which we can then use to make sure other behaviors happened as expected), or that a particular function was called n times for n clicks.
If you're going to deprecate something and not provide anything other than a nebulous deprecation warning, you're not doing developers any favors. Mocking/stubbing functions isn't an antipattern, it's a pretty important part of verifying inputs and outputs.
Yes, it CAN make tests brittle, yes, it CAN be misused, but removing the ability to be productive for a use case that seems pretty common is more of an antipattern that mocking functions. Better deprecation warnings are not a solution to not being able to easily and succinctly prove that your code does what it says it does.

[afontcu]

Now we get hundreds of deprecation warnings

You can easily fix that by setting config.showDeprecationWarnings = false (source)

If I want to mock a function, and then assert that a button click called that function, it used to be very straightforward, and is extremely useful for verifying the code (and the UI) behaves as expected

I'm not sure I fully agree with this statement. Clicking a button is not the important part of a component. The consequences of that click are – HTTP requests, $emit, creating a Cookie, or whatever. And that's something totally doable without setMethods.

Mocking/stubbing functions isn't an antipattern, it's a pretty important part of verifying inputs and outputs

We haven't said a word against mocking/stubbing – we actually suggest in docs to extract any complex method and spy on it.

But the blanket suggest to "...rethink those tests" is both short-sighted and condescending

I'm sorry we made you feel this way. Truth is, however, that there's no clear migration path. That being said, we're totally open to discuss further improvements on deprecation messages 👍

[dobromir-hristov]

Because I can feel this will get hairy, as people wont change their mindsets, especially as we just wont support this in Vue 3.

If you really really want to override your methods/computeds/what have you, you can do so just before you mount. The component is just an object any way.

const merged = merge(Component, { methods: { myMethod: jest.fn() } })
mount(merged, options)

[MilenaMalysh]

Even though you say that you understand importance of mocking and stubbing in the tests, I don't think you actually do. Without providing a clear way to do it you make vue-test-utils only applicable for small projects without much of frontend business logic. And in reality those projects don't require unit tests. It's really hard to screw up the "click on the button -> send an API request" functionality. You provide a lot of examples on how to test this kind of flow, but how to deal with a bunch of complex, tangled methods calling each other? Tests are really needed in the large and complex projects. These projects are usually hard to mantain and almost impossible to refactor. And the way they are structured strongly depends on the domain logic. So "to extract complex methods from the component and test them in isolation" isn't only hard to do but also absolutely unreasonable. And what if all of the methods in a component are complex? Extract all of them?

If we agree that to refactor the code in order to make the tests work isn't a good idea, let me provide an example of when stubbing can be absolutely required. Let's say we have a image file input field, a button and a file validation function which is called when user clicks on the button. The validation function checks million different things like the size of the files array, files extensions and so on. If everything is ok it calls a function that loads the image file and show it to the user. If we won't be allowed to stub the file loading method in order to test every possible validation case we would have to call the file loading method over and over again. File loading takes some time and if we have thousands of validation cases we would absolutely unnecessary increase tests running time greatly. Especially considering that we don't really need to test js FileLoader...

Vue-test-utils claims to be a unit testing library. And even though I understand that you consider a component as a unit in reality sometimes we have to think about separate methods. The topic is quite controversial but if you could continue supporting the 'setMethods' function and the 'methods' mount option it would keep developers from re-writting their whole unit-tests code base.

[lmiller1990]

VTU is utilities for testing Vue components. You can use it write unit tests, integration tests, whatever you like.

The validation function checks million different things like the size of the files array, files extensions
increase tests running time greatly

If you have 1 million different scenarios, you now need 1 million tests, each rendering a Vue component. This will be slow. Wouldn't it make a lot more sense to put that into a function, then test that function 1 million times?

Eg:

function validateFile(...lotsOfComplexArgs) {
  // if statements
  // API calls
  // whatever else
}

Then in your component, you would just call validateFile(manyArguments). It will be much faster, too, since you don't have the overhead of a DOM, Vue, etc.

This would require a refactor. If this refactor is too significant and does't make much sense from a business point of view, just don't do it - continue using setMethods. It won't be removed from V1 (Vue 2 support). Alternatively, if you don't want to use a deprecated API, you can use the user-land implementation posted above.

The deprecation warning is there because the method will not be in VTU v2 (which is for Vue 3). For those interested, it's found here. It is a rewrite since the Vue internals changed significantly for Vue 3.

---

For some context, I believe I do understand the scenario you are describing. Many of my clients have this problem - one gigantic Component.vue, written by some other developer they consulted to write their Vue app, whom did not write any tests. The client then realizes they have a huge, unmaintainable mess and reach out.

They want to write some tests before they continue development, since that's the only way they can do so without fear of breaking their product - makes perfect sense. Using an elaborate mount method with methods, computed etc is not the way to go, though. Rather than add more complexity, it's far better to just simplify the problem by extracting your complexity into simple functions, and testing those individually.

This is still just my opinion, derived from my experience writing, testing and maintaining Vue.js apps. If you are not satisfied with this recommendation, I have outlined a few alternatives above (continue using setMethods, make use of the user-land work-around).