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

Removing sync mode #1137

Closed
eddyerburgh opened this issue Feb 8, 2019 · 31 comments
Closed

Removing sync mode #1137

eddyerburgh opened this issue Feb 8, 2019 · 31 comments

Comments

@eddyerburgh
Copy link
Member

eddyerburgh commented Feb 8, 2019

After searching for an alternative solution, we've decided to remove synchronous updating (sync mode) from Vue Test Utils.

tl;dr

We will remove sync mode in the next beta version. Test code will change from this:

it('render text', (done) => {
    const wrapper = mount(TestComponent)
    wrapper.trigger('click')
    wrapper.text().toContain('some text')
})

To this:

it('render text', async () => {
    const wrapper = mount(TestComponent)
    wrapper.trigger('click')
    await Vue.nextTick()
    wrapper.text().toContain('some text')
})

Background

By default, Vue batches updates to run asynchronously (on the next "tick"). This is to prevent unnecessary DOM re-renders, and watcher computations (see the docs for more details).

When we started Vue Test Utils, we decided to make updates run synchronously. The reasoning was that synchronous tests are easier to write, and have improved error handling.

For context, the decision was made just after async/await had been released in node 7.6, so asynchronous tests often looked like this:

it('render text', (done) => {
    const wrapper = mount(TestComponent)
    wrapper.trigger('click')
    Vue.nextTick(() => {
        wrapper.text().toContain('some text')
        wrapper.trigger('click')
        Vue.nextTick(() => {
            wrapper.text().toContain('some different text')
            done()
        })
    })
})

Because of this decision Vue Test Utils runs updates synchronously by default. That was a mistake.

Why?

Sync mode causes bugs that don't exist when Vue runs normally. This is frustrating for users, and bad functionality for a testing framework that's intended to give you confidence that your code will work in production.

We went through three different approaches to implement sync mode. Each had problems. The final attempt was to reimplement synchronous updates in Vue core, but there were still bugs caused by synchronous updates.

Solution

The solution is to remove sync mode entirely from Vue Test Utils and rely on the user explicitly waiting for updates to be applied:

it('render text', async () => {
    const wrapper = mount(TestComponent)
    
    wrapper.trigger('click')
    await Vue.nextTick()
    
    wrapper.text().toContain('some text')
    
    wrapper.trigger('click')
    await Vue.nextTick()
    wrapper.text().toContain('some different text')
})

This will be a big change for many test suites. You have two choices:

  1. Refactor tests to run asynchrnously
  2. Keep Vue Test Utils locked at beta.28

We recommend that you update your tests in order to benefit from future features of Vue Test Utils. This will also make your tests more robust since Vue will perform updates in the same way as it does in production.

To make the migration as smooth as possible, so we'll provide documentation and guides to help you write tests asynchronously with Vue.nextTick.

Finally, I'm sorry for the work this change will require. I was a large part of the driving force for running in sync mode by default, and I underestimated the problems that it would cause.

Going forward, this will improve the stability of Vue Test Utils, and we'll be able to release a stable API as v1.

@dominik-bln
Copy link

dominik-bln commented Feb 10, 2019

Thanks for the explanation and better now than after v1.

Just as a thought, wouldn't recommending something like this be better to make it also work when localVue is used:

await wrapper.vm.$nextTick()

Or is that irrelevant?

@eddyerburgh
Copy link
Member Author

You can use either, they're the same function

@ghost
Copy link

ghost commented Feb 11, 2019

As much as I liked it, thank you for sharing the detailed explanation and changing it in the beta rather than causing more refactoring work later. Also thank you for creating an issue prior to changing it! ❤️

@sagalbot
Copy link
Contributor

sagalbot commented Feb 14, 2019

@eddyerburgh can you please adjust the release https://github.com/vuejs/vue-test-utils/releases/tag/v1.0.0-beta.29 and place this under breaking changes? There's BREAKING CHANGES but it's not listed under there and I didn't notice the text at the top.

@brennj
Copy link

brennj commented Feb 17, 2019

Hey @eddyerburgh! I could be talking complete nonsense, but would it be possible to await that next tick in the wrapper.trigger to keep the test code cleaner?

i.e. instead of

wrapper.trigger('click');
await Vue.nextTick();

do

await wrapper.trigger('click');

@eddyerburgh
Copy link
Member Author

Yes that's a good idea, I was thinking the same thing 👍

@ghost
Copy link

ghost commented Feb 18, 2019

can you please adjust the release https://github.com/vuejs/vue-test-utils/releases/tag/v1.0.0-beta.29 and place this under breaking changes?

@sagalbot @eddyerburgh I don't think this was included in v1.0.0-beta.29: v1.0.0-beta.29...dev

#1141 was merged on 9th of February while v1.0.0-beta.29 was already tagged on the 2nd.

@markrian
Copy link
Contributor

await wrapper.trigger('click');

I feel like that's conflating two things: triggering the event, and flushing to the DOM. What if a tester wanted to do more than just trigger a click event before the next tick?

I'd suggest instead either sticking with the original trigger/await nextTick pattern, or adding a new method, something like:

await wrapper.triggerAndNextTick('click');

... but with a less terrible name!

@brennj
Copy link

brennj commented Feb 21, 2019

Fair point. I would imagine there could be some interesting points of inspiration to take from the async utilities/patterns of dom-testing-library that is popular with the React flavor of react-testing-library. https://testing-library.com/docs/api-async

@satyavh
Copy link

satyavh commented May 2, 2019

What a change; this also broke wrapper.setProps({})
So before:

wrapper.setProps({
   prop: someValue,
});
expect(wrapper.vm.prop).toEqual(someValue);

would trigger changes (dom, watchers, computed, etc) in the component, but now you have to do

wrapper.setProps({
   prop: someValue,
});

await wrapper.vm.$nextTick();
expect(wrapper.vm.prop).toEqual(someValue);

Imo this change should be reflected in the docs asap!

@garyo
Copy link
Contributor

garyo commented May 10, 2019

Is this change in 1.0.0-beta.29? I'm fighting #1163 and trying to figure out the best way forward.

@ghost
Copy link

ghost commented May 10, 2019

Is this change in 1.0.0-beta.29?

@garyo No, it is not—see also my comment above: #1137 (comment)

@dietergeerts
Copy link

What a mess, release notes are telling lies then.... I had to update the test-utils, and now I'm getting all sorts of errors on tests that where succeeding before. There is something really wrong with the DOM updates when ran with test-utils. Are there other things I can check to resolve these issues?

@ghost
Copy link

ghost commented Jun 12, 2019

What a mess, release notes are telling lies then....

@dietergeerts I don't see this mentioned in the release notes. 🤔

Are there other things I can check to resolve these issues?

It probably helps to create a new issue with reproduction steps and the error message.

@dietergeerts
Copy link

dietergeerts commented Jun 12, 2019

https://github.com/vuejs/vue-test-utils/releases/tag/v1.0.0-beta.29

Use Vue async mode, this has caused bugs and will be reverted (see #1137)

It's very confusing

@ghost
Copy link

ghost commented Jun 12, 2019

@eddyerburgh I agree with @dietergeerts that this entry in the release notes could have some clarification. It reads as if Use Vue async mode is handled by this issue while it is actually #1062.

@dietergeerts
Copy link

dietergeerts commented Jun 12, 2019

Just to keep you guys up-to-date: When reverting to beta.28, all my problems are resolved.
The issue I had was that certain test cases where failing because other test cases where included in the run, for no apparent reason. it's very strange. We always use createLocalVue to have each test really isolated.

By using the test utils, are there any connection on global level?

@pipopotamasu pipopotamasu mentioned this issue Jun 28, 2019
13 tasks
sharkykh added a commit to pymedusa/Medusa that referenced this issue Jul 5, 2019
p0psicles pushed a commit to pymedusa/Medusa that referenced this issue Jul 5, 2019
* Remove waitFor from QualityChooser

Use `v-if`s instead

* [bundle]

* Convert QualityChooser test to async test

vuejs/vue-test-utils#1137
service-paradis added a commit to service-paradis/buefy that referenced this issue Feb 5, 2020
- Fixed a bug (vuejs/vue-test-utils#1137)
- Remove unnecessary warning when running unit tests
jtommy pushed a commit to buefy/buefy that referenced this issue Feb 5, 2020
- Fixed a bug (vuejs/vue-test-utils#1137)
- Remove unnecessary warning when running unit tests
@kierans
Copy link

kierans commented Feb 12, 2020

Is there a way to pin docs online at a certain version?

The reason I ask is that when using Vuetify, tests can blow up when running in sync mode usually with an error like TypeError: Cannot read property '$scopedSlots' of undefined. This is because when testing form validation DOM nodes are inserted/removed depending on the validation outcome. When testing synchronously some variable is undefined, whereas by waiting for the nextTick the DOM batch is processed and all is well. For those having the same issue with Vuetify see vuetifyjs/vuetify#9151 or vuetifyjs/vuetify#9820.

However when reading through the docs of @vue/test-utils I couldn't find any reference to sync yet my repo was still at beta.29. This was confusing for me when different forums posts on how to fix my tests from blowing up was to set a mounting option that didn't exist according to the docs. It took more googling to find this issue.

I support the idea of removing sync updates because it goes against the grain of Vue, and Node JS in general, so I'm glad that it's gone. However I think it would be helpful for people to be able to view the docs online for the version of the library they're using. Especially for breaking changes like this.

@dobromir-hristov
Copy link
Contributor

The only way I see is to point to that version on Netlify.

shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this issue Mar 6, 2020
I upgraded @vue/test-utils to version `v1.0.0-beta.31`.

We were previously on `v1.0.0-beta25`, here's the changelog:

 - https://github.com/vuejs/vue-test-utils/releases

This upgrade broke a bunch of tests; our tests had to refactored
to run asynchronously, here are some links regarding that:

 - vuejs/vue-test-utils#1137
 - https://vue-test-utils.vuejs.org/guides/#writing-asynchronous-tests-using-nexttick-new

Refs: #2380
shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this issue Mar 6, 2020
I upgraded @vue/test-utils to the latest version, `v1.0.0-beta.31`.

We were previously on `v1.0.0-beta25`, here's the changelog:
 - https://github.com/vuejs/vue-test-utils/releases

This upgrade broke a bunch of tests; our tests had to refactored
to run asynchronously, here are some links regarding that:
 - vuejs/vue-test-utils#1137
 - https://vue-test-utils.vuejs.org/guides/#writing-asynchronous-tests-using-nexttick-new

Refs: #2380
shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this issue Mar 6, 2020
I upgraded @vue/test-utils to the latest version, `v1.0.0-beta.31`.

We were previously on `v1.0.0-beta25`, here's the changelog:
 - https://github.com/vuejs/vue-test-utils/releases

This upgrade broke a bunch of tests; our tests had to refactored
to run asynchronously, here are some links regarding that:
 - vuejs/vue-test-utils#1137
 - https://vue-test-utils.vuejs.org/guides/#writing-asynchronous-tests-using-nexttick-new

Refs: #2380
shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this issue Mar 9, 2020
* Update @vue/test-utils

I upgraded @vue/test-utils to the latest version, `v1.0.0-beta.31`.

We were previously on `v1.0.0-beta25`, here's the changelog:
 - https://github.com/vuejs/vue-test-utils/releases

This upgrade broke a bunch of tests; our tests had to refactored
to run asynchronously, here are some links regarding that:
 - vuejs/vue-test-utils#1137
 - https://vue-test-utils.vuejs.org/guides/#writing-asynchronous-tests-using-nexttick-new

Refs: #2380

* Remove package-lock.json

Using both `package-lock.json` and `yarn-lock.json` can cause confusion
and lead to potential issues, so we'll just use one, `yarn-lock.json`.
:smile:

Refs: #2380
tiltec pushed a commit to karrot-dev/karrot-frontend that referenced this issue May 9, 2020
* Use address autocomplete

As it used to be.
Also defaults place edit map to be group centre if available.

* Remove uneeded extra wrapped div

* Update setup for storyshots testing

"sync: false" seems to be the new way it goes and will be the default
in the future, see vuejs/vue-test-utils#1137

The transition group stubs seem to be there by default now too, I think..
Anyway it works where it did no tbefore :)

Also see vuejs/vue-test-utils#1163

* Add @types/jest - for editor support

See https://blog.jetbrains.com/webstorm/2018/10/testing-with-jest-in-webstorm/

* Increase test wait

Not really sure why this changed, but don't want to think about
it for now...

* Add text to translation file

* Use the spelling of the enemy

* Simplify template logic
tiltec pushed a commit to karrot-dev/karrot-frontend that referenced this issue May 9, 2020
* Initial work to add person-to-person sharing

* A few sharing item UI refinements

* Initial new sharing form with multiple upload

* Add a grey box...

* Remove redundant check

* Rename sharing to offers

* Add a mock chat to offer detail UI

* Create mobile version of offer detail UI

* Tidy up multiple image editing

* A bit more polish on the offer UI

Some translations, icon, breadcrumb, etc...

* Snapshot update

* Add create offer breadcrumb title

* Rename a few offer related things for consistency

* Offer image carousel, multi upload, various bits

* Refine image upload/remove/re-position

* Add websocket listeners for offer changes

* Wireup offer conversation

* Basic features to allow to accept/archive offers

* Remove uneeded demo data

* Move selected offer data/logic to own store module

* Preserve offer filter in more places

The create/save filter preservation does not work actually
as we clear the store contents on route change.

* Remove status from offer form

Status change is now done via explicit actions/buttons

* Use address autocomplete (#1878)

* Use address autocomplete

As it used to be.
Also defaults place edit map to be group centre if available.

* Remove uneeded extra wrapped div

* Update setup for storyshots testing

"sync: false" seems to be the new way it goes and will be the default
in the future, see vuejs/vue-test-utils#1137

The transition group stubs seem to be there by default now too, I think..
Anyway it works where it did no tbefore :)

Also see vuejs/vue-test-utils#1163

* Add @types/jest - for editor support

See https://blog.jetbrains.com/webstorm/2018/10/testing-with-jest-in-webstorm/

* Increase test wait

Not really sure why this changed, but don't want to think about
it for now...

* Add text to translation file

* Use the spelling of the enemy

* Simplify template logic

* Ffix multicroppa

* Further refine offer editing, esp MultiCroppa

* Use gift as offers menu item

* Remove offer upload progress logging

* Nicer create offer button

* Clear offerDetail on route leave

* Add offer list loading indicator

* Add offer body loading indicator

* Add loading spinner when fetching offer for edit

* Correctly wire up offer form reset events

* Only show offer accept/archive buttons if active

* Handle updating offers after save/websocket better

* Cleaner mobile form view

* Only show carousel if >1 image

* Fix offer header when name is very long

* Fix offer detail image display when only 1

* Fix overflowing text on group offers page

* White background offer description

* Snapshot update

* Don't try and be fancy with updating users

* Fix style style

* Hide offers if group does not have the feature

* Check route features in after route hook

* Actually filter offers by current group

* Add offers covert function to convert createdAt

* Display offer information in latest messages

I did it a slightly different way, by adding related offers
into the latestMessages state...

* Keep related offers up to date via websocket

It's a bit overkeen actually as most users won't be in the
conversations for the offers, so we don't need to update the
related item... should really check we need it first

* Import default offer status from store

* Use current route query instead of getter

* Use + for new offer button

* Allow setting new_offer notification types

* Show offer status if not active

Although it's a bit ugly

* Change notification type spinner

* Add group.features to test mocks

* Wrap settings forms in FormContainer

* Rename to KFormContainer and move to base

* Add confirm step to accept/archive offer buttons

* Use photo icon for offer image placeholder

* Add offer as a "fake" empty result

* Filter offers by status and sort by createdAt desc

... entries can get added via websockets that might not fit
the filter, and might need reordering

* Refine group offer cards

- use QImg instead of lots of CSS
- use QItem inside for nicer layout

* Correct RouterLink casing

* Update test snapshots

* Please the pedantic linter

* Remove offer description helper

* Extract mapErrors to statusMixin

* Add accept/archive text into i18n

* Remove unneeded code

* Add offer filter status text to i18n

* Correct size for new offer button

* Remove one layer of div soup

* Put the group offer cards in proper links

* Remove commented out code

* Convert offer in websocket update

* Prefix functional getter with get

For getNotificationTypeStatus

* Preserve route query when clicking on offer conv

* Use icon service for latest messages offer item

* Move withoutKeys to utils

* Use invisible class instead of custom css

* Don't put const value in data

* Save disk space by compacting template

* Remove unneeded v-model

Was only used when I tried out quasar :rules

* Document MultiCroppa value prop a bit more

* Document the data that latestMesages/related has
jeanremy added a commit to jeanremy/vee-validate that referenced this issue Jul 22, 2020
Removed sync mode from vueTestUtils, since (it has been removed)[vuejs/vue-test-utils#1137].
Also add the import of flushPromises(). It took quite some time to find out that it was an external package.
logaretm pushed a commit to logaretm/vee-validate that referenced this issue Jul 23, 2020
Removed sync mode from vueTestUtils, since (it has been removed)[vuejs/vue-test-utils#1137].
Also add the import of flushPromises(). It took quite some time to find out that it was an external package.
da70 added a commit to NYULibraries/dlts-open-square-search-application that referenced this issue Jun 11, 2021
sandbergja added a commit to pulibrary/lux that referenced this issue Jul 19, 2023
Prior to the 1.0.0 release, @vue/test-utils made a few breaking changes:

* The "sync mode" was removed in vuejs/vue-test-utils#1137,
meaning that we have to explicitly call nextTick() before checking if something has
changed in the DOM
* transition is no longer automatically stubbed, we have to create our own transitionStub.
sandbergja added a commit to pulibrary/lux that referenced this issue Jul 19, 2023
Prior to the 1.0.0 release, @vue/test-utils made a few breaking changes:

* The "sync mode" was removed in vuejs/vue-test-utils#1137,
meaning that we have to explicitly call nextTick() before checking if something has
changed in the DOM
* transition is no longer automatically stubbed, we have to create our own transitionStub.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests