diff options
| -rw-r--r-- | dashboard/app/jobs.go | 57 | ||||
| -rw-r--r-- | dashboard/app/jobs_test.go | 21 |
2 files changed, 44 insertions, 34 deletions
diff --git a/dashboard/app/jobs.go b/dashboard/app/jobs.go index dce164bfc..db821b4c8 100644 --- a/dashboard/app/jobs.go +++ b/dashboard/app/jobs.go @@ -302,7 +302,9 @@ func handleRetestForBug(c context.Context, now time.Time, bug *Bug, bugKey *db.K if managerHasJob[crash.Manager] { continue } - build, err := loadBuild(c, bug.Namespace, crash.BuildID) + // Take the last successful build -- the build on which this crash happened + // might contain already obsolete repro and branch values. + build, err := lastManagerBuild(c, bug.Namespace, crash.Manager) if err != nil { return err } @@ -567,42 +569,38 @@ func createJobResp(c context.Context, job *Job, jobKey *db.Key) (*dashapi.JobPol // miss the situation when some actual user sends a patch testing request without // patch. func isRetestReproJob(job *Job, build *Build) bool { - return job.Type == JobTestPatch && + return (job.Type == JobTestPatch || job.Type == JobBisectFix) && job.Patch == 0 && job.KernelRepo == build.KernelRepo && job.KernelBranch == build.KernelBranch } -func handleRetestedRepro(c context.Context, now time.Time, job *Job, jobKey *db.Key, req *dashapi.JobDoneReq) error { +func handleRetestedRepro(c context.Context, now time.Time, job *Job, jobKey *db.Key, + lastBuild *Build, req *dashapi.JobDoneReq) error { bugKey := jobKey.Parent() crashKey := db.NewKey(c, "Crash", "", job.CrashID, bugKey) crash := new(Crash) if err := db.Get(c, crashKey, crash); err != nil { return fmt.Errorf("failed to get crash: %v", crashKey) } - build, err := loadBuild(c, job.Namespace, crash.BuildID) - if err != nil { - return fmt.Errorf("failed to query build: %w", err) - } - if !isRetestReproJob(job, build) { - return nil - } bug := new(Bug) if err := db.Get(c, bugKey, bug); err != nil { return fmt.Errorf("failed to get bug: %v", bugKey) } - if !config.Namespaces[bug.Namespace].RetestRepros { - return nil - } + allTitles := gatherCrashTitles(req) // Update the crash. crash.LastReproRetest = now - allTitles := gatherCrashTitles(req) - if req.Error == nil && len(allTitles) == 0 { - // If there was any crash at all, the repro is still not worth discarding. - // Also, if repro testing itself failed, it might be just a temporary issue. - crash.ReproIsRevoked = true + if req.Error == nil && !crash.ReproIsRevoked { + // If repro testing itself failed, it might be just a temporary issue. + if job.Type == JobTestPatch { + // If there was any crash at all, the repro is still not worth discarding. + crash.ReproIsRevoked = len(allTitles) == 0 + } else if job.Type == JobBisectFix { + // More than one commit is suspected => repro stopped working at some point. + crash.ReproIsRevoked = len(req.Commits) > 0 + } } - crash.UpdateReportingPriority(build, bug) + crash.UpdateReportingPriority(lastBuild, bug) if _, err := db.Put(c, crashKey, crash); err != nil { return fmt.Errorf("failed to put crash: %v", err) } @@ -652,6 +650,16 @@ func doneJob(c context.Context, req *dashapi.JobDoneReq) error { if err != nil { return err } + // Datastore prohibits cross-group queries even inside XG transactions. + // So we have to query last build for the manager before the transaction. + job := new(Job) + if err := db.Get(c, jobKey, job); err != nil { + return fmt.Errorf("job %v: failed to get job: %v", jobID, err) + } + lastBuild, err := lastManagerBuild(c, job.Namespace, job.Manager) + if err != nil { + return err + } now := timeNow(c) tx := func(c context.Context) error { job := new(Job) @@ -661,8 +669,8 @@ func doneJob(c context.Context, req *dashapi.JobDoneReq) error { if !job.Finished.IsZero() { return fmt.Errorf("job %v: already finished", jobID) } - if job.Type == JobTestPatch { - err := handleRetestedRepro(c, now, job, jobKey, req) + if isRetestReproJob(job, lastBuild) { + err := handleRetestedRepro(c, now, job, jobKey, lastBuild, req) if err != nil { return fmt.Errorf("job %v: failed to handle retested repro, %w", jobID, err) } @@ -749,13 +757,6 @@ func updateBugBisection(c context.Context, job *Job, jobKey *db.Key, req *dashap if _, err := db.Put(c, bugKey, bug); err != nil { return fmt.Errorf("failed to put bug: %v", err) } - // The repro is not working on the HEAD commit anymore, update the repro status. - if job.Type == JobBisectFix && req.Error == nil && len(req.Commits) > 0 { - err := handleRetestedRepro(c, now, job, jobKey, req) - if err != nil { - return err - } - } _, bugReporting, _, _, _ := currentReporting(c, bug) // The bug is either already closed or not yet reported in the current reporting, // either way we don't need to report it. If it wasn't reported, it will be reported diff --git a/dashboard/app/jobs_test.go b/dashboard/app/jobs_test.go index 31e4eafea..8bb9ec464 100644 --- a/dashboard/app/jobs_test.go +++ b/dashboard/app/jobs_test.go @@ -374,12 +374,12 @@ func TestReproRetestJob(t *testing.T) { c := NewCtx(t) defer c.Close() - build := testBuild(1) - build.KernelRepo = "git://mygit.com/git.git" - build.KernelBranch = "main" - c.client2.UploadBuild(build) + oldBuild := testBuild(1) + oldBuild.KernelRepo = "git://mygit.com/git.git" + oldBuild.KernelBranch = "main" + c.client2.UploadBuild(oldBuild) - crash := testCrash(build, 1) + crash := testCrash(oldBuild, 1) crash.ReproOpts = []byte("repro opts") crash.ReproSyz = []byte("repro syz") c.client2.ReportCrash(crash) @@ -387,11 +387,20 @@ func TestReproRetestJob(t *testing.T) { _, extBugID, err := email.RemoveAddrContext(sender) c.expectOK(err) - crash2 := testCrash(build, 1) + crash2 := testCrash(oldBuild, 1) crash2.ReproOpts = []byte("repro opts") crash2.ReproSyz = []byte("repro syz") crash2.ReproC = []byte("repro C") c.client2.ReportCrash(crash2) + + // Upload a newer build. + c.advanceTime(time.Minute) + build := testBuild(1) + build.ID = "new-build" + build.KernelRepo = "git://mygit.com/new-git.git" + build.KernelBranch = "new-main" + c.client2.UploadBuild(build) + // Wait until the bug is upstreamed. c.advanceTime(15 * 24 * time.Hour) c.pollEmailBug() |