xhit / go-simple-mail

Golang package for send email. Support keep alive connection, TLS and SSL. Easy for bulk SMTP.
MIT License
650 stars 102 forks source link

How to run tests? #7

Closed dmke closed 1 year ago

dmke commented 4 years ago

The example_test.go wants to connect to localhost:25. I'm not certain, I want to install a mail server on my machine. I definitively won't install a Go environment on my mail server.

$ go test ./...
--- FAIL: TestSendMail (0.00s)
    example_test.go:50: Expected nil, got Mail Error on dailing with encryption type None: dial tcp [::1]:25: connect: connection refused connecting to client
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x63d18c]

goroutine 19 [running]:
testing.tRunner.func1.1(0x6749a0, 0x8a9cf0)
        /home/dm/.local/go/src/testing/testing.go:941 +0x3d0
testing.tRunner.func1(0xc00014e240)
        /home/dm/.local/go/src/testing/testing.go:944 +0x3f9
panic(0x6749a0, 0x8a9cf0)
        /home/dm/.local/go/src/runtime/panic.go:967 +0x15d
github.com/xhit/go-simple-mail/v2.(*SMTPClient).Noop(...)
        /home/dm/code/go-simple-mail/email.go:830
github.com/xhit/go-simple-mail/v2.TestSendMail(0xc00014e240)
        /home/dm/code/go-simple-mail/example_test.go:55 +0x1bc
testing.tRunner(0xc00014e240, 0x6cc578)
        /home/dm/.local/go/src/testing/testing.go:992 +0xdc
created by testing.(*T).Run
        /home/dm/.local/go/src/testing/testing.go:1043 +0x357
FAIL    github.com/xhit/go-simple-mail/v2       0.005s
FAIL

I'd be OK with running the tests in a confined environment, like a Docker container. Do you have something prepared for this?

In the same vein: Are there plans to automate test runs on commit, with say Github Actions?

dmke commented 4 years ago

When moving the example_test.go to example/example_test.go (and fixing the imports), it reveals that email.getFrom() is not valid for an example, as users of this library cannot access unexported functions:

https://github.com/xhit/go-simple-mail/blob/289c73008eaf6162fa86a17c769b288a4dce7354/example_test.go#L153-L155

xhit commented 4 years ago

Hi!

Yes, I have a environment to run the test but is a simple smtp server using mailslurper.

And yes, maybe using github actions will be great.

Currently I'm out this weekend.

Feel free to make a PR if you find a better test with Github Actions.

Thanks for the review 😀

xhit commented 4 years ago

About the getFrom should be exported. I fixed in production but missed this. Thanks.

dmke commented 4 years ago

I too have some plans for this weekend. But I've written a small review on /r/golang (comment), which I might transform into PRs in the coming days.

dmke commented 4 years ago

While preparing #8, I've thought about how to improve the test situation:

I was initially planning to install Postfix when running Github Actions, but I think installing mailslurper or MailHog would be more appropriate. I guess, having a mock server running inline with the tests (similar to (*net/http/httptest).Server) would be even better, however I've not yet found an SMTP server implementation which could be used for that (there are some, but they can't be used as library)...

xhit commented 4 years ago

About test, I'm considering mailtrap. The free version can do the job.

With mailtrap, it's possible make a complete integration test (not with free version, but this allows almost 80% of complete package test).

It's a suggestion, maybe a more reliable smtp mock server is in the sea.

dmke commented 4 years ago

I don't like to rely on services on the internet when running automated tests. Network service can fail at any moment (which generates the worst kind of test: flaky ones), and will always introduce some unwanted latency. I'm OK with running a service on the same network (or host) though, and/or separating "live" tests for manual execution.

In go-acme/lego, we're following this approach: Stub/mock as much as you can, but always provide a way to test the essential functions against real servers. Take for example this test file:

I could model the test suite in a similar fashion. You can then add the credentials for mailtrap in Github Actions as (secret) environment variables (if you so desire). This would be a mid- to long-term goal, though.

To get something more reliable working, I've been browsing through the sources of MailHog's SMTP server implementation, which looks like an ideal base for a smtptest library. It is MIT-licensed and has only two dependencies:

I'm tempted to fork mailhog/smtp, merge some open PRs, and integrate mailhog/data into it. The resulting server implementation can then be used inline as mock server.

What do you think?

xhit commented 4 years ago

Thanks for the research!

I know that tests should be simple and scalable, this is why I make all integration tests with docker for Dixer and also I know this package should have a great test, although is "simple" contains a lot of hidden abstraction for the developer to make the email sending too simple.

The complication to me is not how to make the test, is how to test the SSL, CRAM-MD5 and STARTTLS with one mock implementation.

If you have interest in this package, I could give you the access to collaborate. You can create a new branch to make all testing stuff and merge at the end, by now I'm in 4 projects (Dixer is one, other three include MLB, GBM and a TV channel group).

It specifies a set of environment variables which must be present for a "live" test (ll 12-17).
If some env vars are missing from that set, envTest.IsLiveTest() is false, and TestLivePresent (l. 268) and TestLiveCleanup() (l. 281) are skipped. These are the two tests which will try to communicate with the external service (Cloudflare in this case).
For lego, there's a caveat which this approach though: parts of the library are directly influenced by environment variables themselves, so that's why we need to push potentially existing env vars out of the way with ClearEnv()/RestoreEnv().

I think the mailhog package will deal with this. Also I can add some additional unit testing without necessity of mock.

Let me know if you want to be a collaborator, and thanks for your review and support.

dmke commented 4 years ago

Let me know if you want to be a collaborator

Sure. I was thinking of developing the smtptest module on my fork first create a PR sometime down the line. I guess it's easier to tinker with Github Actions that way, as I assume I'm going to experiment/force-push quite a lot until I've figured out a setup that works... :-)

how to test the SSL, CRAM-MD5 and STARTTLS with one mock implementation

For SSL, we can generate a certificate on-the-fly (or commit a long-lasting into the repo); STARTTLS requires wrapping the connection into a (*crypto/tls).Conn using tls.Server() after the STARTTLS command.

CRAM-MD5 is just a relative simple challenge/response auth method (I'm not sure about the protocol specifics, but the Wikipedia article describes a straight-forward algorithm).

I'm going to look and see, I guess. If it gets to overwhelming, I still might consider just preparing a Docker container with Postfix configured to deliver all mail locally.

xhit commented 4 years ago

Now you are a collaborator, thanks for the contribution!

xhit commented 1 year ago

test were added