Skip to content
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

Test coverage #45

Closed
brandonblock opened this issue Jan 20, 2017 · 6 comments
Closed

Test coverage #45

brandonblock opened this issue Jan 20, 2017 · 6 comments
Assignees

Comments

@brandonblock
Copy link

Hiya, I used go-guerilla to set up a dummy SMTP server for load testing our senders and I'd like to pay back the favor by increasing test coverage.

Curious about priority packages and testing conventions. Thanks!

@flashmob
Copy link
Owner

Thanks, glad that it was useful!

The current tests are written using the go test way, no other conventions. Just add them .travis.yml 's "script:" section if you want them to run after each PR.

It would be good to write tests for the two bugs #44 and #42 so they won't happen again. So that would be the HELO/EHLO commands and DATA command.

So, to start you off - have a look at server_test.go (just committed). It has an example for how to mock net.Conn . (There are other tests such as tests/guerrilla_test.go which tests the server as a package and serve_test.go which test as a command line program where you may get examples form)

This issue also carries a bounty reward - so it would be wonderful to reward for the two cases (or maybe you can do more?)

@brandonblock
Copy link
Author

Cool! I'll likely give it a peek this weekend.

@brandonblock
Copy link
Author

@flashmob Hiya, so I've been working on this a little bit and noticed that the cmd handling logic isn't abstracted at all (so it's a little difficult to atomically test) – I'm wondering if abstracting that to a function would be out-of-bounds or welcome (so the PR would be an abstraction of that function and test cases for all the handled commands).

Also noticed that there's "coverage" on some functions in server.go from client_test.go but that there are uncaught errors in the tests. Is there a slack/irc channel for development that it would be good to ask specific questions about? Or are the issue threads the best place?

@brandonblock
Copy link
Author

brandonblock commented Jan 26, 2017

Oh, also the response types that are currently strings could be statically typed by net/http.Response.StatusCode* but I don't know if the project wants that dependency.

@phires phires mentioned this issue Jan 26, 2017
@flashmob
Copy link
Owner

flashmob commented Jan 26, 2017

Hi, Brandon! @dapaxx has made some progress on the "enhanced status codes" SMTP extension - it has been merged to master recently. If you have a way to improve these - please let us know. Yes, using an int for the code is better than using a string.

The cmd handling logic was introduced by @remohammadi - maybe he already had solved the problem for how to test this individually, maybe he has some advice for us?
If you want to refactor - do you want to do something like this?: - http://stackoverflow.com/questions/35827147/cobra-viper-golang-how-to-test-subcommands

As for slack/irc, then the project may be too small for that - but asking in the issues is OK for now.

Finally, you're welcome to refactor something that makes something more testable, any day!

@brandonblock
Copy link
Author

Looks like @dapaxx has done some great work on this front. I don't have as much free time right now as I was hoping so I'm going to contribute to coverage via unit tests for some of the abstract functions and dive deeper when I have the bandwidth.

flashmob pushed a commit that referenced this issue Jan 28, 2017
* Additional test for HELO/EHLO, see issue #44.
Also raises code coverage, see issue #45
@flashmob flashmob closed this as completed Mar 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants