[recipes] Fix *_VERSION step failure handling

Bug: skia:6473
Change-Id: I2fa6f800f59f40f74b5c080c52cdec3a32329ef7
NOTRY=true
Reviewed-on: https://skia-review.googlesource.com/14240
Commit-Queue: Stephan Altmueller <stephana@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Stephan Altmueller <stephana@google.com>

8 files changed, 22 insertions, 216 deletions

diff --git a/infra/bots/recipe_modules/flavor/api.py b/infra/bots/recipe_modules/flavor/api.py
index 7a1ae2548c..a2ac1f1eb0 100644
--- a/infra/bots/recipe_modules/flavor/api.py
+++ b/infra/bots/recipe_modules/flavor/api.py
@@ -108,8 +108,8 @@ class SkiaFlavorApi(recipe_api.RecipeApi):
   def create_clean_device_dir(self, path):
     return self._f.create_clean_device_dir(path)
 
-  def read_file_on_device(self, path):
-    return self._f.read_file_on_device(path)
+  def read_file_on_device(self, path, **kwargs):
+    return self._f.read_file_on_device(path, **kwargs)
 
   def remove_file_on_device(self, path):
     return self._f.remove_file_on_device(path)
@@ -145,9 +145,10 @@ class SkiaFlavorApi(recipe_api.RecipeApi):
     device_version_file = self.device_path_join(
         self.device_dirs.tmp_dir, version_file)
     if str(actual_version_file) != str(device_version_file):
-      try:
-        device_version = self.read_file_on_device(device_version_file)
-      except self.m.step.StepFailure:
+      device_version = self.read_file_on_device(device_version_file,
+                                                abort_on_failure=False,
+                                                fail_build_on_failure=False)
+      if not device_version:
         device_version = VERSION_NONE
       if device_version != host_version:
         self.remove_file_on_device(device_version_file)
diff --git a/infra/bots/recipe_modules/flavor/example.expected/Perf-ChromeOS-Clang-Chromebook_513C24_K01-GPU-MaliT860-arm-Release.json b/infra/bots/recipe_modules/flavor/example.expected/Perf-ChromeOS-Clang-Chromebook_513C24_K01-GPU-MaliT860-arm-Release.json
index 20bbe0be06..30493caf42 100644
--- a/infra/bots/recipe_modules/flavor/example.expected/Perf-ChromeOS-Clang-Chromebook_513C24_K01-GPU-MaliT860-arm-Release.json
+++ b/infra/bots/recipe_modules/flavor/example.expected/Perf-ChromeOS-Clang-Chromebook_513C24_K01-GPU-MaliT860-arm-Release.json
@@ -311,26 +311,6 @@
       "-t",
       "-t",
       "foo@127.0.0.1",
-      "touch",
-      "/home/chronos/user/SKP_VERSION"
-    ],
-    "env": {
-      "BUILDTYPE": "Release",
-      "CHROME_HEADLESS": "1",
-      "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]",
-      "SKIA_OUT": "[START_DIR]/out"
-    },
-    "infra_step": true,
-    "name": "touch /home/chronos/user/SKP_VERSION"
-  },
-  {
-    "cmd": [
-      "ssh",
-      "-oConnectTimeout=15",
-      "-oBatchMode=yes",
-      "-t",
-      "-t",
-      "foo@127.0.0.1",
       "cat",
       "/home/chronos/user/SKP_VERSION"
     ],
@@ -477,26 +457,6 @@
       "-t",
       "-t",
       "foo@127.0.0.1",
-      "touch",
-      "/home/chronos/user/SK_IMAGE_VERSION"
-    ],
-    "env": {
-      "BUILDTYPE": "Release",
-      "CHROME_HEADLESS": "1",
-      "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]",
-      "SKIA_OUT": "[START_DIR]/out"
-    },
-    "infra_step": true,
-    "name": "touch /home/chronos/user/SK_IMAGE_VERSION"
-  },
-  {
-    "cmd": [
-      "ssh",
-      "-oConnectTimeout=15",
-      "-oBatchMode=yes",
-      "-t",
-      "-t",
-      "foo@127.0.0.1",
       "cat",
       "/home/chronos/user/SK_IMAGE_VERSION"
     ],
@@ -643,26 +603,6 @@
       "-t",
       "-t",
       "foo@127.0.0.1",
-      "touch",
-      "/home/chronos/user/SVG_VERSION"
-    ],
-    "env": {
-      "BUILDTYPE": "Release",
-      "CHROME_HEADLESS": "1",
-      "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]",
-      "SKIA_OUT": "[START_DIR]/out"
-    },
-    "infra_step": true,
-    "name": "touch /home/chronos/user/SVG_VERSION"
-  },
-  {
-    "cmd": [
-      "ssh",
-      "-oConnectTimeout=15",
-      "-oBatchMode=yes",
-      "-t",
-      "-t",
-      "foo@127.0.0.1",
       "cat",
       "/home/chronos/user/SVG_VERSION"
     ],
diff --git a/infra/bots/recipe_modules/flavor/example.expected/failed_read_version.json b/infra/bots/recipe_modules/flavor/example.expected/failed_read_version.json
index 1a6925edf9..bf289bddd9 100644
--- a/infra/bots/recipe_modules/flavor/example.expected/failed_read_version.json
+++ b/infra/bots/recipe_modules/flavor/example.expected/failed_read_version.json
@@ -851,23 +851,6 @@
   {
     "cmd": [
       "adb",
-      "shell",
-      "reboot",
-      "-p"
-    ],
-    "cwd": "[START_DIR]/skia",
-    "env": {
-      "BUILDTYPE": "Debug",
-      "CHROME_HEADLESS": "1",
-      "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]",
-      "SKIA_OUT": "[START_DIR]/out"
-    },
-    "infra_step": true,
-    "name": "shut down device to quarantine bot"
-  },
-  {
-    "cmd": [
-      "adb",
       "kill-server"
     ],
     "cwd": "[START_DIR]/skia",
@@ -882,8 +865,7 @@
   },
   {
     "name": "$result",
-    "reason": "Failed build steps: read /sdcard/revenge_of_the_skiabot/SK_IMAGE_VERSION",
     "recipe_result": null,
-    "status_code": 1
+    "status_code": 0
   }
 ]
\ No newline at end of file
diff --git a/infra/bots/recipe_modules/flavor/gn_android_flavor.py b/infra/bots/recipe_modules/flavor/gn_android_flavor.py
index dce8c601ba..318915afc8 100644
--- a/infra/bots/recipe_modules/flavor/gn_android_flavor.py
+++ b/infra/bots/recipe_modules/flavor/gn_android_flavor.py
@@ -218,9 +218,11 @@ class GNAndroidFlavorUtils(default_flavor.DefaultFlavorUtils):
   def copy_directory_contents_to_host(self, device, host):
     self._adb('pull %s %s' % (device, host), 'pull', device, host)
 
-  def read_file_on_device(self, path):
-    return self._adb('read %s' % path,
-                     'shell', 'cat', path, stdout=self.m.raw_io.output()).stdout
+  def read_file_on_device(self, path, **kwargs):
+    rv = self._adb('read %s' % path,
+                   'shell', 'cat', path, stdout=self.m.raw_io.output(),
+                   **kwargs)
+    return rv.stdout.rstrip() if rv and rv.stdout else None
 
   def remove_file_on_device(self, path):
     self._adb('rm %s' % path, 'shell', 'rm', '-f', path)
diff --git a/infra/bots/recipe_modules/flavor/gn_chromebook_flavor.py b/infra/bots/recipe_modules/flavor/gn_chromebook_flavor.py
index 34ffa54197..c004d9971e 100644
--- a/infra/bots/recipe_modules/flavor/gn_chromebook_flavor.py
+++ b/infra/bots/recipe_modules/flavor/gn_chromebook_flavor.py
@@ -151,11 +151,11 @@ class GNChromebookFlavorUtils(gn_flavor.GNFlavorUtils):
     self._ssh('rm %s' % path, 'rm', '-rf', path)
     self._ssh('mkdir %s' % path, 'mkdir', '-p', path)
 
-  def read_file_on_device(self, path):
-    # To avoid failure if file doesn't exist.
-    self._ssh('touch %s' % path, 'touch', path)
-    return self._ssh('read %s' % path,
-                     'cat', path, stdout=self.m.raw_io.output()).stdout
+  def read_file_on_device(self, path, **kwargs):
+    rv = self._ssh('read %s' % path,
+                   'cat', path, stdout=self.m.raw_io.output(),
+                   **kwargs)
+    return rv.stdout.rstrip() if rv and rv.stdout else None
 
   def remove_file_on_device(self, path):
     # use -f to silently return if path doesn't exist
diff --git a/infra/bots/recipe_modules/flavor/ios_flavor.py b/infra/bots/recipe_modules/flavor/ios_flavor.py
index 45b33448a1..976e840f93 100644
--- a/infra/bots/recipe_modules/flavor/ios_flavor.py
+++ b/infra/bots/recipe_modules/flavor/ios_flavor.py
@@ -65,11 +65,12 @@ class iOSFlavorUtils(gn_flavor.GNFlavorUtils):
     self._run_ios_script('rm',    path)
     self._run_ios_script('mkdir', path)
 
-  def read_file_on_device(self, path):
+  def read_file_on_device(self, path, **kwargs):
     full = self.m.vars.skia_dir.join('platform_tools/ios/bin/ios_cat_file')
-    rc = self.m.run(self.m.step,
+    rv = self.m.run(self.m.step,
                     name = 'cat_file %s' % path,
                     cmd = [full, path],
                     stdout=self.m.raw_io.output(),
-                    infra_step=kInfraStep)
-    return rc.stdout.rstrip() if rc.stdout else rc.stdout
+                    infra_step=kInfraStep,
+                    **kwargs)
+    return rv.stdout.rstrip() if rv and rv.stdout else None
diff --git a/infra/bots/recipes/perf.expected/Perf-ChromeOS-Clang-Chromebook_C100p-GPU-MaliT764-arm-Release.json b/infra/bots/recipes/perf.expected/Perf-ChromeOS-Clang-Chromebook_C100p-GPU-MaliT764-arm-Release.json
index 272d36746d..25c79633d2 100644
--- a/infra/bots/recipes/perf.expected/Perf-ChromeOS-Clang-Chromebook_C100p-GPU-MaliT764-arm-Release.json
+++ b/infra/bots/recipes/perf.expected/Perf-ChromeOS-Clang-Chromebook_C100p-GPU-MaliT764-arm-Release.json
@@ -160,26 +160,6 @@
       "-t",
       "-t",
       "foo@127.0.0.1",
-      "touch",
-      "/home/chronos/user/SKP_VERSION"
-    ],
-    "env": {
-      "BUILDTYPE": "Release",
-      "CHROME_HEADLESS": "1",
-      "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]",
-      "SKIA_OUT": "[START_DIR]/out"
-    },
-    "infra_step": true,
-    "name": "touch /home/chronos/user/SKP_VERSION"
-  },
-  {
-    "cmd": [
-      "ssh",
-      "-oConnectTimeout=15",
-      "-oBatchMode=yes",
-      "-t",
-      "-t",
-      "foo@127.0.0.1",
       "cat",
       "/home/chronos/user/SKP_VERSION"
     ],
@@ -326,26 +306,6 @@
       "-t",
       "-t",
       "foo@127.0.0.1",
-      "touch",
-      "/home/chronos/user/SK_IMAGE_VERSION"
-    ],
-    "env": {
-      "BUILDTYPE": "Release",
-      "CHROME_HEADLESS": "1",
-      "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]",
-      "SKIA_OUT": "[START_DIR]/out"
-    },
-    "infra_step": true,
-    "name": "touch /home/chronos/user/SK_IMAGE_VERSION"
-  },
-  {
-    "cmd": [
-      "ssh",
-      "-oConnectTimeout=15",
-      "-oBatchMode=yes",
-      "-t",
-      "-t",
-      "foo@127.0.0.1",
       "cat",
       "/home/chronos/user/SK_IMAGE_VERSION"
     ],
@@ -492,26 +452,6 @@
       "-t",
       "-t",
       "foo@127.0.0.1",
-      "touch",
-      "/home/chronos/user/SVG_VERSION"
-    ],
-    "env": {
-      "BUILDTYPE": "Release",
-      "CHROME_HEADLESS": "1",
-      "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]",
-      "SKIA_OUT": "[START_DIR]/out"
-    },
-    "infra_step": true,
-    "name": "touch /home/chronos/user/SVG_VERSION"
-  },
-  {
-    "cmd": [
-      "ssh",
-      "-oConnectTimeout=15",
-      "-oBatchMode=yes",
-      "-t",
-      "-t",
-      "foo@127.0.0.1",
       "cat",
       "/home/chronos/user/SVG_VERSION"
     ],
diff --git a/infra/bots/recipes/test.expected/Test-ChromeOS-Clang-Chromebook_C100p-GPU-MaliT764-arm-Debug.json b/infra/bots/recipes/test.expected/Test-ChromeOS-Clang-Chromebook_C100p-GPU-MaliT764-arm-Debug.json
index b66f798bda..a578346fd2 100644
--- a/infra/bots/recipes/test.expected/Test-ChromeOS-Clang-Chromebook_C100p-GPU-MaliT764-arm-Debug.json
+++ b/infra/bots/recipes/test.expected/Test-ChromeOS-Clang-Chromebook_C100p-GPU-MaliT764-arm-Debug.json
@@ -160,26 +160,6 @@
       "-t",
       "-t",
       "foo@127.0.0.1",
-      "touch",
-      "/home/chronos/user/SKP_VERSION"
-    ],
-    "env": {
-      "BUILDTYPE": "Debug",
-      "CHROME_HEADLESS": "1",
-      "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]",
-      "SKIA_OUT": "[START_DIR]/out"
-    },
-    "infra_step": true,
-    "name": "touch /home/chronos/user/SKP_VERSION"
-  },
-  {
-    "cmd": [
-      "ssh",
-      "-oConnectTimeout=15",
-      "-oBatchMode=yes",
-      "-t",
-      "-t",
-      "foo@127.0.0.1",
       "cat",
       "/home/chronos/user/SKP_VERSION"
     ],
@@ -326,26 +306,6 @@
       "-t",
       "-t",
       "foo@127.0.0.1",
-      "touch",
-      "/home/chronos/user/SK_IMAGE_VERSION"
-    ],
-    "env": {
-      "BUILDTYPE": "Debug",
-      "CHROME_HEADLESS": "1",
-      "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]",
-      "SKIA_OUT": "[START_DIR]/out"
-    },
-    "infra_step": true,
-    "name": "touch /home/chronos/user/SK_IMAGE_VERSION"
-  },
-  {
-    "cmd": [
-      "ssh",
-      "-oConnectTimeout=15",
-      "-oBatchMode=yes",
-      "-t",
-      "-t",
-      "foo@127.0.0.1",
       "cat",
       "/home/chronos/user/SK_IMAGE_VERSION"
     ],
@@ -492,26 +452,6 @@
       "-t",
       "-t",
       "foo@127.0.0.1",
-      "touch",
-      "/home/chronos/user/SVG_VERSION"
-    ],
-    "env": {
-      "BUILDTYPE": "Debug",
-      "CHROME_HEADLESS": "1",
-      "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]",
-      "SKIA_OUT": "[START_DIR]/out"
-    },
-    "infra_step": true,
-    "name": "touch /home/chronos/user/SVG_VERSION"
-  },
-  {
-    "cmd": [
-      "ssh",
-      "-oConnectTimeout=15",
-      "-oBatchMode=yes",
-      "-t",
-      "-t",
-      "foo@127.0.0.1",
       "cat",
       "/home/chronos/user/SVG_VERSION"
     ],