-
Notifications
You must be signed in to change notification settings - Fork 16
Add some frontend vue.js component tests #1306
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
Conversation
05d94a3 to
ff4ccea
Compare
ff4ccea to
a65c5b2
Compare
a65c5b2 to
14b68c8
Compare
14b68c8 to
a9bcab2
Compare
|
a9bcab2 to
9ead2b7
Compare
9ead2b7 to
47b63d1
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.
Love it! Great work! 👏
Other than the inline comments I have two more questions / points:
-
I see that we have a
package.jsonscript fortestbut I just wanted to double check that it is already automagically picked up by our CI to be ran or if there's anything else needed there? -
(Nit) The actual component file names are written as
TabBarinstead oftab-bar. I wonder if we should keep the file naming consistent here or is this a limitation of sorts for testing purposes?
Oh also there are some conflicts on the package-lock.json file to be resolved before merge!
|
Thanks for the review and input!
Yes the tests that I am adding run with the other pre-existing frontend tests which already run in CI; the github validate frontend job does an npm install that uses the
Ah yes, I noticed that too but decided to keep the same naming format as the already existing tests in https://github.com/thunderbird/appointment/tree/main/frontend/test/stores, however I see the stores are actually named with
Ok thanks I didn't see this locally but I'll have a look. |
47b63d1 to
d6c2b5d
Compare
d6c2b5d to
28def1e
Compare
|
Thanks for the review @davinotdavid, updates are complete and ready for another review whenever you have some time. |
davinotdavid
left a comment
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.
Awesome! Thank you so much for the research / investigations as well, TIL!
Fixes #1202. Just add some starting vue.js UI component tests. These should be built out further over time.