From fa98444be7845009aceef06b9621df3ccb46eb9f Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Mon, 17 Oct 2022 15:59:29 -0400 Subject: [PATCH] internal/task: handle missing commit in greenness I wanted the greenness check to handle the case where the commit had aged off the first page of results, and have it pick an arbitrary commit it could in that case. But I didn't consider that the dashboard doesn't observe commits instantaneously, and relui can't distinguish the two cases. For now, refuse to proceed if the commit isn't on the result page. For golang/go#48523. Change-Id: I0a8c9977c1b159f794f2a1d209ef749e606fc422 Reviewed-on: https://go-review.googlesource.com/c/build/+/443436 Auto-Submit: Heschi Kreinick Run-TryBot: Heschi Kreinick TryBot-Result: Gopher Robot Reviewed-by: Dmitri Shuralyov Reviewed-by: Dmitri Shuralyov --- internal/task/tagx.go | 12 ++++++++++++ internal/task/tagx_test.go | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/internal/task/tagx.go b/internal/task/tagx.go index 87c91631..d6e63a5a 100644 --- a/internal/task/tagx.go +++ b/internal/task/tagx.go @@ -471,6 +471,18 @@ func (x *TagXReposTasks) findGreen(ctx *wf.TaskContext, repo TagRepo, commit str } } + foundCommit := false + for _, rev := range repoStatus.Revisions { + if rev.Revision == commit { + foundCommit = true + break + } + } + if !foundCommit { + ctx.Printf("commit %v not found on first page of results; too old or too new?", commit) + return "", false, nil + } + // x/ repo statuses are: // // diff --git a/internal/task/tagx_test.go b/internal/task/tagx_test.go index f1a287ef..0ac3916c 100644 --- a/internal/task/tagx_test.go +++ b/internal/task/tagx_test.go @@ -199,6 +199,26 @@ func TestIsGreen(t *testing.T) { }, wantGreenRev: "tools-1", }, + { + name: "not green yet", + rev: "tools-1", + lines: []revLine{ + {"master", 3, 1, true}, + {"release-branch.go1.19", 1, 1, false}, + {"release-branch.go1.18", 1, 1, true}, + }, + wantGreenRev: "", + }, + { + name: "commit not registered on dashboard", + rev: "tools-2", + lines: []revLine{ + {"master", 1, 1, true}, + {"release-branch.go1.19", 1, 1, true}, + {"release-branch.go1.18", 1, 1, true}, + }, + wantGreenRev: "", + }, } for _, tt := range tests {