GatherSymbolFiles: Specify that file collision handling is not needed.

Also, add and test helper functions.

BUG=chromium:1031380
TEST=pytest

Change-Id: I8b29f5a278afb9cf953ff4f8a2e6b96b854cbf4d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2558975
Tested-by: Michael Mortensen <mmortensen@google.com>
Commit-Queue: Michael Mortensen <mmortensen@google.com>
Reviewed-by: Stephane Belmon <sbelmon@google.com>
diff --git a/service/artifacts.py b/service/artifacts.py
index 184f727..6ba098d 100644
--- a/service/artifacts.py
+++ b/service/artifacts.py
@@ -729,13 +729,18 @@
   examined. A path to a directory will result in the directory being searched
   for .sym files. The generator yields SymbolFileTuple objects that contain
   symbol file references which are valid after this exits. Those files may exist
-  externally, or be created in the tempdir (when expanding tarballs).
+  externally, or be created in the tempdir (when expanding tarballs). Typical
+  usage in the BuildAPI will be for the .sym files to exist under a directory
+  such as /build/<board>/usr/lib/debug/breakpad so that the path to a sym file
+  will always be unique.
   Note: the caller must clean up the tempdir.
   Note: this function is recursive for tar files.
 
   Args:
     tempdir: Path to use for temporary files.
-    destdir: All .sym files are copied to this path.
+    destdir: All .sym files are copied to this path. Tarfiles are opened inside
+      a tempdir and any .sym files within them are copied to destdir from within
+      that temp path.
     paths: A list of input paths to walk. Files are returned based on .sym name
       w/out any checking internal symbol file format.
       Dirs are searched for files that end in ".sym". Urls are not handled.
@@ -760,11 +765,6 @@
             filename = os.path.join(root, f)
             relative_path = filename[len(p):].lstrip('/')
             try:
-              # TODO(crbug.com/1031380): Put calls to shutil.copy in a function
-              # that handles collisions in destdir due to different paths
-              # to the same filename (foo/a/a.sym from path foo, bar/a/a.sym
-              # from path bar) since the last shutil.copy will overwrite
-              # previous one.
               shutil.copy(filename, os.path.join(destdir, relative_path))
             except IOError:
               # Handles pre-3.3 Python where we may need to make the target
diff --git a/service/artifacts_unittest.py b/service/artifacts_unittest.py
index 69dbadb..4f0bd7c 100644
--- a/service/artifacts_unittest.py
+++ b/service/artifacts_unittest.py
@@ -876,6 +876,9 @@
     symbol_file_relative_paths.sort()
     self.assertEqual(symbol_file_relative_paths,
                      ['dir1/fileU.sym', 'fileD.sym'])
+    for symfile in symbol_file_relative_paths:
+      extension = symfile.split('.')[1]
+      self.assertEqual(extension, 'sym')
 
   def test_GatherSymbolFileFullFilePaths(self):
     """Test full filepaths (.sym and .txt) only gather .sym files."""
@@ -911,6 +914,17 @@
     files_in_output_dir = self.getFilesWithRelativeDir(output_dir)
     self.assertEqual(files_in_output_dir, ['a_file.sym'])
 
+  def test_IsTarball(self):
+    """Test IsTarball helper function."""
+    self.assertTrue(artifacts.IsTarball('file.tar'))
+    self.assertTrue(artifacts.IsTarball('file.tar.bz2'))
+    self.assertTrue(artifacts.IsTarball('file.tar.gz'))
+    self.assertTrue(artifacts.IsTarball('file.tbz'))
+    self.assertTrue(artifacts.IsTarball('file.txz'))
+    self.assertFalse(artifacts.IsTarball('file.txt'))
+    self.assertFalse(artifacts.IsTarball('file.tart'))
+    self.assertFalse(artifacts.IsTarball('file.bz2'))
+
   def getFilesWithRelativeDir(self, dest_dir):
     """Find all files below dest_dir using dir relative to dest_dir."""
     relative_files = []