From 6066be831c7a7cbd38bb68ea33c651dcf2eef41a Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Fri, 16 Feb 2024 17:04:35 -0800 Subject: [PATCH] chore: refactor playSession etc to use t.Fatal (#494) Signed-off-by: James Hillyerd --- pkg/server/smtp/handler_test.go | 106 ++++++++++---------------------- 1 file changed, 31 insertions(+), 75 deletions(-) diff --git a/pkg/server/smtp/handler_test.go b/pkg/server/smtp/handler_test.go index 550fad0..f86d113 100644 --- a/pkg/server/smtp/handler_test.go +++ b/pkg/server/smtp/handler_test.go @@ -49,9 +49,7 @@ func TestGreetStateValidCommands(t *testing.T) { script := []scriptStep{ tc, {"QUIT", 221}} - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) }) } } @@ -77,9 +75,7 @@ func TestGreetState(t *testing.T) { script := []scriptStep{ tc, {"QUIT", 221}} - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) }) } } @@ -94,18 +90,14 @@ func TestEmptyEnvelope(t *testing.T) { {"HELO localhost", 250}, {"MAIL FROM:<>", 501}, } - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) // Test out some empty envelope with blanks script = []scriptStep{ {"HELO localhost", 250}, {"MAIL FROM: <>", 501}, } - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) } // Test AUTH commands. @@ -125,9 +117,7 @@ func TestAuth(t *testing.T) { {"RSET", 250}, {"AUTH PLAIN aW5idWNrZXQ6cG Fzc3dvcmQK", 500}, } - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) // LOGIN AUTH script = []scriptStep{ @@ -140,9 +130,7 @@ func TestAuth(t *testing.T) { {"", 334}, {"", 235}, } - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) } // Test TLS commands. @@ -157,9 +145,7 @@ func TestTLS(t *testing.T) { {"STARTTLS", 454}, // TLS unconfigured. } - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) } // Test valid commands in READY state. @@ -191,9 +177,7 @@ func TestReadyStateValidCommands(t *testing.T) { {"HELO localhost", 250}, tc, {"QUIT", 221}} - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) }) } } @@ -217,9 +201,7 @@ func TestReadyStateRejectedDomains(t *testing.T) { {"HELO localhost", 250}, tc, {"QUIT", 221}} - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) }) } } @@ -249,9 +231,7 @@ func TestReadyStateInvalidCommands(t *testing.T) { {"HELO localhost", 250}, tc, {"QUIT", 221}} - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) }) } } @@ -276,9 +256,7 @@ func TestMailState(t *testing.T) { {"RCPT TO:", 501}, {"RCPT TO:", 250}, {"RCPT TO:", 250}, } - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) // Test out recipient limit script = []scriptStep{ @@ -310,9 +286,7 @@ func TestMailState(t *testing.T) { {"RCPT TO:", 250}, {"RCPT TO:", 552}, } - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) // Test DATA script = []scriptStep{ @@ -322,9 +296,7 @@ func TestMailState(t *testing.T) { {"DATA", 354}, {".", 250}, } - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) // Test late EHLO, similar to RSET script = []scriptStep{ @@ -335,9 +307,7 @@ func TestMailState(t *testing.T) { {"EHLO localhost", 250}, {"MAIL FROM:", 250}, } - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) // Test RSET script = []scriptStep{ @@ -347,9 +317,7 @@ func TestMailState(t *testing.T) { {"RSET", 250}, {"MAIL FROM:", 250}, } - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) // Test QUIT script = []scriptStep{ @@ -358,9 +326,7 @@ func TestMailState(t *testing.T) { {"RCPT TO:", 250}, {"QUIT", 221}, } - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) } // Test commands in DATA state @@ -382,9 +348,7 @@ func TestDataState(t *testing.T) { {"RCPT TO:", 250}, {"DATA", 354}, } - if err := playScriptAgainst(t, c, script); err != nil { - t.Error(err) - } + playScriptAgainst(t, c, script) // Send a message body := `To: u1@gmail.com @@ -414,9 +378,7 @@ Hi! {"RCPT TO:", 250}, {"DATA", 354}, } - if err := playScriptAgainst(t, c, script); err != nil { - t.Error(err) - } + playScriptAgainst(t, c, script) // Send a message body = `X-Useless-Header: true @@ -434,31 +396,30 @@ Hi! } // playSession creates a new session, reads the greeting and then plays the script -func playSession(t *testing.T, server *Server, script []scriptStep) error { +func playSession(t *testing.T, server *Server, script []scriptStep) { t.Helper() pipe := setupSMTPSession(t, server) c := textproto.NewConn(pipe) if code, _, err := c.ReadCodeLine(220); err != nil { - return fmt.Errorf("expected a 220 greeting, got %v", code) + t.Errorf("expected a 220 greeting, got %v", code) } - err := playScriptAgainst(t, c, script) + playScriptAgainst(t, c, script) - // Not all tests leave the session in a clean state, so the following two - // calls can fail + // Not all tests leave the session in a clean state, so the following two calls can fail _, _ = c.Cmd("QUIT") _, _, _ = c.ReadCodeLine(221) - - return err } // playScriptAgainst an existing connection, does not handle server greeting -func playScriptAgainst(t *testing.T, c *textproto.Conn, script []scriptStep) error { +func playScriptAgainst(t *testing.T, c *textproto.Conn, script []scriptStep) { + t.Helper() + for i, step := range script { id, err := c.Cmd(step.send) if err != nil { - return fmt.Errorf("Step %d, failed to send %q: %v", i, step.send, err) + t.Fatalf("Step %d, failed to send %q: %v", i, step.send, err) } c.StartResponse(id) @@ -470,11 +431,10 @@ func playScriptAgainst(t *testing.T, c *textproto.Conn, script []scriptStep) err c.EndResponse(id) if err != nil { - // Return after c.EndResponse so we don't hang the connection - return err + // Fail after c.EndResponse so we don't hang the connection + t.Fatal(err) } } - return nil } // Tests "MAIL FROM" emits BeforeMailAccepted event. @@ -497,9 +457,7 @@ func TestBeforeMailAcceptedEventEmitted(t *testing.T) { {"HELO localhost", 250}, {"MAIL FROM:", 250}, {"QUIT", 221}} - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) assert.NotNil(t, got, "BeforeMailListener did not receive Address") assert.Equal(t, "john", got.Local, "Address local part had wrong value") @@ -554,9 +512,7 @@ func TestBeforeMailAcceptedEventResponse(t *testing.T) { {"HELO localhost", 250}, tc.script, {"QUIT", 221}} - if err := playSession(t, server, script); err != nil { - t.Error(err) - } + playSession(t, server, script) assert.NotNil(t, gotEvent, "BeforeMailListener did not receive Address") })