cannawen / metric_units_reddit_bot

Reddit bot converting imperial units to metric units
https://www.reddit.com/user/metric_units
GNU General Public License v3.0
77 stars 34 forks source link

Add library for unit testing in javascript #18

Closed cannawen closed 7 years ago

cannawen commented 7 years ago

metric_units is a badly-tested reddit bot that finds imperial units, and replies with a metric conversion. Can you help make it less bad?

bot-test.js is an abomination where I rolled my own doubles (terrible idea, btw). Do you know how to unit test interactions between different files? Any libraries that can help? CAN YOU SAVE THIS POOR FILE (Or, just put it out of its misery and start from scratch)

Currently using mocha, chai, and proxyquire but open to new suggestions

andrewwylde commented 7 years ago

Hey @cannawen I'd be willing to contribute here.

Are there any criteria that would make this considered complete (i.e. acceptance criteria)

From what I can tell, you're looking for:

Does that sound about right?

cannawen commented 7 years ago

Hey! I am having trouble mostly with the test structure because the current way is not sustainable. An example is I refactored a function name in one of the modules, but the bot-test.js tests did not fail, because I was still stubbing the old function name in bot-test.js

I think just some happy-path tests would be sufficient, but however much you feel like doing would be greatly appreciated! Thanks for taking this on, feel free to reach out if you have any more questions!

andrewwylde commented 7 years ago

OK sounds good, let me give it some thought and see what I can do!

cannawen commented 7 years ago

Hey @astoellis we have just added a CONTRIBUTING.md doc, please check it out when you have time! Sections "Etiquette", "Work on an issue" and "Make a PR" are most important

andrewwylde commented 7 years ago

Hey, sorry for the massive delay. After a bit of a dive, I think it'd take some re-writing and re-wiring to get rid of the mocks. One library I've seen used (but I'm no expert in the testing realm), is Sinon.

I don't want to take up too much time, so instead maybe I can just make this recommendation and see where you think you'd like to take it.

I could take a stab at rewriting the tests w/ sinon, but my primary concern would be that it would end up a super large PR to get everything switched over without breaking in-between. Then again, maybe someone else has more experience in that department :).

cannawen commented 7 years ago

No worries! :) I am happy to delete bot-test.js and start anew with Sinon.

Maybe you can just write a few tests (for example, a test to make sure the timers are enabled with the correct numbers, and another test for one network call) so we can get a gist of how Sinon would look like with our project. Don't feel like you have to write all of the tests yourself, this issue was more meant to be "choose a better framework for tests." We can create a separate issue to test each separate flow

cannawen commented 7 years ago

Closing issue due to inactivity - perhaps we can open it up again in the future if anyone is interested