search

jrobind / dev-jot

Note-taking app for online development tutorials
MIT License
54 stars 45 forks source link

Jest #122

Closed gaddscientist closed 3 years ago

gaddscientist commented 3 years ago

Hi I've added a few tests(mostly unit tests) and wanted to get them looked over and merged. I'd like to keep working on testing but would also like to open the issue to anyone else who may contribute as well at this point.

NOTE: I added babel as a dev dependency so that the require/export default statements would work in a node testing environment.

In the init folder are the testing files. One for setTags.js, setUser.js, and showContainers.js respectively. Following that I added a file in the events folder to test the eventListeners.js file.

One additional issue I encountered was mocking a Quill object as it is loaded into the app via a CDN link in the index.html file. To work around this I added a folder js/test/ where I added a jest.setup.js file to create a global Quill object. To go with this is a quillMock.js file in the same folder to create an empty dummy class to resolve the unknown reference error.

All tests pass and no source code was modified. The command "npm run test" will show the output of course, but I added an additional npm script "npm run testCoverage", that runs jest with the --coverage flag to show code coverage output as well.

Please let me know if anything is done improperly and I would be happy to fix it. Thanks!

jrobind commented 3 years ago

Hey Bryan, this review is on my todo list for tomorrow!

gaddscientist commented 3 years ago

Hey sorry for the late reply, I really appreciate all the suggestions. I'll make some adjustments and push them up in the next few days. Thanks for all the help!

gaddscientist commented 3 years ago

I made some changes and pushed them up. I did not figure out my issue with the quill object, but if you'd rather I just remove it as it may just be on my end I would be happy to do so!

jrobind commented 3 years ago

I made some changes and pushed them up. I did not figure out my issue with the quill object, but if you'd rather I just remove it as it may just be on my end I would be happy to do so!

Thank you! I left you a comment regarding the Quill mock – it's not crucial and I'd liked to get this merged in. Can address at a later date.

Thanks again Bryan, this is a fantastic PR!