From 99d40d7554ae912f09673bd46aafa9c929e47eb7 Mon Sep 17 00:00:00 2001
From: Luca Farsi <lucafarsi@google.com>
Date: Tue, 6 Aug 2024 13:35:28 -0700
Subject: [PATCH] Change artifact matching to be more strict

The current artifact matching logic will cause any test target that uses
something like .*-tests.zip to be way too broad in what it causes to
build. Change the logic to look for the target name in the artifact
matching regex to make it so that only tests that look for a specific
target to match.

Test: atest build_test_suites_test
Bug: 348489774
Change-Id: Ia75e38b676607f45f2b1c8fcf948045c248f1729
---
 ci/build_test_suites.py      | 38 +++++++-----------------------------
 ci/build_test_suites_test.py | 19 ++++++++++++++++++
 2 files changed, 26 insertions(+), 31 deletions(-)

diff --git a/ci/build_test_suites.py b/ci/build_test_suites.py
index 75dd9f2f70..deb1f1d0fb 100644
--- a/ci/build_test_suites.py
+++ b/ci/build_test_suites.py
@@ -81,6 +81,7 @@ class BuildPlanner:
 
     build_targets = set()
     packaging_functions = set()
+    self.file_download_options = self._aggregate_file_download_options()
     for target in self.args.extra_targets:
       if self._unused_target_exclusion_enabled(
           target
@@ -107,49 +108,24 @@ class BuildPlanner:
 
   def _build_target_used(self, target: str) -> bool:
     """Determines whether this target's outputs are used by the test configurations listed in the build context."""
-    file_download_regexes = self._aggregate_file_download_regexes()
     # For all of a targets' outputs, check if any of the regexes used by tests
     # to download artifacts would match it. If any of them do then this target
     # is necessary.
-    for artifact in self._get_target_potential_outputs(target):
-      for regex in file_download_regexes:
-        if re.match(regex, artifact):
-          return True
-    return False
+    regex = r'\b(%s)\b' % re.escape(target)
+    return any(re.search(regex, opt) for opt in self.file_download_options)
 
-  def _get_target_potential_outputs(self, target: str) -> set[str]:
-    tests_suffix = '-tests'
-    if target.endswith('tests'):
-      tests_suffix = ''
-    # This is a list of all the potential zips output by the test suite targets.
-    # If the test downloads artifacts from any of these zips, we will be
-    # conservative and avoid skipping the tests.
-    return {
-        f'{target}.zip',
-        f'android-{target}.zip',
-        f'android-{target}-verifier.zip',
-        f'{target}{tests_suffix}_list.zip',
-        f'android-{target}{tests_suffix}_list.zip',
-        f'{target}{tests_suffix}_host-shared-libs.zip',
-        f'android-{target}{tests_suffix}_host-shared-libs.zip',
-        f'{target}{tests_suffix}_configs.zip',
-        f'android-{target}{tests_suffix}_configs.zip',
-    }
-
-  def _aggregate_file_download_regexes(self) -> set[re.Pattern]:
+  def _aggregate_file_download_options(self) -> set[str]:
     """Lists out all test config options to specify targets to download.
 
     These come in the form of regexes.
     """
-    all_regexes = set()
+    all_options = set()
     for test_info in self._get_test_infos():
       for opt in test_info.get('extraOptions', []):
         # check the known list of options for downloading files.
         if opt.get('key') in self._DOWNLOAD_OPTS:
-          all_regexes.update(
-              re.compile(value) for value in opt.get('values', [])
-          )
-    return all_regexes
+          all_options.update(opt.get('values', []))
+    return all_options
 
   def _get_test_infos(self):
     return self.build_context.get('testContext', dict()).get('testInfos', [])
diff --git a/ci/build_test_suites_test.py b/ci/build_test_suites_test.py
index 25c072e2b0..463bdd0d73 100644
--- a/ci/build_test_suites_test.py
+++ b/ci/build_test_suites_test.py
@@ -380,6 +380,25 @@ class BuildPlannerTest(unittest.TestCase):
 
     self.assertSetEqual(build_plan.build_targets, set())
 
+  def test_target_regex_matching_not_too_broad(self):
+    build_target = 'test_target'
+    test_context = self.get_test_context(build_target)
+    test_context['testInfos'][0]['extraOptions'] = [{
+        'key': 'additional-files-filter',
+        'values': [f'.*a{build_target}.*\.zip'],
+    }]
+    build_planner = self.create_build_planner(
+        build_targets={build_target},
+        build_context=self.create_build_context(
+            test_context=test_context,
+            enabled_build_features={'test_target_unused_exclusion'},
+        ),
+    )
+
+    build_plan = build_planner.create_build_plan()
+
+    self.assertSetEqual(build_plan.build_targets, set())
+
   def create_build_planner(
       self,
       build_targets: set[str],