Repository: huggingface/transformers
Pull request: run all test files on CircleCI

# What does this PR do?

Maybe we need to verify that there is at least one python files being modified.

(i.e. if only doc MD files are modified, don't run)

Changed files:
filename: tests/repo_utils/test_tests_fetcher.py
patch: @@ -645,7 +645,7 @@ def test_infer_tests_to_run(self):
             assert set(example_tests_to_run.split(" ")) == example_tests
 
             with patch_transformer_repo_path(tmp_folder):
-                infer_tests_to_run(tmp_folder / "test-output.txt", filter_models=False)
+                infer_tests_to_run(tmp_folder / "test-output.txt")
                 with open(tmp_folder / "test-output.txt") as f:
                     tests_to_run = f.read()
                 with open(tmp_folder / "examples_test_list.txt") as f:

filename: utils/tests_fetcher.py
patch: @@ -59,8 +59,8 @@
 
 from git import Repo
 
+
 # List here the models not to be filtered by `filter_tests`.
-from important_files import IMPORTANT_MODELS
 
 
 PATH_TO_REPO = Path(__file__).parent.parent.resolve()
@@ -69,8 +69,16 @@
 PATH_TO_TESTS = PATH_TO_REPO / "tests"
 
 # The value is just a heuristic to determine if we `guess` all models are impacted.
-# This variable has effect only if `filter_models=False`.
-NUM_MODELS_TO_TRIGGER_FULL_CI = 30
+NUM_MODELS_TO_TRIGGER_FULL_CI = 15
+
+# A list of very important files that should trigger all tests if modified (because they can have impact almost anywhere
+# in the library)
+CORE_FILES = (
+    "setup.py",
+    ".circleci/create_circleci_config.py",
+    "src/transformers/modeling_utils.py",
+    "src/transformers/core_model_loading.py",
+)
 
 
 @contextmanager
@@ -876,18 +884,14 @@ def create_reverse_dependency_map() -> dict[str, list[str]]:
     return reverse_map
 
 
-def create_module_to_test_map(
-    reverse_map: dict[str, list[str]] | None = None, filter_models: bool = False
-) -> dict[str, list[str]]:
+def create_module_to_test_map(reverse_map: dict[str, list[str]] | None = None) -> dict[str, list[str]]:
     """
     Extract the tests from the reverse_dependency_map and potentially filters the model tests.
 
     Args:
         reverse_map (`Dict[str, List[str]]`, *optional*):
             The reverse dependency map as created by `create_reverse_dependency_map`. Will default to the result of
             that function if not provided.
-        filter_models (`bool`, *optional*, defaults to `False`):
-            Whether or not to filter model tests to only include core models if a file impacts a lot of models.
 
     Returns:
         `Dict[str, List[str]]`: A dictionary that maps each file to the tests to execute if that file was modified.
@@ -906,38 +910,7 @@ def is_test(fname):
     # Build the test map
     test_map = {module: [f for f in deps if is_test(f)] for module, deps in reverse_map.items()}
 
-    if not filter_models:
-        return test_map
-
-    # Now we deal with the filtering if `filter_models` is True.
-    num_model_tests = len(list(PATH_TO_TESTS.glob("models/*")))
-
-    def has_many_models(tests):
-        # We filter to core models when a given file impacts more than half the model tests.
-        model_tests = {Path(t).parts[2] for t in tests if t.startswith("tests/models/")}
-        return len(model_tests) > num_model_tests // 2
-
-    # for each module (if specified in the argument `module`) of the form `models/my_model` (i.e. starting with it),
-    # we always keep the tests (those are already in the argument `tests`) which are in `tests/models/my_model`.
-    # This is to avoid them being excluded when a module has many impacted tests: the directly related test files should
-    # always be included!
-    def filter_tests(tests, module=""):
-        filtered_tests = []
-        for t in tests:
-            if (
-                not t.startswith("tests/models/")
-                or Path(t).parts[2] in IMPORTANT_MODELS
-                # at this point, `t` is of the form `tests/models/my_model`, and we check if `models/my_model`
-                # (i.e. `parts[1:3]`) is in `module`.
-                or "/".join(Path(t).parts[1:3]) in module
-            ):
-                filtered_tests += [t]
-        return filtered_tests
-
-    return {
-        module: (filter_tests(tests, module=module) if has_many_models(tests) else tests)
-        for module, tests in test_map.items()
-    }
+    return test_map
 
 
 def _print_list(l) -> str:
@@ -947,9 +920,7 @@ def _print_list(l) -> str:
     return "\n".join([f"- {f}" for f in l])
 
 
-def infer_tests_to_run(
-    output_file: str, diff_with_last_commit: bool = False, filter_models: bool = False, test_all: bool = False
-):
+def infer_tests_to_run(output_file: str, diff_with_last_commit: bool = False, test_all: bool = False):
     """
     The main function called by the test fetcher. Determines the tests to run from the diff.
 
@@ -965,9 +936,6 @@ def infer_tests_to_run(
         diff_with_last_commit (`bool`, *optional*, defaults to `False`):
             Whether to analyze the diff with the last commit (for use on the main branch after a PR is merged) or with
             the branching point from main (for use on each PR).
-        filter_models (`bool`, *optional*, defaults to `True`):
-            Whether or not to filter the tests to core models only, when a file modified results in a lot of model
-            tests.
     """
     if not test_all:
         modified_files = get_modified_python_files(diff_with_last_commit=diff_with_last_commit)
@@ -989,25 +957,24 @@ def infer_tests_to_run(
     model_impacted = {"/".join(x.split("/")[:3]) for x in impacted_files if x.startswith("tests/models/")}
     # Grab the corresponding test files:
     if (
-        any(x in modified_files for x in ["setup.py", ".circleci/create_circleci_config.py"])
-        or not filter_models
-        and len(model_impacted) >= NUM_MODELS_TO_TRIGGER_FULL_CI
+        any(file in CORE_FILES for file in modified_files)
+        or len(model_impacted) >= NUM_MODELS_TO_TRIGGER_FULL_CI
         or commit_flags["test_all"]
     ):
         test_files_to_run = glob.glob("tests/**/test_**.py", recursive=True) + glob.glob(
             "examples/**/*.py", recursive=True
         )
-        if len(model_impacted) >= NUM_MODELS_TO_TRIGGER_FULL_CI and filter_models:
+        if len(model_impacted) >= NUM_MODELS_TO_TRIGGER_FULL_CI:
             print(
-                f"More than {NUM_MODELS_TO_TRIGGER_FULL_CI - 1} models are impacted and `filter_models=False`. CI is configured to test everything."
+                f"More than {NUM_MODELS_TO_TRIGGER_FULL_CI - 1} models are impacted. CI is configured to test everything."
             )
     else:
         # All modified tests need to be run.
         test_files_to_run = [f for f in modified_files if f.startswith("tests") and "/test_" in f]
         impacted_files = get_impacted_files_from_tiny_model_summary(diff_with_last_commit=diff_with_last_commit)
 
         # Then we grab the corresponding test files.
-        test_map = create_module_to_test_map(reverse_map=reverse_map, filter_models=filter_models)
+        test_map = create_module_to_test_map(reverse_map=reverse_map)
         for f in modified_files + impacted_files:
             if f in test_map:
                 test_files_to_run.extend(test_map[f])
@@ -1180,7 +1147,6 @@ def create_test_list_from_filter(full_test_list, out_path):
         infer_tests_to_run(
             args.output_file,
             diff_with_last_commit=diff_with_last_commit,
-            filter_models=False,
             test_all=commit_flags["test_all"],
         )
         filter_tests(args.output_file, ["repo_utils"])