changed filter to be more conservative - any no-whitelisted file runs all tests

2 files changed, 47 insertions, 21 deletions

diff --git a/tools/run_tests/filter_pull_request_tests.py b/tools/run_tests/filter_pull_request_tests.py
index fe8b8ed0f3..29dbd97825 100644
--- a/tools/run_tests/filter_pull_request_tests.py
+++ b/tools/run_tests/filter_pull_request_tests.py
@@ -33,7 +33,8 @@
 from subprocess import call, check_output
 
 # Whitelist for all tests
-# Update all instances in corresponding trigger lists when modifying this
+# If whitelist item should only trigger some tests, the item should be
+# added to this list and the trigger list of tests that should be run
 starts_with_whitelist = ['templates/',
                          'doc/',
                          'examples/',
@@ -110,16 +111,18 @@ def _filter_whitelist(whitelist, triggers):
   return filtered_whitelist
 
 
-def _get_changed_files():
+def _get_changed_files(base_branch):
   """
   Get list of changed files between current branch and base of target merge branch
   """
   # git fetch might need to be called on Jenkins slave
   # todo(mattkwong): remove or uncomment below after seeing if Jenkins needs this
   # call(['git', 'fetch'])
-  # this also collects files that are changed in the repo but not updated in the branch
-  # todo(mattkwong): change this to only collect changes files compared to base and not hardcode branch
-  return check_output(["git", "diff", "--name-only", "..origin/master"]).splitlines()
+
+  # get file changes between branch and merge-base of specified branch
+  # not combined to be Windows friendly
+  base_commit = check_output(["git", "merge-base", base_branch, "HEAD"]).rstrip()
+  return check_output(["git", "diff", base_commit, "--name-only"]).splitlines()
 
 
 def _can_skip_tests(file_names, starts_with_whitelist=[], ends_with_whitelist=[]):
@@ -134,7 +137,6 @@ def _can_skip_tests(file_names, starts_with_whitelist=[], ends_with_whitelist=[]
   # convert lists to tuple to pass into str.startswith() and str.endswith()
   starts_with_whitelist = tuple(starts_with_whitelist)
   ends_with_whitelist = tuple(ends_with_whitelist)
-  print (starts_with_whitelist)
   for file_name in file_names:
     if starts_with_whitelist and not file_name.startswith(starts_with_whitelist) and \
        ends_with_whitelist and not file_name.endswith(ends_with_whitelist):
@@ -144,28 +146,48 @@ def _can_skip_tests(file_names, starts_with_whitelist=[], ends_with_whitelist=[]
 
 def _remove_irrelevant_tests(tests, tag):
   """
-  Filters out tests by config or language
+  Filters out tests by config or language - will not remove sanitizer tests
   :param tests: list of all tests generated by run_tests_matrix.py
   :param tag: string representing language or config to filter - "_(language)_" or "_(config)"
   :return: list of relevant tests
   """
   # todo(mattkwong): find a more reliable way to filter tests - don't use shortname
-  return [test for test in tests if not tag in test.shortname]
+  return [test for test in tests if
+          tag not in test.shortname or
+          '_msan' in test.shortname or
+          '_asan' in test.shortname or
+          '_tsan' in test.shortname]
 
 
-def filter_tests(tests):
+def _remove_irrelevant_sanitizer_tests(tests, language_tag=""):
+  """
+  Filters out sanitizer tests - can specify a language to filter - this should be c++ only
+  :param tests: list of all tests generated by run_tests_matrix.py
+  :param language_tag: string specifying a language from which to filter sanitizer tests - "_(language)_"
+  :return: list of relevant tests
+  """
+  if language_tag:
+    return [test for test in tests if not language_tag in test.shortname and
+            not '_asan' in test.shortname and
+            not '_msan' in test.shortname and
+            not '_tsan' in test.shortname]
+  else:
+    return [test for test in tests if
+            '_asan' not in test.shortname and
+            '_msan' not in test.shortname and
+            '_tsan' not in test.shortname]
+
+def filter_tests(tests, base_branch):
   """
   Filters out tests that are safe to ignore
   :param tests: list of all tests generated by run_tests_matrix.py
   :return: list of relevant tests
   """
-  print("Finding file differences between grpc:master repo and pull request...")
-  changed_files = _get_changed_files()
+  print("Finding file differences between %s repo and current branch..." % base_branch)
+  changed_files = _get_changed_files(base_branch)
   for changed_file in changed_files:
     print(changed_file)
 
-  changed_files = ['src/ruby/dgf']
-
   # Filter core tests
   skip_core = _can_skip_tests(changed_files,
                               starts_with_whitelist=_filter_whitelist(starts_with_whitelist, core_starts_with_triggers),
@@ -178,13 +200,12 @@ def filter_tests(tests):
                              starts_with_whitelist=_filter_whitelist(starts_with_whitelist, cpp_starts_with_triggers),
                              ends_with_whitelist=ends_with_whitelist)
   if skip_cpp:
-    tests = _remove_irrelevant_tests(tests, '_cpp_')
+    tests = _remove_irrelevant_tests(tests, '_c++_')
+    tests = _remove_irrelevant_sanitizer_tests(tests, language_tag='_c++_')
 
-  # Tsan, msan, and asan tests skipped if core and c++ are skipped
+  # Sanitizer tests skipped if core and c++ are skipped
   if skip_core and skip_cpp:
-    tests = _remove_irrelevant_tests(tests, '_tsan')
-    tests = _remove_irrelevant_tests(tests, '_msan')
-    tests = _remove_irrelevant_tests(tests, '_asan')
+    tests = _remove_irrelevant_sanitizer_tests(tests)
 
   # Filter c# tests
   skip_csharp = _can_skip_tests(changed_files,
@@ -213,6 +234,7 @@ def filter_tests(tests):
                              ends_with_whitelist=ends_with_whitelist)
   if skip_php:
     tests = _remove_irrelevant_tests(tests, '_php_')
+    tests = _remove_irrelevant_tests(tests, '_php7_')
 
   # Filter python tests
   skip_python = _can_skip_tests(changed_files,
diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py
index 00a2d65d3d..3cea0f5011 100755
--- a/tools/run_tests/run_tests_matrix.py
+++ b/tools/run_tests/run_tests_matrix.py
@@ -235,6 +235,10 @@ argp.add_argument('--filter_pr_tests',
 		  action='store_const',	
 		  const=True,	  
 		  help='Filters out tests irrelavant to pull request changes.')
+argp.add_argument('--base_branch',
+                  default='origin/master',
+                  type=str,
+                  help='Branch that pull request is requesting to merge into')
 args = argp.parse_args()
 
 extra_args = []
@@ -270,12 +274,12 @@ print
 
 if args.filter_pr_tests:
   print 'IMPORTANT: Test filtering is not active; this is only for testing.'
-  relevant_jobs = filter_tests(jobs)
+  relevant_jobs = filter_tests(jobs, args.base_branch)
   print
   if len(relevant_jobs) == len(jobs):
-    print 'No tests were filtered.'
+    print '(TESTING) No tests will be skipped.'
   else:
-    print 'These tests were filtered:'
+    print '(TESTING) These tests will be skipped:'
     for job in list(set(jobs) - set(relevant_jobs)):
       print '  %s' % job.shortname
   print