From 952c7994534bbc8f8134fc9c4130ff569dbbdbdf Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Mon, 11 Jun 2018 10:02:46 +0200 Subject: dashboard/app: refactor tests Use standard dashboard client provided by dashapi package in tests. Switch everything to use the client instead of API method. Fixes #606 --- dashboard/app/access_test.go | 62 ++++--- dashboard/app/app_test.go | 167 +++++++------------ dashboard/app/email_test.go | 43 +++-- dashboard/app/fix_test.go | 276 ++++++++++++------------------- dashboard/app/jobs_test.go | 52 +++--- dashboard/app/reporting_test.go | 358 +++++++++++----------------------------- dashboard/app/util_test.go | 114 ++++++------- dashboard/dashapi/dashapi.go | 117 +++++++++---- 8 files changed, 473 insertions(+), 716 deletions(-) diff --git a/dashboard/app/access_test.go b/dashboard/app/access_test.go index 5918d3694..bc03ec9eb 100644 --- a/dashboard/app/access_test.go +++ b/dashboard/app/access_test.go @@ -178,10 +178,10 @@ func TestAccess(t *testing.T) { clientName, clientKey = k, v } namespaceAccessPrefix := accessLevelPrefix(config.Namespaces[ns].AccessLevel) - client := c.makeClient(clientName, clientKey) + client := c.makeClient(clientName, clientKey, true) build := testBuild(1) build.KernelConfig = []byte(namespaceAccessPrefix + "build") - client.uploadBuild(build) + client.UploadBuild(build) noteBuildccessLevel(ns, build.ID) for reportingIdx := 0; reportingIdx < 2; reportingIdx++ { @@ -189,36 +189,34 @@ func TestAccess(t *testing.T) { accessPrefix := accessLevelPrefix(accessLevel) crashInvalid := testCrashWithRepro(build, reportingIdx*10+0) - client.reportCrash(crashInvalid) - repInvalid := reportAllBugs(c, 1)[0] + client.ReportCrash(crashInvalid) + repInvalid := client.pollBug() if reportingIdx != 0 { - c.expectTrue(client.updateBug(repInvalid.ID, dashapi.BugStatusUpstream, "").OK) - repInvalid = reportAllBugs(c, 1)[0] + client.updateBug(repInvalid.ID, dashapi.BugStatusUpstream, "") + repInvalid = client.pollBug() } - c.expectTrue(client.updateBug(repInvalid.ID, dashapi.BugStatusInvalid, "").OK) + client.updateBug(repInvalid.ID, dashapi.BugStatusInvalid, "") noteBugAccessLevel(repInvalid.ID, accessLevel) crashFixed := testCrashWithRepro(build, reportingIdx*10+0) - client.reportCrash(crashFixed) - repFixed := reportAllBugs(c, 1)[0] + client.ReportCrash(crashFixed) + repFixed := client.pollBug() if reportingIdx != 0 { - c.expectTrue(client.updateBug(repFixed.ID, dashapi.BugStatusUpstream, "").OK) - repFixed = reportAllBugs(c, 1)[0] + client.updateBug(repFixed.ID, dashapi.BugStatusUpstream, "") + repFixed = client.pollBug() } - cmd := &dashapi.BugUpdate{ + reply, _ := client.ReportingUpdate(&dashapi.BugUpdate{ ID: repFixed.ID, Status: dashapi.BugStatusOpen, FixCommits: []string{ns + "-patch0"}, ExtID: accessPrefix + "reporting-ext-id", Link: accessPrefix + "reporting-link", - } - reply := new(dashapi.BugUpdateReply) - client.expectOK(client.API("reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, true) buildFixing := testBuild(reportingIdx*10 + 2) buildFixing.Manager = build.Manager buildFixing.Commits = []string{ns + "-patch0"} - client.uploadBuild(buildFixing) + client.UploadBuild(buildFixing) noteBuildccessLevel(ns, buildFixing.ID) // Fixed bugs become visible up to the last reporting. finalLevel := config.Namespaces[ns]. @@ -230,42 +228,40 @@ func TestAccess(t *testing.T) { crashOpen.Report = []byte(accessPrefix + "report") crashOpen.ReproC = []byte(accessPrefix + "repro c") crashOpen.ReproSyz = []byte(accessPrefix + "repro syz") - client.reportCrash(crashOpen) - repOpen := reportAllBugs(c, 1)[0] + client.ReportCrash(crashOpen) + repOpen := client.pollBug() if reportingIdx != 0 { - c.expectTrue(client.updateBug(repOpen.ID, dashapi.BugStatusUpstream, "").OK) - repOpen = reportAllBugs(c, 1)[0] + client.updateBug(repOpen.ID, dashapi.BugStatusUpstream, "") + repOpen = client.pollBug() } noteBugAccessLevel(repOpen.ID, accessLevel) crashPatched := testCrashWithRepro(build, reportingIdx*10+1) - client.reportCrash(crashPatched) - repPatched := reportAllBugs(c, 1)[0] + client.ReportCrash(crashPatched) + repPatched := client.pollBug() if reportingIdx != 0 { - c.expectTrue(client.updateBug(repPatched.ID, dashapi.BugStatusUpstream, "").OK) - repPatched = reportAllBugs(c, 1)[0] + client.updateBug(repPatched.ID, dashapi.BugStatusUpstream, "") + repPatched = client.pollBug() } - cmd = &dashapi.BugUpdate{ + reply, _ = client.ReportingUpdate(&dashapi.BugUpdate{ ID: repPatched.ID, Status: dashapi.BugStatusOpen, FixCommits: []string{ns + "-patch0"}, ExtID: accessPrefix + "reporting-ext-id", Link: accessPrefix + "reporting-link", - } - reply = new(dashapi.BugUpdateReply) - client.expectOK(client.API("reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, true) // Patched bugs are also visible up to the last reporting. noteBugAccessLevel(repPatched.ID, finalLevel) crashDup := testCrashWithRepro(build, reportingIdx*10+2) - client.reportCrash(crashDup) - repDup := reportAllBugs(c, 1)[0] + client.ReportCrash(crashDup) + repDup := client.pollBug() if reportingIdx != 0 { - c.expectTrue(client.updateBug(repDup.ID, dashapi.BugStatusUpstream, "").OK) - repDup = reportAllBugs(c, 1)[0] + client.updateBug(repDup.ID, dashapi.BugStatusUpstream, "") + repDup = client.pollBug() } - c.expectTrue(client.updateBug(repDup.ID, dashapi.BugStatusDup, repOpen.ID).OK) + client.updateBug(repDup.ID, dashapi.BugStatusDup, repOpen.ID) noteBugAccessLevel(repDup.ID, accessLevel) } } diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go index f40a648ea..3c5be3ecc 100644 --- a/dashboard/app/app_test.go +++ b/dashboard/app/app_test.go @@ -225,23 +225,20 @@ func TestApp(t *testing.T) { c.expectOK(c.GET("/")) - c.expectFail("unknown api method", c.API(client1, key1, "unsupported_method", nil, nil)) - - ent := &dashapi.LogEntry{ - Name: "name", - Text: "text", - } - c.expectOK(c.API(client1, key1, "log_error", ent, nil)) + apiClient1 := c.makeClient(client1, key1, false) + apiClient2 := c.makeClient(client2, key2, false) + c.expectFail("unknown api method", apiClient1.Query("unsupported_method", nil, nil)) + c.client.LogError("name", "msg %s", "arg") build := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build, nil)) + c.client.UploadBuild(build) // Uploading the same build must be OK. - c.expectOK(c.API(client1, key1, "upload_build", build, nil)) + c.client.UploadBuild(build) // Some bad combinations of client/key. - c.expectFail("unauthorized", c.API(client1, "", "upload_build", build, nil)) - c.expectFail("unauthorized", c.API("unknown", key1, "upload_build", build, nil)) - c.expectFail("unauthorized", c.API(client1, key2, "upload_build", build, nil)) + c.expectFail("unauthorized", c.makeClient(client1, "", false).Query("upload_build", build, nil)) + c.expectFail("unauthorized", c.makeClient("unknown", key1, false).Query("upload_build", build, nil)) + c.expectFail("unauthorized", c.makeClient(client1, key2, false).Query("upload_build", build, nil)) crash1 := &dashapi.Crash{ BuildID: "build1", @@ -250,10 +247,10 @@ func TestApp(t *testing.T) { Log: []byte("log1"), Report: []byte("report1"), } - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) // Test that namespace isolation works. - c.expectFail("unknown build", c.API(client2, key2, "report_crash", crash1, nil)) + c.expectFail("unknown build", apiClient2.Query("report_crash", crash1, nil)) crash2 := &dashapi.Crash{ BuildID: "build1", @@ -265,7 +262,7 @@ func TestApp(t *testing.T) { ReproSyz: []byte("syz repro"), ReproC: []byte("c repro"), } - c.expectOK(c.API(client1, key1, "report_crash", crash2, nil)) + c.client.ReportCrash(crash2) // Provoke purgeOldCrashes. for i := 0; i < 30; i++ { @@ -276,28 +273,22 @@ func TestApp(t *testing.T) { Log: []byte(fmt.Sprintf("log%v", i)), Report: []byte(fmt.Sprintf("report%v", i)), } - c.expectOK(c.API(client1, key1, "report_crash", crash, nil)) + c.client.ReportCrash(crash) } cid := &dashapi.CrashID{ BuildID: "build1", Title: "title1", } - c.expectOK(c.API(client1, key1, "report_failed_repro", cid, nil)) + c.client.ReportFailedRepro(cid) - pr := &dashapi.PollBugsRequest{ - Type: "test", - } - resp := new(dashapi.PollBugsResponse) - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) + c.client.ReportingPollBugs("test") - cmd := &dashapi.BugUpdate{ + c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: "id", Status: dashapi.BugStatusOpen, ReproLevel: dashapi.ReproLevelC, - DupOf: "", - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, nil)) + }) } // Normal workflow: @@ -309,37 +300,35 @@ func testNeedRepro1(t *testing.T, crashCtor func(c *Ctx) *dashapi.Crash) { defer c.Close() crash1 := crashCtor(c) - resp := new(dashapi.ReportCrashResp) - c.expectOK(c.API(client1, key1, "report_crash", crash1, resp)) + resp, _ := c.client.ReportCrash(crash1) c.expectEQ(resp.NeedRepro, true) cid := testCrashID(crash1) - needReproResp := new(dashapi.NeedReproResp) - c.expectOK(c.API(client1, key1, "need_repro", cid, needReproResp)) - c.expectEQ(needReproResp.NeedRepro, true) + needRepro, _ := c.client.NeedRepro(cid) + c.expectEQ(needRepro, true) // Still need repro for this crash. - c.expectOK(c.API(client1, key1, "report_crash", crash1, resp)) + resp, _ = c.client.ReportCrash(crash1) c.expectEQ(resp.NeedRepro, true) - c.expectOK(c.API(client1, key1, "need_repro", cid, needReproResp)) - c.expectEQ(needReproResp.NeedRepro, true) + needRepro, _ = c.client.NeedRepro(cid) + c.expectEQ(needRepro, true) crash2 := new(dashapi.Crash) *crash2 = *crash1 crash2.ReproOpts = []byte("opts") crash2.ReproSyz = []byte("repro syz") - c.expectOK(c.API(client1, key1, "report_crash", crash2, resp)) + resp, _ = c.client.ReportCrash(crash2) c.expectEQ(resp.NeedRepro, true) - c.expectOK(c.API(client1, key1, "need_repro", cid, needReproResp)) - c.expectEQ(needReproResp.NeedRepro, true) + needRepro, _ = c.client.NeedRepro(cid) + c.expectEQ(needRepro, true) crash2.ReproC = []byte("repro C") - c.expectOK(c.API(client1, key1, "report_crash", crash2, resp)) + resp, _ = c.client.ReportCrash(crash2) c.expectEQ(resp.NeedRepro, false) - c.expectOK(c.API(client1, key1, "need_repro", cid, needReproResp)) - c.expectEQ(needReproResp.NeedRepro, false) + needRepro, _ = c.client.NeedRepro(cid) + c.expectEQ(needRepro, false) - c.expectOK(c.API(client1, key1, "report_crash", crash1, resp)) + resp, _ = c.client.ReportCrash(crash2) c.expectEQ(resp.NeedRepro, false) } @@ -357,14 +346,11 @@ func testNeedRepro2(t *testing.T, crashCtor func(c *Ctx) *dashapi.Crash) { crash1.ReproOpts = []byte("opts") crash1.ReproSyz = []byte("repro syz") crash1.ReproC = []byte("repro C") - resp := new(dashapi.ReportCrashResp) - c.expectOK(c.API(client1, key1, "report_crash", crash1, resp)) + resp, _ := c.client.ReportCrash(crash1) c.expectEQ(resp.NeedRepro, false) - cid := testCrashID(crash1) - needReproResp := new(dashapi.NeedReproResp) - c.expectOK(c.API(client1, key1, "need_repro", cid, needReproResp)) - c.expectEQ(needReproResp.NeedRepro, false) + needRepro, _ := c.client.NeedRepro(testCrashID(crash1)) + c.expectEQ(needRepro, false) } func TestNeedRepro2_normal(t *testing.T) { testNeedRepro2(t, normalCrash) } @@ -378,25 +364,20 @@ func testNeedRepro3(t *testing.T, crashCtor func(c *Ctx) *dashapi.Crash) { defer c.Close() crash1 := crashCtor(c) - resp := new(dashapi.ReportCrashResp) - cid := testCrashID(crash1) - needReproResp := new(dashapi.NeedReproResp) - for i := 0; i < maxReproPerBug; i++ { - c.expectOK(c.API(client1, key1, "report_crash", crash1, resp)) + resp, _ := c.client.ReportCrash(crash1) c.expectEQ(resp.NeedRepro, true) - c.expectOK(c.API(client1, key1, "need_repro", cid, needReproResp)) - c.expectEQ(needReproResp.NeedRepro, true) - - c.expectOK(c.API(client1, key1, "report_failed_repro", cid, nil)) + needRepro, _ := c.client.NeedRepro(testCrashID(crash1)) + c.expectEQ(needRepro, true) + c.client.ReportFailedRepro(testCrashID(crash1)) } - c.expectOK(c.API(client1, key1, "report_crash", crash1, resp)) + resp, _ := c.client.ReportCrash(crash1) c.expectEQ(resp.NeedRepro, false) - c.expectOK(c.API(client1, key1, "need_repro", cid, needReproResp)) - c.expectEQ(needReproResp.NeedRepro, false) + needRepro, _ := c.client.NeedRepro(testCrashID(crash1)) + c.expectEQ(needRepro, false) } func TestNeedRepro3_normal(t *testing.T) { testNeedRepro3(t, normalCrash) } @@ -412,23 +393,19 @@ func testNeedRepro4(t *testing.T, crashCtor func(c *Ctx) *dashapi.Crash) { crash1 := crashCtor(c) crash1.ReproOpts = []byte("opts") crash1.ReproSyz = []byte("repro syz") - resp := new(dashapi.ReportCrashResp) - cid := testCrashID(crash1) - needReproResp := new(dashapi.NeedReproResp) - for i := 0; i < maxReproPerBug-1; i++ { - c.expectOK(c.API(client1, key1, "report_crash", crash1, resp)) + resp, _ := c.client.ReportCrash(crash1) c.expectEQ(resp.NeedRepro, true) - c.expectOK(c.API(client1, key1, "need_repro", cid, needReproResp)) - c.expectEQ(needReproResp.NeedRepro, true) + needRepro, _ := c.client.NeedRepro(testCrashID(crash1)) + c.expectEQ(needRepro, true) } - c.expectOK(c.API(client1, key1, "report_crash", crash1, resp)) + resp, _ := c.client.ReportCrash(crash1) c.expectEQ(resp.NeedRepro, false) - c.expectOK(c.API(client1, key1, "need_repro", cid, needReproResp)) - c.expectEQ(needReproResp.NeedRepro, false) + needRepro, _ := c.client.NeedRepro(testCrashID(crash1)) + c.expectEQ(needRepro, false) } func TestNeedRepro4_normal(t *testing.T) { testNeedRepro4(t, normalCrash) } @@ -438,37 +415,18 @@ func TestNeedRepro4_closedRepro(t *testing.T) { testNeedRepro4(t, closedWithRepr func normalCrash(c *Ctx) *dashapi.Crash { build := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build, nil)) + c.client.UploadBuild(build) return testCrash(build, 1) } func dupCrash(c *Ctx) *dashapi.Crash { build := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build, nil)) - - crash1 := testCrash(build, 1) - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - + c.client.UploadBuild(build) + c.client.ReportCrash(testCrash(build, 1)) crash2 := testCrash(build, 2) - c.expectOK(c.API(client1, key1, "report_crash", crash2, nil)) - - pr := &dashapi.PollBugsRequest{ - Type: "test", - } - resp := new(dashapi.PollBugsResponse) - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) - c.expectEQ(len(resp.Reports), 2) - rep1 := resp.Reports[0] - rep2 := resp.Reports[1] - cmd := &dashapi.BugUpdate{ - ID: rep2.ID, - Status: dashapi.BugStatusDup, - DupOf: rep1.ID, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) - + c.client.ReportCrash(crash2) + reports := c.client.pollBugs(2) + c.client.updateBug(reports[1].ID, dashapi.BugStatusDup, reports[0].ID) return crash2 } @@ -482,30 +440,17 @@ func closedWithReproCrash(c *Ctx) *dashapi.Crash { func closedCrashImpl(c *Ctx, withRepro bool) *dashapi.Crash { build := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build, nil)) + c.client.UploadBuild(build) crash := testCrash(build, 1) if withRepro { crash.ReproC = []byte("repro C") } - resp := new(dashapi.ReportCrashResp) - c.expectOK(c.API(client1, key1, "report_crash", crash, resp)) + resp, _ := c.client.ReportCrash(crash) c.expectEQ(resp.NeedRepro, !withRepro) - pr := &dashapi.PollBugsRequest{ - Type: "test", - } - pollResp := new(dashapi.PollBugsResponse) - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, pollResp)) - c.expectEQ(len(pollResp.Reports), 1) - rep := pollResp.Reports[0] - cmd := &dashapi.BugUpdate{ - ID: rep.ID, - Status: dashapi.BugStatusInvalid, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) + rep := c.client.pollBug() + c.client.updateBug(rep.ID, dashapi.BugStatusInvalid, "") crash.ReproC = nil return crash diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go index bc321c07e..b31499863 100644 --- a/dashboard/app/email_test.go +++ b/dashboard/app/email_test.go @@ -10,7 +10,6 @@ import ( "strings" "testing" - "github.com/google/syzkaller/dashboard/dashapi" "github.com/google/syzkaller/pkg/email" ) @@ -19,11 +18,11 @@ func TestEmailReport(t *testing.T) { defer c.Close() build := testBuild(1) - c.expectOK(c.API(client2, key2, "upload_build", build, nil)) + c.client2.UploadBuild(build) crash := testCrash(build, 1) crash.Maintainers = []string{`"Foo Bar" `, `bar@foo.com`} - c.expectOK(c.API(client2, key2, "report_crash", crash, nil)) + c.client2.ReportCrash(crash) // Report the crash over email and check all fields. var sender0, extBugID0, body0 string @@ -120,7 +119,7 @@ For more options, visit https://groups.google.com/d/optout. crash.ReproOpts = []byte("repro opts") crash.ReproSyz = []byte("getpid()") syzRepro := []byte(fmt.Sprintf("%s#%s\n%s", syzReproPrefix, crash.ReproOpts, crash.ReproSyz)) - c.expectOK(c.API(client2, key2, "report_crash", crash, nil)) + c.client2.ReportCrash(crash) { c.expectOK(c.GET("/email_poll")) @@ -249,11 +248,11 @@ Content-Type: text/plain // Now upload a C reproducer. build2 := testBuild(2) build2.KernelCommitTitle = "a really long title, longer than 80 chars, really long-long-long-long-long-long title" - c.expectOK(c.API(client2, key2, "upload_build", build2, nil)) + c.client2.UploadBuild(build2) crash.BuildID = build2.ID crash.ReproC = []byte("int main() {}") crash.Maintainers = []string{"\"qux\" "} - c.expectOK(c.API(client2, key2, "report_crash", crash, nil)) + c.client2.ReportCrash(crash) { c.expectOK(c.GET("/email_poll")) @@ -334,24 +333,22 @@ unknown command "bad-command" c.expectEQ(len(c.emailSink), 0) // Check that the commit is now passed to builders. - builderPollReq := &dashapi.BuilderPollReq{build.Manager} - builderPollResp := new(dashapi.BuilderPollResp) - c.expectOK(c.API(client2, key2, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ := c.client2.BuilderPoll(build.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 1) c.expectEQ(builderPollResp.PendingCommits[0], "some: commit title") build3 := testBuild(3) build3.Manager = build.Manager build3.Commits = []string{"some: commit title"} - c.expectOK(c.API(client2, key2, "upload_build", build3, nil)) + c.client2.UploadBuild(build3) build4 := testBuild(4) build4.Manager = build2.Manager build4.Commits = []string{"some: commit title"} - c.expectOK(c.API(client2, key2, "upload_build", build4, nil)) + c.client2.UploadBuild(build4) // New crash must produce new bug in the first reporting. - c.expectOK(c.API(client2, key2, "report_crash", crash, nil)) + c.client2.ReportCrash(crash) { c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 1) @@ -369,10 +366,10 @@ func TestEmailNoMaintainers(t *testing.T) { defer c.Close() build := testBuild(1) - c.expectOK(c.API(client2, key2, "upload_build", build, nil)) + c.client2.UploadBuild(build) crash := testCrash(build, 1) - c.expectOK(c.API(client2, key2, "report_crash", crash, nil)) + c.client2.ReportCrash(crash) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 1) @@ -400,15 +397,15 @@ func TestEmailDup(t *testing.T) { defer c.Close() build := testBuild(1) - c.expectOK(c.API(client2, key2, "upload_build", build, nil)) + c.client2.UploadBuild(build) crash1 := testCrash(build, 1) crash1.Title = "BUG: slightly more elaborate title" - c.expectOK(c.API(client2, key2, "report_crash", crash1, nil)) + c.client2.ReportCrash(crash1) crash2 := testCrash(build, 2) crash1.Title = "KASAN: another title" - c.expectOK(c.API(client2, key2, "report_crash", crash2, nil)) + c.client2.ReportCrash(crash2) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 2) @@ -422,7 +419,7 @@ func TestEmailDup(t *testing.T) { // Second crash happens again crash2.ReproC = []byte("int main() {}") - c.expectOK(c.API(client2, key2, "report_crash", crash2, nil)) + c.client2.ReportCrash(crash2) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 0) @@ -430,7 +427,7 @@ func TestEmailDup(t *testing.T) { c.incomingEmail(msg1.Sender, "#syz invalid") // New crash must produce new bug in the first reporting. - c.expectOK(c.API(client2, key2, "report_crash", crash2, nil)) + c.client2.ReportCrash(crash2) { c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 1) @@ -444,15 +441,15 @@ func TestEmailUndup(t *testing.T) { defer c.Close() build := testBuild(1) - c.expectOK(c.API(client2, key2, "upload_build", build, nil)) + c.client2.UploadBuild(build) crash1 := testCrash(build, 1) crash1.Title = "BUG: slightly more elaborate title" - c.expectOK(c.API(client2, key2, "report_crash", crash1, nil)) + c.client2.ReportCrash(crash1) crash2 := testCrash(build, 2) crash1.Title = "KASAN: another title" - c.expectOK(c.API(client2, key2, "report_crash", crash2, nil)) + c.client2.ReportCrash(crash2) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 2) @@ -471,7 +468,7 @@ func TestEmailUndup(t *testing.T) { // Now close the original bug, and check that new crashes for the dup does not create bugs. c.incomingEmail(msg1.Sender, "#syz invalid") - c.expectOK(c.API(client2, key2, "report_crash", crash2, nil)) + c.client2.ReportCrash(crash2) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 0) } diff --git a/dashboard/app/fix_test.go b/dashboard/app/fix_test.go index e75d7f61e..4774b22b1 100644 --- a/dashboard/app/fix_test.go +++ b/dashboard/app/fix_test.go @@ -18,87 +18,69 @@ func TestFixBasic(t *testing.T) { defer c.Close() build1 := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build1, nil)) + c.client.UploadBuild(build1) crash1 := testCrash(build1, 1) - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) - builderPollReq := &dashapi.BuilderPollReq{build1.Manager} - builderPollResp := new(dashapi.BuilderPollResp) - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ := c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) - cid := testCrashID(crash1) - needReproResp := new(dashapi.NeedReproResp) - c.expectOK(c.API(client1, key1, "need_repro", cid, needReproResp)) - c.expectEQ(needReproResp.NeedRepro, true) + needRepro, _ := c.client.NeedRepro(testCrashID(crash1)) + c.expectEQ(needRepro, true) - reports := reportAllBugs(c, 1) - rep := reports[0] + rep := c.client.pollBug() // Specify fixing commit for the bug. - cmd := &dashapi.BugUpdate{ + reply, _ := c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep.ID, Status: dashapi.BugStatusOpen, FixCommits: []string{"foo: fix the crash"}, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, true) // Don't need repro once there are fixing commits. - c.expectOK(c.API(client1, key1, "need_repro", cid, needReproResp)) - c.expectEQ(needReproResp.NeedRepro, false) + needRepro, _ = c.client.NeedRepro(testCrashID(crash1)) + c.expectEQ(needRepro, false) // Check that the commit is now passed to builders. - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 1) c.expectEQ(builderPollResp.PendingCommits[0], "foo: fix the crash") // Patches must not be reset on other actions. - cmd = &dashapi.BugUpdate{ - ID: rep.ID, - Status: dashapi.BugStatusOpen, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) + c.client.updateBug(rep.ID, dashapi.BugStatusOpen, "") // Upstream commands must fail if patches are already present. // Right course of action is unclear in this situation, // so this test merely documents the current behavior. - cmd = &dashapi.BugUpdate{ + reply, _ = c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep.ID, Status: dashapi.BugStatusUpstream, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, false) - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - reportAllBugs(c, 0) + c.client.ReportCrash(crash1) + c.client.pollBugs(0) // Upload another build with the commit present. build2 := testBuild(2) build2.Manager = build1.Manager build2.Commits = []string{"foo: fix the crash"} - c.expectOK(c.API(client1, key1, "upload_build", build2, nil)) + c.client.UploadBuild(build2) // Check that the commit is now not passed to this builder. - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) // Ensure that a new crash creates a new bug (the old one must be marked as fixed). - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - reports = reportAllBugs(c, 1) - c.expectEQ(reports[0].Title, "title1 (2)") + c.client.ReportCrash(crash1) + rep2 := c.client.pollBug() + c.expectEQ(rep2.Title, "title1 (2)") // Regression test: previously upstreamming failed because the new bug had fixing commits. - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - cmd = &dashapi.BugUpdate{ - ID: reports[0].ID, - Status: dashapi.BugStatusUpstream, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) + c.client.ReportCrash(crash1) + c.client.updateBug(rep2.ID, dashapi.BugStatusUpstream, "") } // Test bug that is fixed by 2 commits. @@ -107,31 +89,26 @@ func TestFixedByTwoCommits(t *testing.T) { defer c.Close() build1 := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build1, nil)) + c.client.UploadBuild(build1) crash1 := testCrash(build1, 1) - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) - builderPollReq := &dashapi.BuilderPollReq{build1.Manager} - builderPollResp := new(dashapi.BuilderPollResp) - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ := c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) - reports := reportAllBugs(c, 1) - rep := reports[0] + rep := c.client.pollBug() // Specify fixing commit for the bug. - cmd := &dashapi.BugUpdate{ + reply, _ := c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep.ID, Status: dashapi.BugStatusOpen, FixCommits: []string{"bar: prepare for fixing", "\"foo: fix the crash\""}, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, true) // Check that the commit is now passed to builders. - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 2) c.expectEQ(builderPollResp.PendingCommits[0], "bar: prepare for fixing") c.expectEQ(builderPollResp.PendingCommits[1], "foo: fix the crash") @@ -140,31 +117,31 @@ func TestFixedByTwoCommits(t *testing.T) { build2 := testBuild(2) build2.Manager = build1.Manager build2.Commits = []string{"bar: prepare for fixing"} - c.expectOK(c.API(client1, key1, "upload_build", build2, nil)) + c.client.UploadBuild(build2) // Check that it has not fixed the bug. - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 2) c.expectEQ(builderPollResp.PendingCommits[0], "bar: prepare for fixing") c.expectEQ(builderPollResp.PendingCommits[1], "foo: fix the crash") - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - reportAllBugs(c, 0) + c.client.ReportCrash(crash1) + c.client.pollBugs(0) // Now upload build with both commits. build3 := testBuild(3) build3.Manager = build1.Manager build3.Commits = []string{"foo: fix the crash", "bar: prepare for fixing"} - c.expectOK(c.API(client1, key1, "upload_build", build3, nil)) + c.client.UploadBuild(build3) // Check that the commit is now not passed to this builder. - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) // Ensure that a new crash creates a new bug (the old one must be marked as fixed). - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - reports = reportAllBugs(c, 1) - c.expectEQ(reports[0].Title, "title1 (2)") + c.client.ReportCrash(crash1) + rep2 := c.client.pollBug() + c.expectEQ(rep2.Title, "title1 (2)") } // A bug is marked as fixed by one commit and then remarked as fixed by another. @@ -173,38 +150,32 @@ func TestReFixed(t *testing.T) { defer c.Close() build1 := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build1, nil)) + c.client.UploadBuild(build1) crash1 := testCrash(build1, 1) - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) - builderPollReq := &dashapi.BuilderPollReq{build1.Manager} - builderPollResp := new(dashapi.BuilderPollResp) - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ := c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) - reports := reportAllBugs(c, 1) - rep := reports[0] + rep := c.client.pollBug() // Specify fixing commit for the bug. - cmd := &dashapi.BugUpdate{ + reply, _ := c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep.ID, Status: dashapi.BugStatusOpen, FixCommits: []string{"a wrong one"}, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, true) - cmd = &dashapi.BugUpdate{ + reply, _ = c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep.ID, Status: dashapi.BugStatusOpen, FixCommits: []string{"the right one"}, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, true) - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 1) c.expectEQ(builderPollResp.PendingCommits[0], "the right one") @@ -212,24 +183,24 @@ func TestReFixed(t *testing.T) { build2 := testBuild(2) build2.Manager = build1.Manager build2.Commits = []string{"a wrong one"} - c.expectOK(c.API(client1, key1, "upload_build", build2, nil)) + c.client.UploadBuild(build2) // Check that it has not fixed the bug. - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 1) c.expectEQ(builderPollResp.PendingCommits[0], "the right one") - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - reportAllBugs(c, 0) + c.client.ReportCrash(crash1) + c.client.pollBugs(0) // Now upload build with the right commit. build3 := testBuild(3) build3.Manager = build1.Manager build3.Commits = []string{"the right one"} - c.expectOK(c.API(client1, key1, "upload_build", build3, nil)) + c.client.UploadBuild(build3) // Check that the commit is now not passed to this builder. - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) } @@ -239,41 +210,34 @@ func TestFixTwoManagers(t *testing.T) { defer c.Close() build1 := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build1, nil)) + c.client.UploadBuild(build1) crash1 := testCrash(build1, 1) - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) - builderPollReq := &dashapi.BuilderPollReq{build1.Manager} - builderPollResp := new(dashapi.BuilderPollResp) - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ := c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) - reports := reportAllBugs(c, 1) - rep := reports[0] + rep := c.client.pollBug() // Specify fixing commit for the bug. - cmd := &dashapi.BugUpdate{ + reply, _ := c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep.ID, Status: dashapi.BugStatusOpen, FixCommits: []string{"foo: fix the crash"}, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, true) // Now the second manager appears. build2 := testBuild(2) - c.expectOK(c.API(client1, key1, "upload_build", build2, nil)) + c.client.UploadBuild(build2) // Check that the commit is now passed to builders. - builderPollReq = &dashapi.BuilderPollReq{build1.Manager} - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 1) c.expectEQ(builderPollResp.PendingCommits[0], "foo: fix the crash") - builderPollReq = &dashapi.BuilderPollReq{build2.Manager} - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build2.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 1) c.expectEQ(builderPollResp.PendingCommits[0], "foo: fix the crash") @@ -281,37 +245,34 @@ func TestFixTwoManagers(t *testing.T) { build3 := testBuild(3) build3.Manager = build1.Manager build3.Commits = []string{"foo: fix the crash"} - c.expectOK(c.API(client1, key1, "upload_build", build3, nil)) + c.client.UploadBuild(build3) // Check that the commit is now not passed to this builder. - builderPollReq = &dashapi.BuilderPollReq{build1.Manager} - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) // But still passed to another. - builderPollReq = &dashapi.BuilderPollReq{build2.Manager} - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build2.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 1) c.expectEQ(builderPollResp.PendingCommits[0], "foo: fix the crash") // Check that the bug is still open. - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - reportAllBugs(c, 0) + c.client.ReportCrash(crash1) + c.client.pollBugs(0) // Now the second manager picks up the commit. build4 := testBuild(4) build4.Manager = build2.Manager build4.Commits = []string{"foo: fix the crash"} - c.expectOK(c.API(client1, key1, "upload_build", build4, nil)) + c.client.UploadBuild(build4) // Now the bug must be fixed. - builderPollReq = &dashapi.BuilderPollReq{build2.Manager} - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build2.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - reports = reportAllBugs(c, 1) - c.expectEQ(reports[0].Title, "title1 (2)") + c.client.ReportCrash(crash1) + rep2 := c.client.pollBug() + c.expectEQ(rep2.Title, "title1 (2)") } func TestReFixedTwoManagers(t *testing.T) { @@ -319,60 +280,51 @@ func TestReFixedTwoManagers(t *testing.T) { defer c.Close() build1 := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build1, nil)) + c.client.UploadBuild(build1) crash1 := testCrash(build1, 1) - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) - builderPollReq := &dashapi.BuilderPollReq{build1.Manager} - builderPollResp := new(dashapi.BuilderPollResp) - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ := c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) - reports := reportAllBugs(c, 1) - rep := reports[0] + rep := c.client.pollBug() // Specify fixing commit for the bug. - cmd := &dashapi.BugUpdate{ + reply, _ := c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep.ID, Status: dashapi.BugStatusOpen, FixCommits: []string{"foo: fix the crash"}, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, true) // Now the second manager appears. build2 := testBuild(2) - c.expectOK(c.API(client1, key1, "upload_build", build2, nil)) + c.client.UploadBuild(build2) // Now first manager picks up the commit. build3 := testBuild(3) build3.Manager = build1.Manager build3.Commits = []string{"foo: fix the crash"} - c.expectOK(c.API(client1, key1, "upload_build", build3, nil)) + c.client.UploadBuild(build3) - builderPollReq = &dashapi.BuilderPollReq{build1.Manager} - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) // Now we change the fixing commit. - cmd = &dashapi.BugUpdate{ + reply, _ = c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep.ID, Status: dashapi.BugStatusOpen, FixCommits: []string{"the right one"}, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, true) // Now it must again appear on both managers. - builderPollReq = &dashapi.BuilderPollReq{build1.Manager} - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 1) c.expectEQ(builderPollResp.PendingCommits[0], "the right one") - builderPollReq = &dashapi.BuilderPollReq{build2.Manager} - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 1) c.expectEQ(builderPollResp.PendingCommits[0], "the right one") @@ -380,35 +332,33 @@ func TestReFixedTwoManagers(t *testing.T) { build4 := testBuild(4) build4.Manager = build2.Manager build4.Commits = []string{"the right one"} - c.expectOK(c.API(client1, key1, "upload_build", build4, nil)) + c.client.UploadBuild(build4) // The bug must be still open. - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - reportAllBugs(c, 0) + c.client.ReportCrash(crash1) + c.client.pollBugs(0) // Specify fixing commit again, but it's the same one as before, so nothing changed. - cmd = &dashapi.BugUpdate{ + reply, _ = c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep.ID, Status: dashapi.BugStatusOpen, FixCommits: []string{"the right one"}, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, true) // Now the first manager picks up the second commit. build5 := testBuild(5) build5.Manager = build1.Manager build5.Commits = []string{"the right one"} - c.expectOK(c.API(client1, key1, "upload_build", build5, nil)) + c.client.UploadBuild(build5) // Now the bug must be fixed. - builderPollReq = &dashapi.BuilderPollReq{build1.Manager} - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - reports = reportAllBugs(c, 1) - c.expectEQ(reports[0].Title, "title1 (2)") + c.client.ReportCrash(crash1) + rep2 := c.client.pollBug() + c.expectEQ(rep2.Title, "title1 (2)") } // TestFixedWithCommitTags tests fixing of bugs with Reported-by commit tags. @@ -417,52 +367,46 @@ func TestFixedWithCommitTags(t *testing.T) { defer c.Close() build1 := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build1, nil)) + c.client.UploadBuild(build1) build2 := testBuild(2) - c.expectOK(c.API(client1, key1, "upload_build", build2, nil)) + c.client.UploadBuild(build2) crash1 := testCrash(build1, 1) - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) - rep := reportAllBugs(c, 1)[0] + rep := c.client.pollBug() // Upload build with 2 fixing commits for this bug. build1.FixCommits = []dashapi.FixCommit{{"fix commit 1", rep.ID}, {"fix commit 2", rep.ID}} - c.expectOK(c.API(client1, key1, "upload_build", build1, nil)) + c.client.UploadBuild(build1) // Now the commits must be associated with the bug and the second // manager must get them as pending. - builderPollReq := &dashapi.BuilderPollReq{build2.Manager} - builderPollResp := new(dashapi.BuilderPollResp) - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ := c.client.BuilderPoll(build2.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 2) c.expectEQ(builderPollResp.PendingCommits[0], "fix commit 1") c.expectEQ(builderPollResp.PendingCommits[1], "fix commit 2") // The first manager must not get them. - builderPollReq = &dashapi.BuilderPollReq{build1.Manager} - builderPollResp = new(dashapi.BuilderPollResp) - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build1.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) // The bug is still not fixed. - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - reportAllBugs(c, 0) + c.client.ReportCrash(crash1) + c.client.pollBugs(0) // Now the second manager reports the same commits. // This must close the bug. build2.FixCommits = build1.FixCommits - c.expectOK(c.API(client1, key1, "upload_build", build2, nil)) + c.client.UploadBuild(build2) // Commits must not be passed to managers. - builderPollReq = &dashapi.BuilderPollReq{build2.Manager} - builderPollResp = new(dashapi.BuilderPollResp) - c.expectOK(c.API(client1, key1, "builder_poll", builderPollReq, builderPollResp)) + builderPollResp, _ = c.client.BuilderPoll(build2.Manager) c.expectEQ(len(builderPollResp.PendingCommits), 0) // Ensure that a new crash creates a new bug. - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - rep = reportAllBugs(c, 1)[0] - c.expectEQ(rep.Title, "title1 (2)") + c.client.ReportCrash(crash1) + rep2 := c.client.pollBug() + c.expectEQ(rep2.Title, "title1 (2)") } diff --git a/dashboard/app/jobs_test.go b/dashboard/app/jobs_test.go index bffaf07a5..797b4eaa0 100644 --- a/dashboard/app/jobs_test.go +++ b/dashboard/app/jobs_test.go @@ -20,7 +20,7 @@ func TestJob(t *testing.T) { defer c.Close() build := testBuild(1) - c.expectOK(c.API(client2, key2, "upload_build", build, nil)) + c.client2.UploadBuild(build) patch := `--- a/mm/kasan/kasan.c +++ b/mm/kasan/kasan.c @@ -31,7 +31,7 @@ func TestJob(t *testing.T) { // Report crash without repro, check that test requests are not accepted. crash := testCrash(build, 1) crash.Maintainers = []string{"maintainer@kernel.org"} - c.expectOK(c.API(client2, key2, "report_crash", crash, nil)) + c.client2.ReportCrash(crash) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 1) @@ -57,7 +57,7 @@ func TestJob(t *testing.T) { crash.ReproOpts = []byte("repro opts") crash.ReproSyz = []byte("repro syz") crash.ReproC = []byte("repro C") - c.expectOK(c.API(client2, key2, "report_crash", crash, nil)) + c.client2.ReportCrash(crash) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 1) @@ -82,8 +82,7 @@ func TestJob(t *testing.T) { EmailOptFrom("\"foo\" ")) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 0) - pollResp := new(dashapi.JobPollResp) - c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp)) + pollResp, _ := c.client2.JobPoll([]string{build.Manager}) c.expectEQ(pollResp.ID, "") c.incomingEmail(sender, "#syz test: git://git.git/git.git kernel-branch\n"+patch, @@ -99,9 +98,9 @@ func TestJob(t *testing.T) { c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 0) - c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{"foobar"}}, pollResp)) + pollResp, _ = c.client2.JobPoll([]string{"foobar"}) c.expectEQ(pollResp.ID, "") - c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp)) + pollResp, _ = c.client2.JobPoll([]string{build.Manager}) c.expectEQ(pollResp.ID != "", true) c.expectEQ(pollResp.Manager, build.Manager) c.expectEQ(pollResp.KernelRepo, "git://git.git/git.git") @@ -113,8 +112,7 @@ func TestJob(t *testing.T) { c.expectEQ(pollResp.ReproSyz, []byte("repro syz")) c.expectEQ(pollResp.ReproC, []byte("repro C")) - pollResp2 := new(dashapi.JobPollResp) - c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp2)) + pollResp2, _ := c.client2.JobPoll([]string{build.Manager}) c.expectEQ(pollResp2, pollResp) jobDoneReq := &dashapi.JobDoneReq{ @@ -124,7 +122,7 @@ func TestJob(t *testing.T) { CrashLog: []byte("test crash log"), CrashReport: []byte("test crash report"), } - c.expectOK(c.API(client2, key2, "job_done", jobDoneReq, nil)) + c.client2.JobDone(jobDoneReq) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 1) @@ -165,13 +163,13 @@ patch: %[1]v // Testing fails with an error. c.incomingEmail(sender, "#syz test: git://git.git/git.git kernel-branch\n"+patch, EmailOptMessageID(2)) - c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp)) + pollResp, _ = c.client2.JobPoll([]string{build.Manager}) jobDoneReq = &dashapi.JobDoneReq{ ID: pollResp.ID, Build: *build, Error: []byte("failed to apply patch"), } - c.expectOK(c.API(client2, key2, "job_done", jobDoneReq, nil)) + c.client2.JobDone(jobDoneReq) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 1) { @@ -205,13 +203,13 @@ patch: %[1]v // Testing fails with a huge error that can't be inlined in email. c.incomingEmail(sender, "#syz test: git://git.git/git.git kernel-branch\n"+patch, EmailOptMessageID(3)) - c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp)) + pollResp, _ = c.client2.JobPoll([]string{build.Manager}) jobDoneReq = &dashapi.JobDoneReq{ ID: pollResp.ID, Build: *build, Error: bytes.Repeat([]byte{'a', 'b', 'c'}, (maxInlineError+100)/3), } - c.expectOK(c.API(client2, key2, "job_done", jobDoneReq, nil)) + c.client2.JobDone(jobDoneReq) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 1) { @@ -250,12 +248,12 @@ patch: %[3]v } c.incomingEmail(sender, "#syz test: git://git.git/git.git kernel-branch\n"+patch, EmailOptMessageID(4)) - c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp)) + pollResp, _ = c.client2.JobPoll([]string{build.Manager}) jobDoneReq = &dashapi.JobDoneReq{ ID: pollResp.ID, Build: *build, } - c.expectOK(c.API(client2, key2, "job_done", jobDoneReq, nil)) + c.client2.JobDone(jobDoneReq) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 1) { @@ -287,7 +285,7 @@ Note: testing is done by a robot and is best-effort only. c.checkURLContents(kernelConfigLink, build.KernelConfig) } - c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp)) + pollResp, _ = c.client2.JobPoll([]string{build.Manager}) c.expectEQ(pollResp.ID, "") } @@ -297,12 +295,12 @@ func TestJobWithoutPatch(t *testing.T) { defer c.Close() build := testBuild(1) - c.expectOK(c.API(client2, key2, "upload_build", build, nil)) + c.client2.UploadBuild(build) crash := testCrash(build, 1) crash.ReproOpts = []byte("repro opts") crash.ReproSyz = []byte("repro syz") - c.expectOK(c.API(client2, key2, "report_crash", crash, nil)) + c.client2.ReportCrash(crash) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 1) @@ -313,8 +311,7 @@ func TestJobWithoutPatch(t *testing.T) { } c.incomingEmail(sender, "#syz test: git://mygit.com/git.git 5e6a2eea\n", EmailOptMessageID(1)) - pollResp := new(dashapi.JobPollResp) - c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp)) + pollResp, _ := c.client2.JobPoll([]string{build.Manager}) testBuild := testBuild(2) testBuild.KernelRepo = "git://mygit.com/git.git" testBuild.KernelBranch = "" @@ -323,7 +320,7 @@ func TestJobWithoutPatch(t *testing.T) { ID: pollResp.ID, Build: *testBuild, } - c.expectOK(c.API(client2, key2, "job_done", jobDoneReq, nil)) + c.client2.JobDone(jobDoneReq) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 1) { @@ -352,7 +349,7 @@ Note: testing is done by a robot and is best-effort only. c.checkURLContents(kernelConfigLink, testBuild.KernelConfig) } - c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp)) + pollResp, _ = c.client2.JobPoll([]string{build.Manager}) c.expectEQ(pollResp.ID, "") } @@ -363,11 +360,11 @@ func TestJobRestrictedManager(t *testing.T) { build := testBuild(1) build.Manager = "restricted-manager" - c.expectOK(c.API(client2, key2, "upload_build", build, nil)) + c.client2.UploadBuild(build) crash := testCrash(build, 1) crash.ReproSyz = []byte("repro syz") - c.expectOK(c.API(client2, key2, "report_crash", crash, nil)) + c.client2.ReportCrash(crash) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 1) @@ -376,13 +373,12 @@ func TestJobRestrictedManager(t *testing.T) { // Testing on a wrong repo must fail and no test jobs passed to manager. c.incomingEmail(sender, "#syz test: git://mygit.com/git.git master\n", EmailOptMessageID(1)) c.expectEQ(strings.Contains((<-c.emailSink).Body, "you should test only on restricted.git"), true) - pollResp := new(dashapi.JobPollResp) - c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp)) + pollResp, _ := c.client2.JobPoll([]string{build.Manager}) c.expectEQ(pollResp.ID, "") // Testing on the right repo must succeed. c.incomingEmail(sender, "#syz test: git://restricted.git/restricted.git master\n", EmailOptMessageID(2)) - c.expectOK(c.API(client2, key2, "job_poll", &dashapi.JobPollReq{[]string{build.Manager}}, pollResp)) + pollResp, _ = c.client2.JobPoll([]string{build.Manager}) c.expectEQ(pollResp.ID != "", true) c.expectEQ(pollResp.Manager, build.Manager) c.expectEQ(pollResp.KernelRepo, "git://restricted.git/restricted.git") diff --git a/dashboard/app/reporting_test.go b/dashboard/app/reporting_test.go index 80ddea92b..4d13fd0d5 100644 --- a/dashboard/app/reporting_test.go +++ b/dashboard/app/reporting_test.go @@ -6,7 +6,6 @@ package dash import ( - "fmt" "testing" "time" @@ -18,7 +17,7 @@ func TestReportBug(t *testing.T) { defer c.Close() build := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build, nil)) + c.client.UploadBuild(build) crash1 := &dashapi.Crash{ BuildID: "build1", @@ -27,19 +26,14 @@ func TestReportBug(t *testing.T) { Log: []byte("log1"), Report: []byte("report1"), } - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) // Must get no reports for "unknown" type. - pr := &dashapi.PollBugsRequest{ - Type: "unknown", - } - resp := new(dashapi.PollBugsResponse) - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) + resp, _ := c.client.ReportingPollBugs("unknown") c.expectEQ(len(resp.Reports), 0) // Must get a proper report for "test" type. - pr.Type = "test" - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) + resp, _ = c.client.ReportingPollBugs("test") c.expectEQ(len(resp.Reports), 1) rep := resp.Reports[0] if rep.ID == "" { @@ -73,66 +67,51 @@ func TestReportBug(t *testing.T) { c.expectEQ(rep, want) // Since we did not update bug status yet, should get the same report again. - reports := reportAllBugs(c, 1) - c.expectEQ(reports[0], want) + c.expectEQ(c.client.pollBug(), want) // Now add syz repro and check that we get another bug report. crash1.ReproOpts = []byte("some opts") crash1.ReproSyz = []byte("getpid()") want.First = false want.ReproSyz = []byte(syzReproPrefix + "#some opts\ngetpid()") - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) - reports = reportAllBugs(c, 1) - if want.CrashID == reports[0].CrashID { + c.client.ReportCrash(crash1) + rep1 := c.client.pollBug() + if want.CrashID == rep1.CrashID { t.Fatal("get the same CrashID for new crash") } _, dbCrash, _ = c.loadBug(rep.ID) - want.CrashID = reports[0].CrashID + want.CrashID = rep1.CrashID want.NumCrashes = 2 want.ReproSyzLink = externalLink(c.ctx, textReproSyz, dbCrash.ReproSyz) want.LogLink = externalLink(c.ctx, textCrashLog, dbCrash.Log) want.ReportLink = externalLink(c.ctx, textCrashReport, dbCrash.Report) - c.expectEQ(reports[0], want) + c.expectEQ(rep1, want) - cmd := &dashapi.BugUpdate{ + reply, _ := c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep.ID, Status: dashapi.BugStatusOpen, ReproLevel: dashapi.ReproLevelSyz, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, true) // After bug update should not get the report again. - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) - c.expectEQ(len(resp.Reports), 0) + c.client.pollBugs(0) // Now close the bug in the first reporting. - cmd = &dashapi.BugUpdate{ - ID: rep.ID, - Status: dashapi.BugStatusUpstream, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) + c.client.updateBug(rep.ID, dashapi.BugStatusUpstream, "") // Check that bug updates for the first reporting fail now. - cmd = &dashapi.BugUpdate{ - ID: rep.ID, - Status: dashapi.BugStatusOpen, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + reply, _ = c.client.ReportingUpdate(&dashapi.BugUpdate{ID: rep.ID, Status: dashapi.BugStatusOpen}) c.expectEQ(reply.OK, false) // Report another crash with syz repro for this bug, // ensure that we still report the original crash in the next reporting. // That's what we've upstreammed, it's bad to switch crashes without reason. crash1.Report = []byte("report2") - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) // Check that we get the report in the second reporting. - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) - c.expectEQ(len(resp.Reports), 1) - rep2 := resp.Reports[0] + rep2 := c.client.pollBug() if rep2.ID == "" || rep2.ID == rep.ID { t.Fatalf("bad report ID: %q", rep2.ID) } @@ -143,11 +122,10 @@ func TestReportBug(t *testing.T) { c.expectEQ(rep2, want) // Check that that we can't upstream the bug in the final reporting. - cmd = &dashapi.BugUpdate{ + reply, _ = c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep2.ID, Status: dashapi.BugStatusUpstream, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, false) } @@ -156,60 +134,37 @@ func TestInvalidBug(t *testing.T) { defer c.Close() build := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build, nil)) + c.client.UploadBuild(build) - crash1 := testCrash(build, 1) - crash1.ReproC = []byte("int main() {}") - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + crash1 := testCrashWithRepro(build, 1) + c.client.ReportCrash(crash1) - pr := &dashapi.PollBugsRequest{ - Type: "test", - } - resp := new(dashapi.PollBugsResponse) - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) - c.expectEQ(len(resp.Reports), 1) - rep := resp.Reports[0] + rep := c.client.pollBug() c.expectEQ(rep.Title, "title1") - cmd := &dashapi.BugUpdate{ + reply, _ := c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep.ID, Status: dashapi.BugStatusOpen, ReproLevel: dashapi.ReproLevelC, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, true) { - req := &dashapi.PollClosedRequest{ - IDs: []string{rep.ID, "foobar"}, - } - resp := new(dashapi.PollClosedResponse) - c.expectOK(c.API(client1, key1, "reporting_poll_closed", req, resp)) - c.expectEQ(len(resp.IDs), 0) + closed, _ := c.client.ReportingPollClosed([]string{rep.ID, "foobar"}) + c.expectEQ(len(closed), 0) } // Mark the bug as invalid. - cmd = &dashapi.BugUpdate{ - ID: rep.ID, - Status: dashapi.BugStatusInvalid, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) + c.client.updateBug(rep.ID, dashapi.BugStatusInvalid, "") { - req := &dashapi.PollClosedRequest{ - IDs: []string{rep.ID, "foobar"}, - } - resp := new(dashapi.PollClosedResponse) - c.expectOK(c.API(client1, key1, "reporting_poll_closed", req, resp)) - c.expectEQ(len(resp.IDs), 1) - c.expectEQ(resp.IDs[0], rep.ID) + closed, _ := c.client.ReportingPollClosed([]string{rep.ID, "foobar"}) + c.expectEQ(len(closed), 1) + c.expectEQ(closed[0], rep.ID) } // Now it should not be reported in either reporting. - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) - c.expectEQ(len(resp.Reports), 0) + c.client.pollBugs(0) // Now a similar crash happens again. crash2 := &dashapi.Crash{ @@ -219,12 +174,10 @@ func TestInvalidBug(t *testing.T) { Report: []byte("report2"), ReproC: []byte("int main() { return 1; }"), } - c.expectOK(c.API(client1, key1, "report_crash", crash2, nil)) + c.client.ReportCrash(crash2) // Now it should be reported again. - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) - c.expectEQ(len(resp.Reports), 1) - rep = resp.Reports[0] + rep = c.client.pollBug() if rep.ID == "" { t.Fatalf("empty report ID") } @@ -255,12 +208,7 @@ func TestInvalidBug(t *testing.T) { HappenedOn: []string{"repo1/branch1"}, } c.expectEQ(rep, want) - - cid := &dashapi.CrashID{ - BuildID: build.ID, - Title: crash1.Title, - } - c.expectOK(c.API(client1, key1, "report_failed_repro", cid, nil)) + c.client.ReportFailedRepro(testCrashID(crash1)) } func TestReportingQuota(t *testing.T) { @@ -268,39 +216,18 @@ func TestReportingQuota(t *testing.T) { defer c.Close() build := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build, nil)) + c.client.UploadBuild(build) const numReports = 8 // quota is 3 per day for i := 0; i < numReports; i++ { - crash := &dashapi.Crash{ - BuildID: "build1", - Title: fmt.Sprintf("title%v", i), - Log: []byte(fmt.Sprintf("log%v", i)), - Report: []byte(fmt.Sprintf("report%v", i)), - } - c.expectOK(c.API(client1, key1, "report_crash", crash, nil)) + c.client.ReportCrash(testCrash(build, i)) } for _, reports := range []int{3, 3, 2, 0, 0} { c.advanceTime(24 * time.Hour) - pr := &dashapi.PollBugsRequest{ - Type: "test", - } - resp := new(dashapi.PollBugsResponse) - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) - c.expectEQ(len(resp.Reports), reports) - for _, rep := range resp.Reports { - cmd := &dashapi.BugUpdate{ - ID: rep.ID, - Status: dashapi.BugStatusOpen, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) - } + c.client.pollBugs(reports) // Out of quota for today, so must get 0 reports. - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) - c.expectEQ(len(resp.Reports), 0) + c.client.pollBugs(0) } } @@ -310,111 +237,56 @@ func TestReportingDup(t *testing.T) { defer c.Close() build := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build, nil)) + c.client.UploadBuild(build) crash1 := testCrash(build, 1) - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) crash2 := testCrash(build, 2) - c.expectOK(c.API(client1, key1, "report_crash", crash2, nil)) - - pr := &dashapi.PollBugsRequest{ - Type: "test", - } - resp := new(dashapi.PollBugsResponse) - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) - c.expectEQ(len(resp.Reports), 2) - - rep1 := resp.Reports[0] - cmd := &dashapi.BugUpdate{ - ID: rep1.ID, - Status: dashapi.BugStatusOpen, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) + c.client.ReportCrash(crash2) - rep2 := resp.Reports[1] - cmd = &dashapi.BugUpdate{ - ID: rep2.ID, - Status: dashapi.BugStatusOpen, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) + reports := c.client.pollBugs(2) + rep1 := reports[0] + rep2 := reports[1] // Dup. - cmd = &dashapi.BugUpdate{ - ID: rep2.ID, - Status: dashapi.BugStatusDup, - DupOf: rep1.ID, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) - + c.client.updateBug(rep2.ID, dashapi.BugStatusDup, rep1.ID) { // Both must be reported as open. - req := &dashapi.PollClosedRequest{ - IDs: []string{rep1.ID, rep2.ID}, - } - resp := new(dashapi.PollClosedResponse) - c.expectOK(c.API(client1, key1, "reporting_poll_closed", req, resp)) - c.expectEQ(len(resp.IDs), 0) + closed, _ := c.client.ReportingPollClosed([]string{rep1.ID, rep2.ID}) + c.expectEQ(len(closed), 0) } // Undup. - cmd = &dashapi.BugUpdate{ - ID: rep2.ID, - Status: dashapi.BugStatusOpen, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) + c.client.updateBug(rep2.ID, dashapi.BugStatusOpen, "") // Dup again. - cmd = &dashapi.BugUpdate{ - ID: rep2.ID, - Status: dashapi.BugStatusDup, - DupOf: rep1.ID, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) + c.client.updateBug(rep2.ID, dashapi.BugStatusDup, rep1.ID) // Dup crash happens again, new bug must not be created. - c.expectOK(c.API(client1, key1, "report_crash", crash2, nil)) - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) - c.expectEQ(len(resp.Reports), 0) + c.client.ReportCrash(crash2) + c.client.pollBugs(0) // Now close the original bug, and check that new bugs for dup are now created. - cmd = &dashapi.BugUpdate{ - ID: rep1.ID, - Status: dashapi.BugStatusInvalid, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) - + c.client.updateBug(rep1.ID, dashapi.BugStatusInvalid, "") { // Now both must be reported as closed. - req := &dashapi.PollClosedRequest{ - IDs: []string{rep1.ID, rep2.ID}, - } - resp := new(dashapi.PollClosedResponse) - c.expectOK(c.API(client1, key1, "reporting_poll_closed", req, resp)) - c.expectEQ(len(resp.IDs), 2) - c.expectEQ(resp.IDs[0], rep1.ID) - c.expectEQ(resp.IDs[1], rep2.ID) + closed, _ := c.client.ReportingPollClosed([]string{rep1.ID, rep2.ID}) + c.expectEQ(len(closed), 2) + c.expectEQ(closed[0], rep1.ID) + c.expectEQ(closed[1], rep2.ID) } - c.expectOK(c.API(client1, key1, "report_crash", crash2, nil)) - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) - c.expectEQ(len(resp.Reports), 1) - c.expectEQ(resp.Reports[0].Title, crash2.Title+" (2)") + c.client.ReportCrash(crash2) + rep3 := c.client.pollBug() + c.expectEQ(rep3.Title, crash2.Title+" (2)") // Unduping after the canonical bugs was closed must not work // (we already created new bug for this report). - cmd = &dashapi.BugUpdate{ + reply, _ := c.client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep2.ID, Status: dashapi.BugStatusOpen, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + }) c.expectEQ(reply.OK, false) } @@ -425,35 +297,21 @@ func TestReportingDupToClosed(t *testing.T) { defer c.Close() build := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build, nil)) + c.client.UploadBuild(build) crash1 := testCrash(build, 1) - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) crash2 := testCrash(build, 2) - c.expectOK(c.API(client1, key1, "report_crash", crash2, nil)) - - reports := reportAllBugs(c, 2) - - cmd := &dashapi.BugUpdate{ - ID: reports[0].ID, - Status: dashapi.BugStatusInvalid, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) + c.client.ReportCrash(crash2) - cmd = &dashapi.BugUpdate{ - ID: reports[1].ID, - Status: dashapi.BugStatusDup, - DupOf: reports[0].ID, - } - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) + reports := c.client.pollBugs(2) + c.client.updateBug(reports[0].ID, dashapi.BugStatusInvalid, "") + c.client.updateBug(reports[1].ID, dashapi.BugStatusDup, reports[0].ID) - c.expectOK(c.API(client1, key1, "report_crash", crash2, nil)) - reports2 := reportAllBugs(c, 1) - c.expectEQ(reports2[0].Title, crash2.Title+" (2)") + c.client.ReportCrash(crash2) + rep2 := c.client.pollBug() + c.expectEQ(rep2.Title, crash2.Title+" (2)") } // Test that marking dups across reporting levels is not permitted. @@ -462,56 +320,44 @@ func TestReportingDupCrossReporting(t *testing.T) { defer c.Close() build := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build, nil)) + c.client.UploadBuild(build) crash1 := testCrash(build, 1) - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) crash2 := testCrash(build, 2) - c.expectOK(c.API(client1, key1, "report_crash", crash2, nil)) + c.client.ReportCrash(crash2) - reports := reportAllBugs(c, 2) + reports := c.client.pollBugs(2) rep1 := reports[0] rep2 := reports[1] // Upstream second bug. - cmd := &dashapi.BugUpdate{ - ID: rep2.ID, - Status: dashapi.BugStatusUpstream, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) - - reports = reportAllBugs(c, 1) - rep3 := reports[0] + c.client.updateBug(rep2.ID, dashapi.BugStatusUpstream, "") + rep3 := c.client.pollBug() { - req := &dashapi.PollClosedRequest{ - IDs: []string{rep1.ID, rep2.ID, rep3.ID}, - } - resp := new(dashapi.PollClosedResponse) - c.expectOK(c.API(client1, key1, "reporting_poll_closed", req, resp)) - c.expectEQ(len(resp.IDs), 1) - c.expectEQ(resp.IDs[0], rep2.ID) + closed, _ := c.client.ReportingPollClosed([]string{rep1.ID, rep2.ID, rep3.ID}) + c.expectEQ(len(closed), 1) + c.expectEQ(closed[0], rep2.ID) } // Duping must fail all ways. cmds := []*dashapi.BugUpdate{ - &dashapi.BugUpdate{ID: rep1.ID, DupOf: rep1.ID}, - &dashapi.BugUpdate{ID: rep1.ID, DupOf: rep2.ID}, - &dashapi.BugUpdate{ID: rep1.ID, DupOf: rep3.ID}, - &dashapi.BugUpdate{ID: rep2.ID, DupOf: rep1.ID}, - &dashapi.BugUpdate{ID: rep2.ID, DupOf: rep2.ID}, - &dashapi.BugUpdate{ID: rep2.ID, DupOf: rep3.ID}, - &dashapi.BugUpdate{ID: rep3.ID, DupOf: rep1.ID}, - &dashapi.BugUpdate{ID: rep3.ID, DupOf: rep2.ID}, - &dashapi.BugUpdate{ID: rep3.ID, DupOf: rep3.ID}, + {ID: rep1.ID, DupOf: rep1.ID}, + {ID: rep1.ID, DupOf: rep2.ID}, + {ID: rep1.ID, DupOf: rep3.ID}, + {ID: rep2.ID, DupOf: rep1.ID}, + {ID: rep2.ID, DupOf: rep2.ID}, + {ID: rep2.ID, DupOf: rep3.ID}, + {ID: rep3.ID, DupOf: rep1.ID}, + {ID: rep3.ID, DupOf: rep2.ID}, + {ID: rep3.ID, DupOf: rep3.ID}, } for _, cmd := range cmds { t.Logf("duping %v -> %v", cmd.ID, cmd.DupOf) cmd.Status = dashapi.BugStatusDup - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) + reply, _ := c.client.ReportingUpdate(cmd) c.expectEQ(reply.OK, false) } } @@ -521,42 +367,36 @@ func TestReportingFilter(t *testing.T) { defer c.Close() build := testBuild(1) - c.expectOK(c.API(client1, key1, "upload_build", build, nil)) + c.client.UploadBuild(build) crash1 := testCrash(build, 1) crash1.Title = "skip without repro 1" - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) // This does not skip first reporting, because it does not have repro. - rep1 := reportAllBugs(c, 1)[0] + rep1 := c.client.pollBug() c.expectEQ(string(rep1.Config), `{"Index":1}`) crash1.ReproSyz = []byte("getpid()") - c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + c.client.ReportCrash(crash1) // This has repro but was already reported to first reporting, // so repro must go to the first reporting as well. - rep2 := reportAllBugs(c, 1)[0] + rep2 := c.client.pollBug() c.expectEQ(string(rep2.Config), `{"Index":1}`) // Now upstream it and it must go to the second reporting. - cmd := &dashapi.BugUpdate{ - ID: rep1.ID, - Status: dashapi.BugStatusUpstream, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.OK, true) + c.client.updateBug(rep1.ID, dashapi.BugStatusUpstream, "") - rep3 := reportAllBugs(c, 1)[0] + rep3 := c.client.pollBug() c.expectEQ(string(rep3.Config), `{"Index":2}`) // Now report a bug that must go to the second reporting right away. crash2 := testCrash(build, 2) crash2.Title = "skip without repro 2" crash2.ReproSyz = []byte("getpid()") - c.expectOK(c.API(client1, key1, "report_crash", crash2, nil)) + c.client.ReportCrash(crash2) - rep4 := reportAllBugs(c, 1)[0] + rep4 := c.client.pollBug() c.expectEQ(string(rep4.Config), `{"Index":2}`) } diff --git a/dashboard/app/util_test.go b/dashboard/app/util_test.go index 2504a52d1..80ee445a7 100644 --- a/dashboard/app/util_test.go +++ b/dashboard/app/util_test.go @@ -39,6 +39,8 @@ type Ctx struct { ctx context.Context mockedTime time.Time emailSink chan *aemail.Message + client *apiClient + client2 *apiClient } func NewCtx(t *testing.T) *Ctx { @@ -62,6 +64,8 @@ func NewCtx(t *testing.T) *Ctx { mockedTime: time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC), emailSink: make(chan *aemail.Message, 100), } + c.client = c.makeClient(client1, key1, true) + c.client2 = c.makeClient(client2, key2, true) registerContext(r, c) return c } @@ -133,35 +137,6 @@ func (c *Ctx) advanceTime(d time.Duration) { c.mockedTime = c.mockedTime.Add(d) } -// API makes an api request to the app from the specified client. -func (c *Ctx) API(client, key, method string, req, reply interface{}) error { - doer := func(r *http.Request) (*http.Response, error) { - registerContext(r, c) - w := httptest.NewRecorder() - http.DefaultServeMux.ServeHTTP(w, r) - // Later versions of Go have a nice w.Result method, - // but we stuck on 1.6 on appengine. - if w.Body == nil { - w.Body = new(bytes.Buffer) - } - res := &http.Response{ - StatusCode: w.Code, - Status: http.StatusText(w.Code), - Body: ioutil.NopCloser(bytes.NewReader(w.Body.Bytes())), - } - return res, nil - } - - c.t.Logf("API(%v): %#v", method, req) - err := dashapi.Query(client, "", key, method, c.inst.NewRequest, doer, req, reply) - if err != nil { - c.t.Logf("ERROR: %v", err) - return err - } - c.t.Logf("REPLY: %#v", reply) - return nil -} - // GET sends admin-authorized HTTP GET request to the app. func (c *Ctx) GET(url string) error { _, err := c.httpRequest("GET", url, "", AccessAdmin) @@ -258,39 +233,44 @@ func (c *Ctx) checkURLContents(url string, want []byte) { type apiClient struct { *Ctx - client string - key string + *dashapi.Dashboard } -func (c *Ctx) makeClient(client, key string) *apiClient { +func (c *Ctx) makeClient(client, key string, failOnErrors bool) *apiClient { + doer := func(r *http.Request) (*http.Response, error) { + registerContext(r, c) + w := httptest.NewRecorder() + http.DefaultServeMux.ServeHTTP(w, r) + // Later versions of Go have a nice w.Result method, + // but we stuck on 1.6 on appengine. + if w.Body == nil { + w.Body = new(bytes.Buffer) + } + res := &http.Response{ + StatusCode: w.Code, + Status: http.StatusText(w.Code), + Body: ioutil.NopCloser(bytes.NewReader(w.Body.Bytes())), + } + return res, nil + } + logger := func(msg string, args ...interface{}) { + c.t.Logf("%v: "+msg, append([]interface{}{caller(3)}, args...)...) + } + errorHandler := func(err error) { + if failOnErrors { + c.t.Fatalf("\n%v: %v", caller(2), err) + } + } return &apiClient{ - Ctx: c, - client: client, - key: key, + Ctx: c, + Dashboard: dashapi.NewCustom(client, "", key, c.inst.NewRequest, doer, logger, errorHandler), } } -func (client *apiClient) API(method string, req, reply interface{}) error { - return client.Ctx.API(client.client, client.key, method, req, reply) -} - -func (client *apiClient) uploadBuild(build *dashapi.Build) { - client.expectOK(client.API("upload_build", build, nil)) -} - -func (client *apiClient) reportCrash(crash *dashapi.Crash) { - client.expectOK(client.API("report_crash", crash, nil)) -} - -// TODO(dvyukov): make this apiClient method. -func reportAllBugs(c *Ctx, expect int) []*dashapi.BugReport { - pr := &dashapi.PollBugsRequest{ - Type: "test", - } - resp := new(dashapi.PollBugsResponse) - c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp)) +func (client *apiClient) pollBugs(expect int) []*dashapi.BugReport { + resp, _ := client.ReportingPollBugs("test") if len(resp.Reports) != expect { - c.t.Fatalf("\n%v: want %v reports, got %v", caller(0), expect, len(resp.Reports)) + client.t.Fatalf("\n%v: want %v reports, got %v", caller(0), expect, len(resp.Reports)) } for _, rep := range resp.Reports { reproLevel := dashapi.ReproLevelNone @@ -299,28 +279,28 @@ func reportAllBugs(c *Ctx, expect int) []*dashapi.BugReport { } else if len(rep.ReproSyz) != 0 { reproLevel = dashapi.ReproLevelSyz } - cmd := &dashapi.BugUpdate{ + reply, _ := client.ReportingUpdate(&dashapi.BugUpdate{ ID: rep.ID, Status: dashapi.BugStatusOpen, ReproLevel: reproLevel, - } - reply := new(dashapi.BugUpdateReply) - c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply)) - c.expectEQ(reply.Error, false) - c.expectEQ(reply.OK, true) + }) + client.expectEQ(reply.Error, false) + client.expectEQ(reply.OK, true) } return resp.Reports } -func (client *apiClient) updateBug(extID string, status dashapi.BugStatus, dup string) *dashapi.BugUpdateReply { - cmd := &dashapi.BugUpdate{ +func (client *apiClient) pollBug() *dashapi.BugReport { + return client.pollBugs(1)[0] +} + +func (client *apiClient) updateBug(extID string, status dashapi.BugStatus, dup string) { + reply, _ := client.ReportingUpdate(&dashapi.BugUpdate{ ID: extID, Status: status, DupOf: dup, - } - reply := new(dashapi.BugUpdateReply) - client.expectOK(client.API("reporting_update", cmd, reply)) - return reply + }) + client.expectTrue(reply.OK) } type ( diff --git a/dashboard/dashapi/dashapi.go b/dashboard/dashapi/dashapi.go index 9abf0f908..bec406d78 100644 --- a/dashboard/dashapi/dashapi.go +++ b/dashboard/dashapi/dashapi.go @@ -20,16 +20,35 @@ import ( ) type Dashboard struct { - Client string - Addr string - Key string + Client string + Addr string + Key string + ctor RequestCtor + doer RequestDoer + logger RequestLogger + errorHandler func(error) } func New(client, addr, key string) *Dashboard { + return NewCustom(client, addr, key, http.NewRequest, http.DefaultClient.Do, nil, nil) +} + +type ( + RequestCtor func(method, url string, body io.Reader) (*http.Request, error) + RequestDoer func(req *http.Request) (*http.Response, error) + RequestLogger func(msg string, args ...interface{}) +) + +func NewCustom(client, addr, key string, ctor RequestCtor, doer RequestDoer, + logger RequestLogger, errorHandler func(error)) *Dashboard { return &Dashboard{ - Client: client, - Addr: addr, - Key: key, + Client: client, + Addr: addr, + Key: key, + ctor: ctor, + doer: doer, + logger: logger, + errorHandler: errorHandler, } } @@ -58,7 +77,7 @@ type FixCommit struct { } func (dash *Dashboard) UploadBuild(build *Build) error { - return dash.query("upload_build", build, nil) + return dash.Query("upload_build", build, nil) } // BuilderPoll request is done by kernel builder before uploading a new build @@ -83,7 +102,7 @@ func (dash *Dashboard) BuilderPoll(manager string) (*BuilderPollResp, error) { Manager: manager, } resp := new(BuilderPollResp) - err := dash.query("builder_poll", req, resp) + err := dash.Query("builder_poll", req, resp) return resp, err } @@ -125,12 +144,12 @@ type JobDoneReq struct { func (dash *Dashboard) JobPoll(managers []string) (*JobPollResp, error) { req := &JobPollReq{Managers: managers} resp := new(JobPollResp) - err := dash.query("job_poll", req, resp) + err := dash.Query("job_poll", req, resp) return resp, err } func (dash *Dashboard) JobDone(req *JobDoneReq) error { - return dash.query("job_done", req, nil) + return dash.Query("job_done", req, nil) } type BuildErrorReq struct { @@ -139,7 +158,7 @@ type BuildErrorReq struct { } func (dash *Dashboard) ReportBuildError(req *BuildErrorReq) error { - return dash.query("report_build_error", req, nil) + return dash.Query("report_build_error", req, nil) } // Crash describes a single kernel crash (potentially with repro). @@ -162,7 +181,7 @@ type ReportCrashResp struct { func (dash *Dashboard) ReportCrash(crash *Crash) (*ReportCrashResp, error) { resp := new(ReportCrashResp) - err := dash.query("report_crash", crash, resp) + err := dash.Query("report_crash", crash, resp) return resp, err } @@ -180,13 +199,13 @@ type NeedReproResp struct { // NeedRepro checks if dashboard needs a repro for this crash or not. func (dash *Dashboard) NeedRepro(crash *CrashID) (bool, error) { resp := new(NeedReproResp) - err := dash.query("need_repro", crash, resp) + err := dash.Query("need_repro", crash, resp) return resp.NeedRepro, err } // ReportFailedRepro notifies dashboard about a failed repro attempt for the crash. func (dash *Dashboard) ReportFailedRepro(crash *CrashID) error { - return dash.query("report_failed_repro", crash, nil) + return dash.Query("report_failed_repro", crash, nil) } type LogEntry struct { @@ -200,7 +219,7 @@ func (dash *Dashboard) LogError(name, msg string, args ...interface{}) { Name: name, Text: fmt.Sprintf(msg, args...), } - dash.query("log_error", req, nil) + dash.Query("log_error", req, nil) } // BugReport describes a single bug. @@ -283,6 +302,36 @@ type PollClosedResponse struct { IDs []string } +func (dash *Dashboard) ReportingPollBugs(typ string) (*PollBugsResponse, error) { + req := &PollBugsRequest{ + Type: typ, + } + resp := new(PollBugsResponse) + if err := dash.Query("reporting_poll_bugs", req, resp); err != nil { + return nil, err + } + return resp, nil +} + +func (dash *Dashboard) ReportingPollClosed(ids []string) ([]string, error) { + req := &PollClosedRequest{ + IDs: ids, + } + resp := new(PollClosedResponse) + if err := dash.Query("reporting_poll_closed", req, resp); err != nil { + return nil, err + } + return resp.IDs, nil +} + +func (dash *Dashboard) ReportingUpdate(upd *BugUpdate) (*BugUpdateReply, error) { + resp := new(BugUpdateReply) + if err := dash.Query("reporting_update", upd, resp); err != nil { + return nil, err + } + return resp, nil +} + type ManagerStatsReq struct { Name string Addr string @@ -299,7 +348,7 @@ type ManagerStatsReq struct { } func (dash *Dashboard) UploadManagerStats(req *ManagerStatsReq) error { - return dash.query("manager_stats", req, nil) + return dash.Query("manager_stats", req, nil) } type ( @@ -321,17 +370,27 @@ const ( ReproLevelC ) -func (dash *Dashboard) query(method string, req, reply interface{}) error { - return Query(dash.Client, dash.Addr, dash.Key, method, - http.NewRequest, http.DefaultClient.Do, req, reply) +func (dash *Dashboard) Query(method string, req, reply interface{}) error { + if dash.logger != nil { + dash.logger("API(%v): %#v", method, req) + } + err := dash.queryImpl(method, req, reply) + if err != nil { + if dash.logger != nil { + dash.logger("API(%v): ERROR: %v", method, err) + } + if dash.errorHandler != nil { + dash.errorHandler(err) + } + return err + } + if dash.logger != nil { + dash.logger("API(%v): REPLY: %#v", method, reply) + } + return nil } -type ( - RequestCtor func(method, url string, body io.Reader) (*http.Request, error) - RequestDoer func(req *http.Request) (*http.Response, error) -) - -func Query(client, addr, key, method string, ctor RequestCtor, doer RequestDoer, req, reply interface{}) error { +func (dash *Dashboard) queryImpl(method string, req, reply interface{}) error { if reply != nil { // json decoding behavior is somewhat surprising // (see // https://github.com/golang/go/issues/21092). @@ -343,8 +402,8 @@ func Query(client, addr, key, method string, ctor RequestCtor, doer RequestDoer, reflect.ValueOf(reply).Elem().Set(reflect.New(typ.Elem()).Elem()) } values := make(url.Values) - values.Add("client", client) - values.Add("key", key) + values.Add("client", dash.Client) + values.Add("key", dash.Key) values.Add("method", method) if req != nil { data, err := json.Marshal(req) @@ -361,12 +420,12 @@ func Query(client, addr, key, method string, ctor RequestCtor, doer RequestDoer, } values.Add("payload", buf.String()) } - r, err := ctor("POST", fmt.Sprintf("%v/api", addr), strings.NewReader(values.Encode())) + r, err := dash.ctor("POST", fmt.Sprintf("%v/api", dash.Addr), strings.NewReader(values.Encode())) if err != nil { return err } r.Header.Set("Content-Type", "application/x-www-form-urlencoded") - resp, err := doer(r) + resp, err := dash.doer(r) if err != nil { return fmt.Errorf("http request failed: %v", err) } -- cgit mrf-deployment