search

testing-library / vue-testing-library

🦎 Simple and complete Vue.js testing utilities that encourage good testing practices.
http://testing-library.com/vue
MIT License
1.08k stars 110 forks source link

Fix updateProps Function #170

Closed ITenthusiasm closed 3 years ago

ITenthusiasm commented 3 years ago

The Problem

I just discovered a bug in one of my projects with the updateProps function returned from render. Whenever updateProps is called multiple times, several errors are logged in the console and Jest is prevented from finishing its test run. This is a huge bug for obvious reasons. Anyone can reproduce this error by grabbing the (currently) latest version of VTL and making multiple awaited calls to updateProps in a test for a Vue component.

The logged errors seem to be related to asynchronicity. I think that since the Promise returned from updateProps is different from the one returned from waitFor, everything gets thrown out of balance when the developer tries to run tests with multiple updateProps calls.

The fix I'm proposing simply returns the Promise obtained from wrapper.setProps directly. I applied this fix to my node_modules, and my tests behaved properly afterwards.

Changes

codecov[bot] commented 3 years ago

Codecov Report

Merging #170 (8d8aee2) into master (dbcf740) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #170   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           90        88    -2     
  Branches        28        28           
=========================================
- Hits            90        88    -2
Impacted Files Coverage Δ
src/vue-testing-library.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update dbcf740...8d8aee2. Read the comment docs.

afontcu commented 3 years ago

Hi! Thank you very much for this 🙌 It would be great if you could add a test that would fail with the previous implementation and that passes with the proposed one, so that we could easily see the issue and how it is prevented :)

ITenthusiasm commented 3 years ago

I know the code change, but I don't seem to be able to run a check locally. After running an npm install with the next branch, I haven't been able to run tests properly on master. Also, I noticed that for some reason package-lock.json files aren't generated when an install occurs.

ITenthusiasm commented 3 years ago

@afontcu Done.

Regarding my previous comment, it seems that

  1. Performing npm install on master (or a branch on top of it)
  2. Then performing npm install on next (or a branch on top of it)

Will cause problems to happen -- at least on certain machines. And the problem is that future installs on the master branch (or "related" branches) will permanently be broken, causing attempted tests to end in failure frequently. The next branch still seems to be able to run its tests just fine.

I'm running Ubuntu 18.04. After installing on master, then next, I was unable to perform a proper install on master anymore. More specifically, after performing an install on the master branch, I would still be unable to run any tests successfully. I can't tell if this is an issue with the master branch, the next branch, my machine, or some combination in between. Something could be wrong with the packages too? But it complicates contributing because switching between next and master branches requires me to use 2 different local versions of the forked repo. I think it's something worth investigating.

afontcu commented 3 years ago

Hi!

I know the code change, but I don't seem to be able to run a check locally. After running an npm install with the next branch, I haven't been able to run tests properly on master.

next branch is using Vue 3 and I recently merged some changes regarding types that might break that branch. That's OK, I'll be fixing them soon.

Also, I noticed that for some reason package-lock.json files aren't generated when an install occurs.

This is intentional, see https://github.com/testing-library/vue-testing-library/blob/master/.npmrc

github-actions[bot] commented 3 years ago

:tada: This PR is included in version 5.3.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

ITenthusiasm commented 3 years ago

That makes sense. Thanks!

afontcu commented 3 years ago

Thank you! 🤗

github-actions[bot] commented 2 years ago

:tada: This PR is included in version 6.6.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: