From 2129f66e2d062b700c69f711b52cf8401ab35e83 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Wed, 17 Jan 2018 10:49:41 +0100 Subject: dashboard/app: use Reported-by tags to fix bugs Accept and use Reported-by tags in commits to mark bugs as fixed. --- dashboard/app/api.go | 38 ++++++++++++++++++++++++------- dashboard/app/fix_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++++++ dashboard/app/reporting.go | 25 ++++----------------- 3 files changed, 90 insertions(+), 29 deletions(-) (limited to 'dashboard') diff --git a/dashboard/app/api.go b/dashboard/app/api.go index 3d33ee818..5a0d3823d 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -10,6 +10,7 @@ import ( "fmt" "io/ioutil" "net/http" + "reflect" "sort" "strings" "time" @@ -291,9 +292,14 @@ func uploadBuild(c context.Context, ns string, req *dashapi.Build, typ BuildType func addCommitsToBugs(c context.Context, ns, manager string, titles []string, fixCommits []dashapi.FixCommit) error { - commitMap := make(map[string]bool) + presentCommits := make(map[string]bool) + bugFixedBy := make(map[string][]string) for _, com := range titles { - commitMap[com] = true + presentCommits[com] = true + } + for _, com := range fixCommits { + presentCommits[com.Title] = true + bugFixedBy[com.BugID] = append(bugFixedBy[com.BugID], com.Title) } managers, err := managerList(c, ns) if err != nil { @@ -309,7 +315,12 @@ func addCommitsToBugs(c context.Context, ns, manager string, } now := timeNow(c) for i, bug := range bugs { - if !fixedWith(bug, manager, commitMap) { + var fixCommits []string + for i := range bug.Reporting { + fixCommits = append(fixCommits, bugFixedBy[bug.Reporting[i].ID]...) + } + sort.Strings(fixCommits) + if !bugNeedsCommitUpdate(c, bug, manager, fixCommits, presentCommits) { continue } tx := func(c context.Context) error { @@ -317,9 +328,13 @@ func addCommitsToBugs(c context.Context, ns, manager string, if err := datastore.Get(c, keys[i], bug); err != nil { return fmt.Errorf("failed to get bug %v: %v", keys[i].StringID(), err) } - if !fixedWith(bug, manager, commitMap) { + if !bugNeedsCommitUpdate(nil, bug, manager, fixCommits, presentCommits) { return nil } + if len(fixCommits) != 0 && !reflect.DeepEqual(bug.Commits, fixCommits) { + bug.Commits = fixCommits + bug.PatchedOn = nil + } bug.PatchedOn = append(bug.PatchedOn, manager) if bug.Status == BugStatusOpen { fixed := true @@ -367,16 +382,23 @@ func managerList(c context.Context, ns string) ([]string, error) { return managers, nil } -func fixedWith(bug *Bug, manager string, commits map[string]bool) bool { - if stringInList(bug.PatchedOn, manager) { +func bugNeedsCommitUpdate(c context.Context, bug *Bug, manager string, fixCommits []string, + presentCommits map[string]bool) bool { + if len(fixCommits) != 0 && !reflect.DeepEqual(bug.Commits, fixCommits) { + if c != nil { + log.Infof(c, "bug %q is fixed with %q", bug.Title, fixCommits) + } + return true + } + if len(bug.Commits) == 0 || stringInList(bug.PatchedOn, manager) { return false } for _, com := range bug.Commits { - if !commits[com] { + if !presentCommits[com] { return false } } - return len(bug.Commits) > 0 + return true } func stringInList(list []string, str string) bool { diff --git a/dashboard/app/fix_test.go b/dashboard/app/fix_test.go index 2f070f00f..cf8105cf3 100644 --- a/dashboard/app/fix_test.go +++ b/dashboard/app/fix_test.go @@ -439,3 +439,59 @@ func TestReFixedTwoManagers(t *testing.T) { reports = reportAllBugs(c, 1) c.expectEQ(reports[0].Title, "title1 (2)") } + +// TestFixedWithCommitTags tests fixing of bugs with Reported-by commit tags. +func TestFixedWithCommitTags(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + build1 := testBuild(1) + c.expectOK(c.API(client1, key1, "upload_build", build1, nil)) + + build2 := testBuild(2) + c.expectOK(c.API(client1, key1, "upload_build", build2, nil)) + + crash1 := testCrash(build1, 1) + c.expectOK(c.API(client1, key1, "report_crash", crash1, nil)) + + rep := reportAllBugs(c, 1)[0] + + // 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)) + + // 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)) + 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)) + 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) + + // 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)) + + // 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)) + 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)") +} diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go index 048128771..892e8a906 100644 --- a/dashboard/app/reporting.go +++ b/dashboard/app/reporting.go @@ -6,6 +6,7 @@ package dash import ( "encoding/json" "fmt" + "reflect" "sort" "strings" "time" @@ -477,27 +478,9 @@ func incomingCommandTx(c context.Context, now time.Time, cmd *dashapi.BugUpdate, return false, internalError, fmt.Errorf("unknown bug status %v", cmd.Status) } if len(cmd.FixCommits) != 0 && (bug.Status == BugStatusOpen || bug.Status == BugStatusDup) { - m := make(map[string]bool) - for _, com := range cmd.FixCommits { - m[com] = true - } - same := false - if len(bug.Commits) == len(m) { - same = true - for _, com := range bug.Commits { - if !m[com] { - same = false - break - } - } - } - if !same { - commits := make([]string, 0, len(m)) - for com := range m { - commits = append(commits, com) - } - sort.Strings(commits) - bug.Commits = commits + sort.Strings(cmd.FixCommits) + if !reflect.DeepEqual(bug.Commits, cmd.FixCommits) { + bug.Commits = cmd.FixCommits bug.PatchedOn = nil } } -- cgit mrf-deployment