-
Notifications
You must be signed in to change notification settings - Fork 113
fix(#119): allow vue-router creation through routes param from both jest and mocha #127
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
... both jest and mocha
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.
The fix-code is ugly cause the problem is that in mocha unit tests we need to bundle the code with webpack (mocha-webpack) and that does not play well with using a require here.
The problem could be also solved with an async import (to follow ES6 notation) but that would probably make the code even uglier cause it would introduce async code.
Another workaround when using mocha, would be passing an already built router through router mounting option (instead of routes) but that cannot work be done with the current implementation.
Enabling this option for mocha users would be highly appreciated. Thanks for your awesome work!
Hi! Thanks for this! 😃
-
Is there a way to add some tests to make sure things are working as expected? I'm not well versed with mocha, but that would be great (right now CI pipeline is failing due to code coverage descending).
-
Should we do the same thing with Vuex?
Codecov Report
@@ Coverage Diff @@ ## master #127 +/- ## ===================================== Coverage 100% 100% ===================================== Files 1 1 Lines 67 67 Branches 13 14 +1 ===================================== Hits 67 67
Continue to review full report at Codecov.
|
Is there a way to add some tests to make sure things are working as expected? I'm not well versed with mocha, but that would be great (right now CI pipeline is failing due to code coverage descending).
Some mocha tests could be added but that would not affect the code coverage as mocha does not use jest runner.
However, I just commited a hacky jest test (vue-router-mocha.js) that exercises the added code to keep the same coverage ( not the most expressive test though, I know 😅)
Should we do the same thing with Vuex?
No, we don't need it because vuex is built/exported in a different way than vue-router so that require works out of the box with mocha.
Looks good! I'm willing to merge this 🎉. Yet, do you have an external repository or something where we could see the fix actually fixing the issue? I've never used mocha, so I just want to make sure mocha is 100% usable after this 😄
Yet, do you have an external repository or something where we could see the fix actually fixing the issue?
(削除) The repository pointing the issue (#119) reproduces the error with this mocha test (削除ここまで)
@afontcu If you replace vue-testing-library with the code in current PR, that test will ✅ 😊
UPDATE: I double checked that repo and It seems that I lied 😅 as the linked repo did not have a correct chai setup. I quickly fixed it in this fork.
BTW, I prefer jest over mocha but it need to use it in a legacy project. I think that I could help to mimic vue-testing-library examples in mocha but I would suggest to do it in the official vue-testing-library repository so it's easier to find/maintain them.
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.
Sure, let's go with this! Since I won't be using mocha, feel free to keep submitting PRs if you run into similar issues 😄
Thanks for this!
This code fixes issue.