posva / vuex-mock-store

✅Simple and straightforward Vuex Store mock for vue-test-utils
MIT License
272 stars 19 forks source link

chore(deps): update dependency vue to v3, @vue/test-utils to v2, and vuex to v4 #203

Closed winniehell closed 1 year ago

winniehell commented 1 year ago

supersedes https://github.com/posva/vuex-mock-store/pull/194, https://github.com/posva/vuex-mock-store/pull/197, and https://github.com/posva/vuex-mock-store/pull/201

winniehell commented 1 year ago

@posva can you please review?

codecov[bot] commented 1 year ago

Codecov Report

Merging #203 (61842cc) into master (c394b0d) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 61842cc differs from pull request most recent head b986036. Consider uploading reports for the commit b986036 to get more accurate results

@@            Coverage Diff            @@
##            master      #203   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           54        54           
  Branches         5         5           
=========================================
  Hits            54        54           
posva commented 1 year ago

I will but please don’t ping me each time you open a pull request because I already have the notification without the ping 😉

winniehell commented 1 year ago

sure, sorry about that :+1:

that is a habit I have developed at GitLab where merge requests or issues are generally not looked at if you don't ping people :shrug:

winniehell commented 1 year ago
- Coverage   100.00%   98.14%   -1.86%     

this probably deserves to fixed prior to merging

winniehell commented 1 year ago
- Coverage   100.00%   98.14%   -1.86%     

this probably deserves to fixed prior to merging

it looks like the corresponding code, namely https://github.com/posva/vuex-mock-store/blob/d7fc7b32154f35c73dad6bd8ba9e38297bed8359/src/index.js#L77-L84, was only tested accidentally and not explicitly on master. I am not sure how to test it because I can't find a use case for either of the conditions.

posva commented 1 year ago

This would require to migrate vuex as well but it's impontant to keep the version working with Vue 2 so it's not something I can just merge. I will take a look when I have time

winniehell commented 1 year ago

I have cherry-picked the Vuex update and added a GitHub Actions workflow for the Vue 2 compatibilty.

winniehell commented 1 year ago

I will take a look when I have time

@posva do you have a rough estimate when that will be?

I'm considering to release a temporary version from my fork to unblock https://gitlab.com/gitlab-org/gitlab/-/merge_requests/117107#note_1362089457 (which itself blocks a bugfix)

posva commented 1 year ago

Are you telling me the plugin already working with Vuex 4 and Vue 3? 😅 That's quite the surprise

winniehell commented 1 year ago

yes, looks like it does—and it was a surprise for me, too. :smiley: I guess you did a great job on implementing this. :muscle:

winniehell commented 1 year ago

@posva thank you for merging this! :heart: can I ask you to release a new version, too?

you would do me a favor, if you choose to make it a 1.x release (because of the concern in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/117107#note_1362056820). if you think that is not justified, can you please let me know what is missing?

winniehell commented 1 year ago

ah, I see, you have already released 0.1.0 :rocket: