Skip to content

Commit

Permalink
Finish TestEmitMessage, fix writer short write bug
Browse files Browse the repository at this point in the history
email/message.go is now 100% covered. Though I may reorganize the tests
and helpers further, I'm generally pleased with how the tests turned
out.

Also fixes a latent bug in writer.Write that would result in
io.ErrShortWrite errors. Per the comment from the new
ReturnsInputLenAfterErrToAvoidIoErrShortWrite test case:

From: https://pkg.go.dev/io#pkg-variables

> ErrShortWrite means that a write accepted fewer bytes than requested
> but failed to return an explicit error.

This bug was originally surfaced by TestEmitMessage/ReturnsWriteErrors.
  • Loading branch information
mbland committed May 3, 2023
1 parent fb9db02 commit 1350493
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 16 deletions.
7 changes: 7 additions & 0 deletions email/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package email

import (
"bytes"
"fmt"
"io"
"mime"
"mime/multipart"
Expand Down Expand Up @@ -75,6 +76,10 @@ func (mt *MessageTemplate) EmitMessage(b io.Writer, sub *Subscriber) error {
} else {
mt.emitMultipart(w, sub)
}

if w.err != nil {
w.err = fmt.Errorf("error emitting message to %s: %s", sub.Email, w.err)
}
return w.err
}

Expand All @@ -94,6 +99,8 @@ func (w *writer) Write(b []byte) (n int, err error) {
if w.err == nil {
n, err = w.buf.Write(b)
w.err = err
} else {
n = len(b)
}
return
}
Expand Down
117 changes: 101 additions & 16 deletions email/message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,25 @@ func TestWriter(t *testing.T) {

assert.Equal(t, sb.String(), msg+"\r\n")
})

t.Run("ReturnsInputLenAfterErrToAvoidIoErrShortWrite", func(t *testing.T) {
// From: https://pkg.go.dev/io#pkg-variables
//
// > ErrShortWrite means that a write accepted fewer bytes than
// > requested but failed to return an explicit error.
//
// This bug was originally surfaced by TestEmitMessage/
// ReturnsWriteErrors.
sb, w := setup()
w.err = errors.New("make subsequent callers think Write succeeded")
const msg = "Hello, World!"

n, err := w.Write([]byte(msg))

assert.NilError(t, err)
assert.Equal(t, "", sb.String())
assert.Equal(t, len(msg), n)
})
}

func TestConvertToCrlf(t *testing.T) {
Expand Down Expand Up @@ -554,27 +573,93 @@ func TestEmitMultipart(t *testing.T) {
})
}

func TestMessage(t *testing.T) {
const testUnsubHeaderValue = "<mailto:" + testUnsubEmail +
"?subject=subscriber@foo.com%20" + testUid + ">, " +
"<" + testUnsubBaseUrl + "subscriber@foo.com/" + testUid + ">"

const expectedHeaders = "From: EListMan@foo.com\r\n" +
"To: subscriber@foo.com\r\n" +
"Subject: This is a test\r\n" +
"List-Unsubscribe: " + testUnsubHeaderValue + "\r\n" +
"List-Unsubscribe-Post: List-Unsubscribe=One-Click\r\n" +
"MIME-Version: 1.0\r\n"

type testHeader struct {
mail.Header
}

func (th *testHeader) assert(t *testing.T, name string, expected string) {
t.Helper()

if actual := th.Get(name); actual != expected {
t.Errorf("expected %s header: %s, actual: %s", name, expected, actual)
}
}

func assertMessageHeaders(t *testing.T, msg *mail.Message, content string) {
t.Helper()

th := testHeader{msg.Header}
th.assert(t, "From", testMessage.From)
th.assert(t, "To", testSubscriber.Email)
th.assert(t, "Subject", testMessage.Subject)
th.assert(t, "List-Unsubscribe", testUnsubHeaderValue)
th.assert(t, "List-Unsubscribe-Post", "List-Unsubscribe=One-Click")
th.assert(t, "MIME-Version", "1.0")
}

func TestEmitMessage(t *testing.T) {
setup := func() (*strings.Builder, *writer, *Subscriber) {
sb := &strings.Builder{}
sub := newTestSubscriber()
sub.SetUnsubscribeInfo(testUnsubEmail, testUnsubBaseUrl)
return sb, &writer{buf: sb}, sub
}

setupWithError := func(errMsg string) (*writer, *ErrWriter, *Subscriber) {
sb, w, sub := setup()
ew := &ErrWriter{buf: sb, err: errors.New(errMsg)}
w.buf = ew
return w, ew, sub
}

t.Run("EmitsPlaintextMessage", func(t *testing.T) {
t.Skip("unimplemented")
sb, w, sub := setup()
textTemplate := *testTemplate
textTemplate.htmlBody = []byte{}

err := textTemplate.EmitMessage(w, sub)

assert.NilError(t, err)
content := sb.String()
msg, qpr := parseTextMessage(t, content)
assert.Equal(t, expectedHeaders+textOnlyContent, content)
assertMessageHeaders(t, msg, content)
assertDecodedContent(t, qpr, decodedTextContent)
})

t.Run("EmitsMultipartMessage", func(t *testing.T) {
t.Skip("pause")
mt := NewMessageTemplate(testMessage)
buf := &strings.Builder{}
sub := *testSubscriber
sub.SetUnsubscribeInfo(
"unsubscribe@foo.com", "https://foo.com/email/unsubscribe/",
)

err := mt.EmitMessage(buf, &sub)
assert.NilError(t, err)
sb, w, sub := setup()

err := testTemplate.EmitMessage(w, sub)

msg := buf.String()
_, err = mail.ReadMessage(strings.NewReader(msg))
assert.NilError(t, err)
assert.Assert(t, strings.HasSuffix(msg, "\r\n"))
assert.Equal(t, msg, "")
content := sb.String()
msg, boundary, pr := parseMultipartMessageAndBoundary(t, content)
assert.Equal(t, expectedHeaders+multipartContent(boundary), content)
assertMessageHeaders(t, msg, content)
assertNextPart(t, pr, "text/plain", decodedTextContent)
assertNextPart(t, pr, "text/html", decodedHtmlContent)
})

t.Run("ReturnsWriteErrors", func(t *testing.T) {
w, ew, sub := setupWithError("write MIME-Version error")
ew.errorOn = "MIME-Version"

err := testTemplate.EmitMessage(w, sub)

expected := "error emitting message to " + sub.Email +
": write MIME-Version error"
assert.Error(t, err, expected)
})
}

0 comments on commit 1350493

Please sign in to comment.