search

guileen / node-sendmail

send mail without setting up a SMTP server
http://guileen.github.com/node-sendmail
MIT License
560 stars 108 forks source link

Add Testing #28

Open GreenPioneer opened 7 years ago

GreenPioneer commented 7 years ago

Add testing to the module

CherryNerd commented 7 years ago

As you may or may not now, we have some modules published of our own and we almost always fully test our modules.
You can check these out: Mollie-ES6 and obj-denied.

Is that the kind of testing you'd like? Because I can make a start like that if you'd like.

GreenPioneer commented 7 years ago

@Geex-Renzo Testing Branch . I have to wipe my computer tonight so i pushed what i had started on and so far its really bare bones . But the idea is to test our that our emails are comming out as expected and then to test that were getting back good data back when we look at the hosts domain.

CherryNerd commented 7 years ago

Have you looked at the way I make the tests? Because for starters, I have no idea why you have added "ical-toolkit" to the testing script.

I think the following should be done: The main file should be refactored. Functions declared in the main export should be placed in some sort of library folder (lib) and the individual functions should be tested as well. This way, you're always sure it works the way you think you do, even if you add something new.

GreenPioneer commented 7 years ago

I added "icall toolkit" to test creating appointments and sending them through send mail

you make some good points. I think i will actually merge your code in and rewrite from there for testing.

CherryNerd commented 7 years ago

Nice, thanks for the merge! I think it's a good call to first start with writing the tests, before adding something new, to make sure the project works as is. This way, we can also make sure it works like it did, after adding "icall toolkit".

Would you like it if I made a start with writing tests, the way I'm used to doing it?

GreenPioneer commented 7 years ago

Yea i think that would be a huge help cause this holiday season has really just taken my time. Hope you had a merry christmas @Geex-Renzo and a have a happy new year

Talk soon

CherryNerd commented 7 years ago

No problem, I'll make a start this week ;) And yes, thank you, I had a merry Christmas, I hope you did as well! Happy new year @GreenPioneer!

CherryNerd commented 7 years ago

@GreenPioneer I've started to work on the tests on a fork of this repo, since I don't have access to this repo. You can see my progress here: https://github.com/Geexteam/node-sendmail/tree/tests

GreenPioneer commented 7 years ago

It looks like a good start. Keep up the good work @Geex-Renzo

geofmureithi-zz commented 7 years ago

@GreenPioneer @Geex-Renzo I could help with tests

CherryNerd commented 7 years ago

@geofmureithi Sounds good, I'll add a PR for my current status of the testing branch tomorrow, maybe you can work from there.

geofmureithi-zz commented 7 years ago

Sure will be glad to.

On Jul 11, 2017 5:16 PM, "Renzo" notifications@github.com wrote:

@geofmureithi https://github.com/geofmureithi Sounds good, I'll add a PR for my current status of the testing branch tomorrow, maybe you can work from there.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/guileen/node-sendmail/issues/28#issuecomment-314458015, or mute the thread https://github.com/notifications/unsubscribe-auth/AGnizlQX4ieDUxYC1VbXpBkMzx6rcVlPks5sM4O_gaJpZM4LFDRj .

geofmureithi-zz commented 6 years ago

@Geex-Renzo Not yet??

GreenPioneer commented 6 years ago

@geofmureithi @Geex-Renzo I'm looking at picking this back up to get this done because it's becoming apparent we need testing now more than ever

geofmureithi-zz commented 6 years ago

Renzo never got back. How do you plan to do them? I can help

On Mar 14, 2018 10:49 AM, "jason humphrey" notifications@github.com wrote:

@geofmureithi https://github.com/geofmureithi @Geex-Renzo https://github.com/geex-renzo I'm looking at picking this back up to get this done because it's becoming apparent we need testing now more than ever

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/guileen/node-sendmail/issues/28#issuecomment-372932740, or mute the thread https://github.com/notifications/unsubscribe-auth/AGnizvrtFdL-yu4ASfkR1f4cvtiu26Vmks5teMuEgaJpZM4LFDRj .

CherryNerd commented 6 years ago

Sorry @GreenPioneer I've been busy with my internship so I don't have a lot of time atm... I can look at them tonight, as my new project needs this package (again) as well :) I'll get back to you within 48 hours, okay? :)

GreenPioneer commented 6 years ago

@geofmureithi @Geex-Renzo Sounds like testing will happen

Maybe we should also talk about 2.x features as well and bundle in testing?

do you guys have any thoughts towards 2.x ?

geofmureithi-zz commented 6 years ago

@GreenPioneer I will just work on unit testing using mocha. Let me see if we can do some integration tests. On 2.x, inbuilt promises would be good for a start. Allocate me some tasks btw :wink:

geofmureithi-zz commented 6 years ago

@GreenPioneer Also is it possible we can modularize the internal functions such as getHost to allow better unit testing?