From 3fe533890b585e03aa9ce456d37512f374076d54 Mon Sep 17 00:00:00 2001
From: Jonathan Amsterdam <jba@google.com>
Date: Sun, 8 Dec 2024 10:26:45 -0500
Subject: [PATCH] deploy: improve artifact upload

The changes in this CL improve the uploading of build artifacts
in deploy.yaml.

Astonishingly, Cloud Build does not upload artifacts if the build fails.
(The feature request is about five years old:
https://issuetracker.google.com/issues/143836671).
This makes it impossible to debug failed screentests.
The hacky solution is to cause failed build steps to succeed.
The -neverfail flag to screentest.sh accomplishes this.
It isn't used in this CL, but a subsequent CL with more debugging
support will use it.

When the build does succeed, the directory of failed screentests does
not exist, which causes artifact upload to fail. So create the
directories along with dummy files.

The wildcard syntax for Cloud Build artifact paths does not support
'**' as documented, so provide all upload paths.

Change-Id: I1c4223f4a906836000e5deac20a5b9aa920fbb97
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/634556
Commit-Queue: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Bypass: Jonathan Amsterdam <jba@google.com>
---
 deploy/deploy.yaml   |  4 +++-
 deploy/screentest.sh | 29 +++++++++++++++++++++++++++--
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/deploy/deploy.yaml b/deploy/deploy.yaml
index e63399c59..309101bf4 100644
--- a/deploy/deploy.yaml
+++ b/deploy/deploy.yaml
@@ -95,4 +95,6 @@ steps:
 artifacts:
   objects:
     location: 'gs://$PROJECT_ID/screentest-artifacts'
-    paths: ['tests/screentest/output/**']
+    paths:
+      - 'tests/screentest/output/*'
+      - 'tests/screentest/output/testcases/*'
diff --git a/deploy/screentest.sh b/deploy/screentest.sh
index 47421e82c..fd003d0be 100755
--- a/deploy/screentest.sh
+++ b/deploy/screentest.sh
@@ -10,23 +10,48 @@ source private/devtools/lib.sh || { echo "Are you at repo root?"; exit 1; }
 usage() {
   >&2 cat <<EOUSAGE
 
-  Usage: $0 [exp|dev|staging|prod|beta] IDTOKEN
+  Usage: $0 [OPTIONS] [exp|dev|staging|prod|beta] [IDTOKEN]
 
   Run the screentest check against a live instance of the given environment.
   These tests will only pass against staging and prod.
+  If IDTOKEN is omitted, it is read from the file _ID_TOKEN.
+
+  Options:
+    -neverfail
+      Do not return a non-zero exit code, even if the screentests fail.
+      This causes Cloud Build to upload screentest results for debugging.
+      It should never be used as part of a normal deployment.
 
 EOUSAGE
   exit 1
 }
 
 main() {
+  if [[ $# = 0 ]]; then
+    usage
+  fi
+  neverfail=false
+  if [[ $1 = -neverfail ]]; then
+    neverfail=true
+    shift
+  fi
+
   local env=$1
   local idtoken=$2
   check_env $env
   if [ -z $idtoken ]; then
     idtoken=$(cat _ID_TOKEN)
   fi
-  ./tests/screentest/run.sh -idtoken $idtoken -concurrency 1 $env
+  ./tests/screentest/run.sh -idtoken $idtoken -concurrency 1 $env || $neverfail
+  # If the tests passed, the tests/screentest/output directory will be empty.
+  # That causes the "artifacts" part of deploy.yaml to fail, because Cloud Build
+  # can't find any files there to upload. So create something there.
+  outdir=tests/screentest/output
+  if [[ ! -d $outdir ]]; then
+    mkdir -p $outdir/testcases
+    touch $outdir/success
+    touch $outdir/testcases/success
+  fi
 }
 
 main $@