From 79e0278d51029f3bc69828cef47e49cbded70dcf Mon Sep 17 00:00:00 2001
From: Aleksandr Nogikh <nogikh@google.com>
Date: Mon, 28 Aug 2023 14:49:49 +0200
Subject: dashboard: redo origin testing for newer crashes

If a higher-priority crash has become available, re-run bug origin
tests.

Currently, since fix candidate testing runs on tree origin testing
results, we remain bound to the manager used back then. And it can be
that the manager does not support bisections (e.g. qemu) or is quite
problematic (arm64).
---
 dashboard/app/app_test.go  |  5 +++++
 dashboard/app/tree.go      | 28 ++++++++++++++--------------
 dashboard/app/tree_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go
index cb34c19bd..042e2c8da 100644
--- a/dashboard/app/app_test.go
+++ b/dashboard/app/app_test.go
@@ -535,6 +535,11 @@ var testConfig = &GlobalConfig{
 					DetectMissingBackports: true,
 				},
 			},
+			Managers: map[string]ConfigManager{
+				"better-manager": {
+					Priority: 1,
+				},
+			},
 			Reporting: []Reporting{
 				{
 					AccessLevel: AccessAdmin,
diff --git a/dashboard/app/tree.go b/dashboard/app/tree.go
index 3b50e65a8..d6a2dcce5 100644
--- a/dashboard/app/tree.go
+++ b/dashboard/app/tree.go
@@ -551,7 +551,7 @@ func (ctx *bugTreeContext) loadCrashInfo() error {
 		crashKey := db.NewKey(ctx.c, "Crash", "", crashID, ctx.bugKey)
 		crash := new(Crash)
 		// We need to also tolerate the case when the crash was just deleted.
-		err := db.Get(ctx.c, crashKey, crash)
+		err := db.Get(ctx.cGlobal, crashKey, crash)
 		if err != nil && err != db.ErrNoSuchEntity {
 			return fmt.Errorf("failed to get crash: %w", err)
 		} else if err == nil {
@@ -566,20 +566,20 @@ func (ctx *bugTreeContext) loadCrashInfo() error {
 			}
 		}
 	}
+
 	// Query the most relevant crash with repro.
-	if ctx.crash == nil {
-		crash, crashKey, err := findCrashForBug(ctx.c, ctx.bug)
-		if err != nil {
-			return err
-		}
-		ok, build, err := ctx.isCrashRelevant(crash)
-		if err != nil {
-			return err
-		} else if ok {
-			ctx.build = build
-			ctx.crash = crash
-			ctx.crashKey = crashKey
-		}
+	crash, crashKey, err := findCrashForBug(ctx.cGlobal, ctx.bug)
+	if err != nil {
+		return err
+	}
+	ok, build, err := ctx.isCrashRelevant(crash)
+	if err != nil {
+		return err
+	} else if ok && (ctx.crash == nil || crash.ReportLen > ctx.crash.ReportLen) {
+		// Update the crash only if we found a better one.
+		ctx.build = build
+		ctx.crash = crash
+		ctx.crashKey = crashKey
 	}
 	// Load the rest of the data.
 	if ctx.crash != nil {
diff --git a/dashboard/app/tree_test.go b/dashboard/app/tree_test.go
index 52541029d..67390737b 100644
--- a/dashboard/app/tree_test.go
+++ b/dashboard/app/tree_test.go
@@ -121,6 +121,51 @@ This report is generated by a bot. It may contain errors.`)
 	c.client.pollNotifs(0)
 }
 
+func TestTreeOriginBetterReport(t *testing.T) {
+	// Ensure that, once a higher priority crash becomes available, we perform origin testing again.
+	c := NewCtx(t)
+	defer c.Close()
+
+	ctx := setUpTreeTest(c, downstreamUpstreamRepos)
+	ctx.uploadBug(`https://downstream.repo/repo`, `master`, dashapi.ReproLevelC)
+	ctx.entries = []treeTestEntry{
+		{
+			alias:   `downstream`,
+			results: []treeTestEntryPeriod{{fromDay: 0, result: treeTestCrash}},
+		},
+		{
+			alias:      `lts`,
+			mergeAlias: `downstream`,
+			results:    []treeTestEntryPeriod{{fromDay: 0, result: treeTestOK}},
+		},
+		{
+			alias:   `upstream`,
+			results: []treeTestEntryPeriod{{fromDay: 0, result: treeTestOK}},
+		},
+	}
+	ctx.jobTestDays = []int{10, 20, 30}
+	ctx.moveToDay(10)
+	ctx.ensureLabels("origin:downstream")
+	c.expectEQ(ctx.entries[1].jobsDone, 1)
+	c.expectEQ(ctx.entries[2].jobsDone, 1)
+
+	// No retets are needed yet.
+	ctx.moveToDay(20)
+	c.expectEQ(ctx.entries[1].jobsDone, 1)
+	c.expectEQ(ctx.entries[2].jobsDone, 1)
+
+	// Use a "better" manager.
+	ctx.manager = "better-manager"
+	ctx.uploadBug(`https://downstream.repo/repo`, `master`, dashapi.ReproLevelC)
+
+	// With that reproducer, lts begins to crash as well.
+	ctx.entries[1].results = []treeTestEntryPeriod{{fromDay: 0, result: treeTestCrash}}
+	ctx.moveToDay(30)
+	ctx.ensureLabels("origin:lts")
+	c.expectEQ(ctx.entries[1].jobsDone, 2)
+	c.expectEQ(ctx.entries[2].jobsDone, 2)
+}
+
 func TestTreeOriginLts(t *testing.T) {
 	c := NewCtx(t)
 	defer c.Close()
-- 
cgit mrf-deployment