-
Notifications
You must be signed in to change notification settings - Fork 157
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
[unit-tests-only] Add unit tests for SharedViaLink view #5778
Conversation
package.json
Outdated
@@ -77,6 +77,7 @@ | |||
"eslint-plugin-promise": "^5.1.0", | |||
"eslint-plugin-vue": "^7.13.0", | |||
"eslint-plugin-vuejs-accessibility": "^0.7.1", | |||
"flush-promises": "^1.0.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This package helps to flush all promises which means that we don't need to use wrapper.vm.$nextTick()
twice or more in order to wait for all promises to get resolved.
https://vue-test-utils.vuejs.org/guides/#testing-asynchronous-behavior
Should we use it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dpakach does this helps for the test you're writing for autocomplete inputs taking some time to trigger. If such problems with static waits or of course multiple nextTick can be resolved...imo this might be useful
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sadly doesn't works in my case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR needs a rebase and I'm not sure why there was an attempt to use promises so heavily here that it needs an additional library to work around it. Trying a different approach, I'll ping you once I know if it works;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pascalwengerter, @kiranparajuli589 has made an update to this PR. Please review.
bbff3af
to
8ab03d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm 👍🏼
8ab03d7
to
1e09232
Compare
f78e678
to
da1cfd9
Compare
da1cfd9
to
9537850
Compare
9537850
to
0aedd13
Compare
296f502
to
ee432a6
Compare
ee432a6
to
7b9e8dd
Compare
7b9e8dd
to
59012c7
Compare
blocked waiting for review @kulmann @pascalwengerter |
59012c7
to
3a54934
Compare
74838e9
to
68280f7
Compare
add flush-promises package add unit tests for SharedViaLink view add unit tests for SharedViaLink view address reviews update tests after component changes
68280f7
to
ebea6d4
Compare
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I think we can take a look into adding better tests for the Files App Views but this is a good start 🥅
Description
Added unit tests for
SharedViaLink
viewRelated Issue
Motivation and Context
increase unit test coverage
How Has This Been Tested?
Types of changes
Checklist: