From d7e03eccccd1b19d0eb3e1aea43d871c2bed366d Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Tue, 12 Mar 2019 13:37:41 +0100 Subject: dashboard/app: use pkg/html to format text emails Use pkg/html helpers to format text emails. Shorten short hashes to 8 char while we are here, the length used by git log --oneline. Tidy up tests a bit. Update #501 --- dashboard/app/email_test.go | 36 ++++++++++-------------------- dashboard/app/jobs_test.go | 45 +++++++++++++------------------------- dashboard/app/mail_bug.txt | 2 +- dashboard/app/mail_test_result.txt | 2 +- dashboard/app/reporting_email.go | 35 ++++------------------------- 5 files changed, 33 insertions(+), 87 deletions(-) diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go index 012e50f12..beaacce42 100644 --- a/dashboard/app/email_test.go +++ b/dashboard/app/email_test.go @@ -42,11 +42,11 @@ func TestEmailReport(t *testing.T) { c.expectEQ(msg.To, []string{to}) c.expectEQ(msg.Subject, crash.Title) c.expectEQ(len(msg.Attachments), 0) - body := fmt.Sprintf(`Hello, + c.expectEQ(msg.Body, fmt.Sprintf(`Hello, syzbot found the following crash on: -HEAD commit: 111111111111 kernel_commit_title1 +HEAD commit: 11111111 kernel_commit_title1 git tree: repo1 branch1 console output: %[2]v kernel config: %[3]v @@ -68,10 +68,7 @@ syzbot engineers can be reached at syzkaller@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot.`, - extBugID0, crashLogLink, kernelConfigLink) - if msg.Body != body { - t.Fatalf("got email body:\n%s\n\nwant:\n%s", msg.Body, body) - } + extBugID0, crashLogLink, kernelConfigLink)) c.checkURLContents(crashLogLink, crash.Log) c.checkURLContents(kernelConfigLink, build.KernelConfig) } @@ -144,9 +141,9 @@ For more options, visit https://groups.google.com/d/optout. c.expectEQ(msg.Subject, "Re: "+crash.Title) c.expectEQ(len(msg.Attachments), 0) c.expectEQ(msg.Headers["In-Reply-To"], []string{"<1234>"}) - body := fmt.Sprintf(`syzbot has found a reproducer for the following crash on: + c.expectEQ(msg.Body, fmt.Sprintf(`syzbot has found a reproducer for the following crash on: -HEAD commit: 101010101010 a really long title, longer than 80 chars, re.. +HEAD commit: 10101010 a really long title, longer than 80 chars, really.. git tree: repo10alias console output: %[3]v kernel config: %[4]v @@ -159,10 +156,7 @@ IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+%[1]v@testapp.appspotmail.com report1 -`, extBugID0, reproSyzLink, crashLogLink, kernelConfigLink) - if msg.Body != body { - t.Fatalf("got email body:\n%s\n\nwant:\n%s", msg.Body, body) - } +`, extBugID0, reproSyzLink, crashLogLink, kernelConfigLink)) c.checkURLContents(reproSyzLink, syzRepro) c.checkURLContents(crashLogLink, crash.Log) c.checkURLContents(kernelConfigLink, build2.KernelConfig) @@ -189,11 +183,11 @@ report1 "default@maintainers.com", "foo@bar.com", "maintainers@repo10.org"}) c.expectEQ(msg.Subject, crash.Title) c.expectEQ(len(msg.Attachments), 0) - body := fmt.Sprintf(`Hello, + c.expectEQ(msg.Body, fmt.Sprintf(`Hello, syzbot found the following crash on: -HEAD commit: 101010101010 a really long title, longer than 80 chars, re.. +HEAD commit: 10101010 a really long title, longer than 80 chars, really.. git tree: repo10alias console output: %[3]v kernel config: %[4]v @@ -216,10 +210,7 @@ syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches`, - extBugID1, reproSyzLink, crashLogLink, kernelConfigLink) - if msg.Body != body { - t.Fatalf("got email body:\n%s\n\nwant:\n%s", msg.Body, body) - } + extBugID1, reproSyzLink, crashLogLink, kernelConfigLink)) c.checkURLContents(reproSyzLink, syzRepro) c.checkURLContents(crashLogLink, crash.Log) c.checkURLContents(kernelConfigLink, build2.KernelConfig) @@ -262,9 +253,9 @@ Content-Type: text/plain "maintainers@repo10.org", "new@new.com", "qux@qux.com"}) c.expectEQ(msg.Subject, "Re: "+crash.Title) c.expectEQ(len(msg.Attachments), 0) - body := fmt.Sprintf(`syzbot has found a reproducer for the following crash on: + c.expectEQ(msg.Body, fmt.Sprintf(`syzbot has found a reproducer for the following crash on: -HEAD commit: 101010101010 a really long title, longer than 80 chars, re.. +HEAD commit: 10101010 a really long title, longer than 80 chars, really.. git tree: repo10alias console output: %[4]v kernel config: %[5]v @@ -278,10 +269,7 @@ IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+%[1]v@testapp.appspotmail.com report1 -`, extBugID1, reproCLink, reproSyzLink, crashLogLink, kernelConfigLink) - if msg.Body != body { - t.Fatalf("got email body:\n%s\n\nwant:\n%s", msg.Body, body) - } +`, extBugID1, reproCLink, reproSyzLink, crashLogLink, kernelConfigLink)) c.checkURLContents(reproCLink, crash.ReproC) c.checkURLContents(reproSyzLink, syzRepro) c.checkURLContents(crashLogLink, crash.Log) diff --git a/dashboard/app/jobs_test.go b/dashboard/app/jobs_test.go index 70dc8acb3..e74a6d0fb 100644 --- a/dashboard/app/jobs_test.go +++ b/dashboard/app/jobs_test.go @@ -128,7 +128,7 @@ func TestJob(t *testing.T) { c.expectEQ(msg.To, to) c.expectEQ(msg.Subject, "Re: "+crash.Title) c.expectEQ(len(msg.Attachments), 0) - body := fmt.Sprintf(`Hello, + c.expectEQ(msg.Body, fmt.Sprintf(`Hello, syzbot has tested the proposed patch but the reproducer still triggered crash: test crash title @@ -137,17 +137,14 @@ test crash report Tested on: -commit: 111111111111 kernel_commit_title1 +commit: 11111111 kernel_commit_title1 git tree: repo1 branch1 console output: %[3]v kernel config: %[2]v compiler: compiler1 patch: %[1]v -`, patchLink, kernelConfigLink, logLink) - if msg.Body != body { - t.Fatalf("got email body:\n%s\n\nwant:\n%s", msg.Body, body) - } +`, patchLink, kernelConfigLink, logLink)) c.checkURLContents(patchLink, []byte(patch)) c.checkURLContents(kernelConfigLink, build.KernelConfig) c.checkURLContents(logLink, jobDoneReq.CrashLog) @@ -168,7 +165,7 @@ patch: %[1]v kernelConfigLink := externalLink(c.ctx, textKernelConfig, dbBuild.KernelConfig) msg := c.pollEmailBug() c.expectEQ(len(msg.Attachments), 0) - body := fmt.Sprintf(`Hello, + c.expectEQ(msg.Body, fmt.Sprintf(`Hello, syzbot tried to test the proposed patch but build/boot failed: @@ -177,16 +174,13 @@ failed to apply patch Tested on: -commit: 111111111111 kernel_commit_title1 +commit: 11111111 kernel_commit_title1 git tree: repo1 branch1 kernel config: %[2]v compiler: compiler1 patch: %[1]v -`, patchLink, kernelConfigLink) - if msg.Body != body { - t.Fatalf("got email body:\n%s\n\nwant:\n%s", msg.Body, body) - } +`, patchLink, kernelConfigLink)) c.checkURLContents(patchLink, []byte(patch)) c.checkURLContents(kernelConfigLink, build.KernelConfig) } @@ -208,7 +202,7 @@ patch: %[1]v msg := c.pollEmailBug() c.expectEQ(len(msg.Attachments), 0) truncatedError := string(jobDoneReq.Error[len(jobDoneReq.Error)-maxInlineError:]) - body := fmt.Sprintf(`Hello, + c.expectEQ(msg.Body, fmt.Sprintf(`Hello, syzbot tried to test the proposed patch but build/boot failed: @@ -220,16 +214,13 @@ Error text is too large and was truncated, full error text is at: Tested on: -commit: 111111111111 kernel_commit_title1 +commit: 11111111 kernel_commit_title1 git tree: repo1 branch1 kernel config: %[4]v compiler: compiler1 patch: %[3]v -`, truncatedError, errorLink, patchLink, kernelConfigLink) - if msg.Body != body { - t.Fatalf("got email body:\n%s\n\nwant:\n%s", msg.Body, body) - } +`, truncatedError, errorLink, patchLink, kernelConfigLink)) c.checkURLContents(patchLink, []byte(patch)) c.checkURLContents(errorLink, jobDoneReq.Error) c.checkURLContents(kernelConfigLink, build.KernelConfig) @@ -248,7 +239,7 @@ patch: %[3]v kernelConfigLink := externalLink(c.ctx, textKernelConfig, dbBuild.KernelConfig) msg := c.pollEmailBug() c.expectEQ(len(msg.Attachments), 0) - body := fmt.Sprintf(`Hello, + c.expectEQ(msg.Body, fmt.Sprintf(`Hello, syzbot has tested the proposed patch and the reproducer did not trigger crash: @@ -256,17 +247,14 @@ Reported-and-tested-by: syzbot+%v@testapp.appspotmail.com Tested on: -commit: 111111111111 kernel_commit_title1 +commit: 11111111 kernel_commit_title1 git tree: repo1 branch1 kernel config: %[3]v compiler: compiler1 patch: %[2]v Note: testing is done by a robot and is best-effort only. -`, extBugID, patchLink, kernelConfigLink) - if msg.Body != body { - t.Fatalf("got email body:\n%s\n\nwant:\n%s", msg.Body, body) - } +`, extBugID, patchLink, kernelConfigLink)) c.checkURLContents(patchLink, []byte(patch)) c.checkURLContents(kernelConfigLink, build.KernelConfig) } @@ -307,7 +295,7 @@ func TestJobWithoutPatch(t *testing.T) { kernelConfigLink := externalLink(c.ctx, textKernelConfig, dbBuild.KernelConfig) msg := c.pollEmailBug() c.expectEQ(len(msg.Attachments), 0) - body := fmt.Sprintf(`Hello, + c.expectEQ(msg.Body, fmt.Sprintf(`Hello, syzbot has tested the proposed patch and the reproducer did not trigger crash: @@ -315,16 +303,13 @@ Reported-and-tested-by: syzbot+%v@testapp.appspotmail.com Tested on: -commit: 5e6a2eea5e6a kernel_commit_title2 +commit: 5e6a2eea kernel_commit_title2 git tree: git://mygit.com/git.git kernel config: %[2]v compiler: compiler2 Note: testing is done by a robot and is best-effort only. -`, extBugID, kernelConfigLink) - if msg.Body != body { - t.Fatalf("got email body:\n%s\n\nwant:\n%s", msg.Body, body) - } +`, extBugID, kernelConfigLink)) c.checkURLContents(kernelConfigLink, testBuild.KernelConfig) } diff --git a/dashboard/app/mail_bug.txt b/dashboard/app/mail_bug.txt index 23f14385f..5b2ddf532 100644 --- a/dashboard/app/mail_bug.txt +++ b/dashboard/app/mail_bug.txt @@ -4,7 +4,7 @@ Hello, {{end -}} syzbot {{if .First}}found{{else}}has found a reproducer for{{end}} the following crash on: -HEAD commit: {{.KernelCommit}} {{.KernelCommitTitle}} +HEAD commit: {{formatShortHash .KernelCommit}} {{formatCommitTableTitle .KernelCommitTitle}} git tree: {{.KernelRepo}} {{if .LogLink}}console output: {{.LogLink}} {{end}}{{if .KernelConfigLink}}kernel config: {{.KernelConfigLink}} diff --git a/dashboard/app/mail_test_result.txt b/dashboard/app/mail_test_result.txt index f993824a9..30bad132b 100644 --- a/dashboard/app/mail_test_result.txt +++ b/dashboard/app/mail_test_result.txt @@ -19,7 +19,7 @@ Reported-and-tested-by: {{.CreditEmail}} {{end}} Tested on: -commit: {{.KernelCommit}} {{.KernelCommitTitle}} +commit: {{formatShortHash .KernelCommit}} {{formatCommitTableTitle .KernelCommitTitle}} git tree: {{.KernelRepo}} {{if .LogLink}}console output: {{.LogLink}} {{end}}{{if .KernelConfigLink}}kernel config: {{.KernelConfigLink}} diff --git a/dashboard/app/reporting_email.go b/dashboard/app/reporting_email.go index 2b6ffdc2f..3292bed98 100644 --- a/dashboard/app/reporting_email.go +++ b/dashboard/app/reporting_email.go @@ -13,11 +13,11 @@ import ( "regexp" "strconv" "strings" - "text/template" "time" "github.com/google/syzkaller/dashboard/dashapi" "github.com/google/syzkaller/pkg/email" + "github.com/google/syzkaller/pkg/html" "golang.org/x/net/context" "google.golang.org/appengine" "google.golang.org/appengine/log" @@ -49,8 +49,6 @@ const ( // entry pending for review. The prefix makes Patchwork // treat it as a comment for a previous patch. replySubjectPrefix = "Re: " - commitHashLen = 12 - commitTitleLen = 47 // so that whole line fits into 78 chars replyNoBugID = "I see the command but can't find the corresponding bug.\n" + "Please resend the email to %[1]v address\n" + @@ -262,7 +260,7 @@ func emailReport(c context.Context, rep *dashapi.BugReport, templ string) error KernelRepo string KernelCommit string KernelCommitTitle string - KernelCommitDate string + KernelCommitDate time.Time UserSpaceArch string CrashTitle string Report []byte @@ -286,7 +284,7 @@ func emailReport(c context.Context, rep *dashapi.BugReport, templ string) error KernelRepo: rep.KernelRepoAlias, KernelCommit: rep.KernelCommit, KernelCommitTitle: rep.KernelCommitTitle, - KernelCommitDate: formatKernelTime(rep.KernelCommitDate), + KernelCommitDate: rep.KernelCommitDate, UserSpaceArch: userspaceArch, CrashTitle: rep.CrashTitle, Report: rep.Report, @@ -300,12 +298,6 @@ func emailReport(c context.Context, rep *dashapi.BugReport, templ string) error HappenedOn: rep.HappenedOn, PatchLink: rep.PatchLink, } - if len(data.KernelCommit) > commitHashLen { - data.KernelCommit = data.KernelCommit[:commitHashLen] - } - if len(data.KernelCommitTitle) > commitTitleLen { - data.KernelCommitTitle = data.KernelCommitTitle[:commitTitleLen-2] + ".." - } log.Infof(c, "sending email %q to %q", rep.Title, to) return sendMailTemplate(c, rep.Title, from, to, rep.ExtID, nil, templ, data) } @@ -577,23 +569,4 @@ func appURL(c context.Context) string { return fmt.Sprintf("https://%v.appspot.com", appengine.AppID(c)) } -func formatKernelTime(t time.Time) string { - if t.IsZero() { - return "" - } - // This is how dates appear in git log. - return t.Format("Mon Jan 2 15:04:05 2006 -0700") -} - -func formatStringList(list []string) string { - return strings.Join(list, ", ") -} - -var ( - mailTemplates = template.Must(template.New("").Funcs(mailFuncs).ParseGlob("mail_*.txt")) - - mailFuncs = template.FuncMap{ - "formatTime": formatKernelTime, - "formatList": formatStringList, - } -) +var mailTemplates = html.CreateTextGlob("mail_*.txt") -- cgit mrf-deployment