Reland "Reland "Temporary add verify_file_exists to check files downloaded.""

This reverts commit 7cba29c9.

Reason for revert: Switch to logging.error instead of raise exception

Original change's description:
> Revert "Reland "Temporary add verify_file_exists to check files downloaded.""
>
> This reverts commit 44e43142.
>
> Reason for revert: Lots of builders are failing in gclient runhook.
> https://ci.chromium.org/p/chrome/builders/ci/test-wifi-tablet/19099?
>
> Original change's description:
> > Reland "Temporary add verify_file_exists to check files downloaded."
> >
> > This reverts commit 2b17e053.
> >
> > Reason for revert: Reland initial cl after fix the typo
> >
> > Original change's description:
> > > Revert "Temporary add verify_file_exists to check files downloaded."
> > >
> > > This reverts commit e69f63a3.
> > >
> > > Reason for revert: Causing infra failures on chrome/ci/android-internal-chromium-tot, see crbug.com/1122206.
> > >
> > > Original change's description:
> > > > Temporary add verify_file_exists to check files downloaded.
> > > >
> > > > R=​bsheedy@chromium.org
> > > >
> > > > Context: to make sure those wprarchive files are downloaded properly.
> > > > Bug: 1096656
> > > > Change-Id: Id0197c39f1a260bf9356269f41c12a46deecbf17
> > > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2378386
> > > > Reviewed-by: Dirk Pranke <dpranke@google.com>
> > > > Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
> > > > Commit-Queue: Arthur Wang <wuwang@chromium.org>
> > > > Cr-Commit-Position: refs/heads/master@{#801957}
> > >
> > > TBR=dpranke@google.com,bsheedy@chromium.org,wuwang@chromium.org
> > >
> > > Change-Id: I4d7f023d4e4d8f441d2cbdc33788e17f859377ab
> > > No-Presubmit: true
> > > No-Tree-Checks: true
> > > No-Try: true
> > > Bug: 1096656
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2378541
> > > Reviewed-by: Brandon Wylie <wylieb@chromium.org>
> > > Reviewed-by: Dirk Pranke <dpranke@google.com>
> > > Commit-Queue: Brandon Wylie <wylieb@chromium.org>
> > > Commit-Queue: Dirk Pranke <dpranke@google.com>
> > > Cr-Commit-Position: refs/heads/master@{#801977}
> >
> > R=​bsheedy@chromium.org
> > TBR=dpranke@google.com, wuwang@chromium.org, wylieb@chromium.org
> >
> > # Not skipping CQ checks because this is a reland.
> >
> > Bug: 1096656
> > Change-Id: If288ccd984009baacb9dda60f14aa548ec94a272
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2378892
> > Commit-Queue: Arthur Wang <wuwang@chromium.org>
> > Reviewed-by: Arthur Wang <wuwang@chromium.org>
> > Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
> > Reviewed-by: Dirk Pranke <dpranke@google.com>
> > Cr-Commit-Position: refs/heads/master@{#802390}
>
> TBR=dpranke@google.com,bsheedy@chromium.org,wylieb@chromium.org,wuwang@chromium.org
>
> Change-Id: Ia62b89df8121aa6cbf6726a81f8f64a400a49d33
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1096656
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2380742
> Reviewed-by: Xing Liu <xingliu@chromium.org>
> Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
> Commit-Queue: Xing Liu <xingliu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#802407}

R=bsheedy@chromium.com, dpranke@google.com
TBR=bsheedy@chromium.org, wuwang@chromium.org, wylieb@chromium.org, xingliu@chromium.org


Bug: 1096656
Change-Id: I58f357581ac01287e47dcf1473c4a1b83c94c0a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2380327Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Arthur Wang <wuwang@chromium.org>
Commit-Queue: Arthur Wang <wuwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802577}

chrome/test/data/android/manage_wpr_archives.py

@@ -7,10 +7,12 @@
 # for Android WPR record replay tests or upload any newly generated ones.
 
 import argparse
+import logging
 import os
 
 from upload_download_utils import download
 from upload_download_utils import upload
+from upload_download_utils import verify_file_exists
 
 STORAGE_BUCKET = 'chrome-wpr-archives'
 THIS_DIR = os.path.abspath(os.path.dirname(__file__))
@@ -39,6 +41,8 @@ def main():
     for d in WPR_RECORD_REPLAY_TEST_DIRECTORIES:
       download(d, _is_file_of_interest,
                'WPR archives', STORAGE_BUCKET)
+      if not verify_file_exists(d, _is_file_of_interest):
+        logging.error('There is not file of interest in dir {}'.format(d))
   else:
     for d in WPR_RECORD_REPLAY_TEST_DIRECTORIES:
       upload(d, _is_file_of_interest,

chrome/test/data/android/upload_download_utils.py

@@ -57,6 +57,16 @@ def download(directory, filter, scenario, gcs_bucket):
                   '%d: %s', scenario, directory, e.returncode, e.output)
 
 
+# TODO(crbug/1096656): Remove this after root cause is found.
+def verify_file_exists(directory, filter):
+  """Verifies that some file exists in the directory."""
+  for _, _, file_list in os.walk(directory):
+    for f in file_list:
+      if filter(f):
+        return True
+  return False
+
+
 def _get_files_to_delete(directory, filter):
   """Returns a list of local files to delete.
 

chrome/test/data/android/upload_download_utils_test.py

@@ -82,7 +82,8 @@ class UploadDownloadUtilsTest(fake_filesystem_unittest.TestCase):
         os.path.join(pathname, 'scenario1.wprgo'),
     ].sort()
     self.assertEquals(
-        upload_download_utils._get_files_to_upload(pathname, filter_all).sort(),
+        upload_download_utils._get_files_to_upload(
+            pathname, filter_all).sort(),
         expected_file_list)
 
     expected_file_list = [
@@ -90,8 +91,8 @@ class UploadDownloadUtilsTest(fake_filesystem_unittest.TestCase):
         os.path.join(pathname, 'image2.png'),
     ].sort()
     self.assertEquals(
-        upload_download_utils._get_files_to_upload(pathname, filter_image_only)
-            .sort(),
+        upload_download_utils._get_files_to_upload(
+            pathname, filter_image_only).sort(),
         expected_file_list)
 
   def test_get_files_to_upload_no_entries(self):
@@ -105,6 +106,56 @@ class UploadDownloadUtilsTest(fake_filesystem_unittest.TestCase):
         upload_download_utils._get_files_to_upload(pathname, filter_image_only),
         [])
 
+  def test_verify_file_exists_no_file(self):
+    pathname = os.path.join(THIS_DIR, 'tmp')
+    self.fs.create_dir(pathname)
+    self.assertFalse(upload_download_utils.verify_file_exists(
+        pathname, filter_all))
+
+  def test_verify_file_exists_in_top_directory(self):
+    pathname = os.path.join(THIS_DIR, 'tmp')
+    self.fs.create_dir(pathname)
+    self.fs.create_file(os.path.join(pathname, 'file'))
+    self.assertTrue(upload_download_utils.verify_file_exists(
+        pathname, filter_all))
+
+  def test_verify_file_exists_not_matching_filter(self):
+    pathname = os.path.join(THIS_DIR, 'tmp')
+    self.fs.create_dir(pathname)
+    self.fs.create_file(os.path.join(pathname, 'file'))
+    self.assertFalse(upload_download_utils.verify_file_exists(
+        pathname, filter_image_only))
+
+  def test_verify_file_exists_deep_in_directory(self):
+    pathname = os.path.join(THIS_DIR, 'tmp')
+    self.fs.create_dir(pathname)
+    pathname_2 = os.path.join(pathname, 'deep')
+    self.fs.create_dir(pathname_2)
+
+    pathname_3 = os.path.join(pathname_2, 'deeper')
+    self.fs.create_dir(pathname_3)
+
+    pathname_4 = os.path.join(pathname_3, 'deeper')
+    self.fs.create_dir(pathname_4)
+    self.fs.create_file(os.path.join(pathname_4, 'file'))
+    self.assertTrue(upload_download_utils.verify_file_exists(
+        pathname, filter_all))
+
+  def test_verify_file_exists_deep_in_directory_not_match_filter(self):
+    pathname = os.path.join(THIS_DIR, 'tmp')
+    self.fs.create_dir(pathname)
+    pathname_2 = os.path.join(pathname, 'deep')
+    self.fs.create_dir(pathname_2)
+
+    pathname_3 = os.path.join(pathname_2, 'deeper')
+    self.fs.create_dir(pathname_3)
+
+    pathname_4 = os.path.join(pathname_3, 'deeper')
+    self.fs.create_dir(pathname_4)
+    self.fs.create_file(os.path.join(pathname_4, 'file'))
+    self.assertFalse(upload_download_utils.verify_file_exists(
+        pathname, filter_image_only))
+
   @mock.patch.object(multiprocessing, 'cpu_count')
   def test_get_thread_count_not_implemented(self, multiplecore_mock):
     multiplecore_mock.side_effect = NotImplementedError('abc')