[Test refactor 2/5] Tests fetcher (#15726)

* Tests fetcher

* Review comments
Co-authored-by: sgugger <sylvain.gugger@gmail.com>
Review comments

utils/check_repo.py

@@ -93,18 +93,18 @@ IGNORE_NON_TESTED = PRIVATE_MODELS.copy() + [
 # Update this list with test files that don't have a tester with a `all_model_classes` variable and which don't
 # trigger the common tests.
 TEST_FILES_WITH_NO_COMMON_TESTS = [
-    "test_modeling_camembert.py",
-    "test_modeling_flax_mt5.py",
-    "test_modeling_mbart.py",
-    "test_modeling_mt5.py",
-    "test_modeling_pegasus.py",
-    "test_modeling_tf_camembert.py",
-    "test_modeling_tf_mt5.py",
-    "test_modeling_tf_xlm_roberta.py",
-    "test_modeling_xlm_prophetnet.py",
-    "test_modeling_xlm_roberta.py",
-    "test_modeling_vision_text_dual_encoder.py",
-    "test_modeling_flax_vision_text_dual_encoder.py",
+    "camembert/test_modeling_camembert.py",
+    "mt5/test_modeling_flax_mt5.py",
+    "mbart/test_modeling_mbart.py",
+    "mt5/test_modeling_mt5.py",
+    "pegasus/test_modeling_pegasus.py",
+    "camembert/test_modeling_tf_camembert.py",
+    "mt5/test_modeling_tf_mt5.py",
+    "xlm_roberta/test_modeling_tf_xlm_roberta.py",
+    "xlm_prophetnet/test_modeling_xlm_prophetnet.py",
+    "xlm_roberta/test_modeling_xlm_roberta.py",
+    "vision_text_dual_encoder/test_modeling_vision_text_dual_encoder.py",
+    "vision_text_dual_encoder/test_modeling_flax_vision_text_dual_encoder.py",
 ]
 
 # Update this list for models that are not in any of the auto MODEL_XXX_MAPPING. Being in this list is an exception and
@@ -295,13 +295,20 @@ def get_model_test_files():
         "test_modeling_tf_encoder_decoder",
     ]
     test_files = []
-    for filename in os.listdir(PATH_TO_TESTS):
-        if (
-            os.path.isfile(f"{PATH_TO_TESTS}/{filename}")
-            and filename.startswith("test_modeling")
-            and not os.path.splitext(filename)[0] in _ignore_files
-        ):
-            test_files.append(filename)
+    for file_or_dir in os.listdir(PATH_TO_TESTS):
+        path = os.path.join(PATH_TO_TESTS, file_or_dir)
+        if os.path.isdir(path):
+            filenames = [os.path.join(file_or_dir, file) for file in os.listdir(path)]
+        else:
+            filenames = [file_or_dir]
+
+        for filename in filenames:
+            if (
+                os.path.isfile(os.path.join(PATH_TO_TESTS, filename))
+                and "test_modeling" in filename
+                and not os.path.splitext(filename)[0] in _ignore_files
+            ):
+                test_files.append(filename)
     return test_files
 
 
@@ -356,9 +363,13 @@ def check_all_models_are_tested():
     test_files = get_model_test_files()
     failures = []
     for module in modules:
-        test_file = f"test_{module.__name__.split('.')[-1]}.py"
-        if test_file not in test_files:
+        test_file = [file for file in test_files if f"test_{module.__name__.split('.')[-1]}.py" in file]
+        if len(test_file) == 0:
             failures.append(f"{module.__name__} does not have its corresponding test file {test_file}.")
+        elif len(test_file) > 1:
+            failures.append(f"{module.__name__} has several test files: {test_file}.")
+        else:
+            test_file = test_file[0]
         new_failures = check_models_are_tested(module, test_file)
         if new_failures is not None:
             failures += new_failures

utils/tests_fetcher.py

@@ -195,12 +195,24 @@ def get_test_dependencies(test_fname):
         content = f.read()
 
     # Tests only have relative imports for other test files
-    relative_imports = re.findall(r"from\s+\.(\S+)\s+import\s+([^\n]+)\n", content)
+    # TODO Sylvain: handle relative imports cleanly
+    relative_imports = re.findall(r"from\s+(\.\S+)\s+import\s+([^\n]+)\n", content)
     relative_imports = [test for test, imp in relative_imports if "# tests_ignore" not in imp]
-    dependencies = [os.path.join("tests", f"{test}.py") for test in relative_imports]
-    # Some tests use docstrings with relative imports in them and this could catch them, so we check the files
-    # actually exist
-    return [f for f in dependencies if os.path.isfile(f)]
+
+    # Removes the double trailing '..' for parent imports, and creates an absolute path from the root dir with
+    # `tests` as a prefix.
+    parent_imports = [imp.strip(".") for imp in relative_imports if ".." in imp]
+    parent_imports = [os.path.join("tests", f"{test.replace('.', os.path.sep)}.py") for test in parent_imports]
+
+    # Removes the single trailing '.' for current dir imports, and creates an absolute path from the root dir with
+    # tests/{module_name} as a prefix.
+    current_dir_imports = [imp.strip(".") for imp in relative_imports if ".." not in imp]
+    directory = os.path.sep.join(test_fname.split(os.path.sep)[:-1])
+    current_dir_imports = [
+        os.path.join(directory, f"{test.replace('.', os.path.sep)}.py") for test in current_dir_imports
+    ]
+
+    return [f for f in [*parent_imports, *current_dir_imports] if os.path.isfile(f)]
 
 
 def create_reverse_dependency_map():
@@ -227,6 +239,8 @@ def create_reverse_dependency_map():
         something_changed = False
         for m in all_files:
             for d in direct_deps[m]:
+                if d not in direct_deps:
+                    raise ValueError(f"KeyError:{d}. From {m}")
                 for dep in direct_deps[d]:
                     if dep not in direct_deps[m]:
                         direct_deps[m].append(dep)
@@ -246,47 +260,55 @@ def create_reverse_dependency_map():
 # Any module file that has a test name which can't be inferred automatically from its name should go here. A better
 # approach is to (re-)name the test file accordingly, and second best to add the correspondence map here.
 SPECIAL_MODULE_TO_TEST_MAP = {
-    "commands/add_new_model_like.py": "test_add_new_model_like.py",
+    "commands/add_new_model_like.py": "utils/test_add_new_model_like.py",
     "configuration_utils.py": "test_configuration_common.py",
-    "convert_graph_to_onnx.py": "test_onnx.py",
-    "data/data_collator.py": "test_data_collator.py",
+    "convert_graph_to_onnx.py": "onnx/test_onnx.py",
+    "data/data_collator.py": "trainer/test_data_collator.py",
     "deepspeed.py": "deepspeed/",
     "feature_extraction_sequence_utils.py": "test_sequence_feature_extraction_common.py",
     "feature_extraction_utils.py": "test_feature_extraction_common.py",
-    "file_utils.py": ["test_file_utils.py", "test_model_output.py"],
-    "modelcard.py": "test_model_card.py",
+    "file_utils.py": ["utils/test_file_utils.py", "utils/test_model_output.py"],
+    "modelcard.py": "utils/test_model_card.py",
     "modeling_flax_utils.py": "test_modeling_flax_common.py",
-    "modeling_tf_utils.py": ["test_modeling_tf_common.py", "test_modeling_tf_core.py"],
-    "modeling_utils.py": ["test_modeling_common.py", "test_offline.py"],
-    "models/auto/modeling_auto.py": ["test_modeling_auto.py", "test_modeling_tf_pytorch.py", "test_modeling_bort.py"],
-    "models/auto/modeling_flax_auto.py": "test_modeling_flax_auto.py",
+    "modeling_tf_utils.py": ["test_modeling_tf_common.py", "utils/test_modeling_tf_core.py"],
+    "modeling_utils.py": ["test_modeling_common.py", "utils/test_offline.py"],
+    "models/auto/modeling_auto.py": [
+        "auto/test_modeling_auto.py",
+        "auto/test_modeling_tf_pytorch.py",
+        "bort/test_modeling_bort.py",
+    ],
+    "models/auto/modeling_flax_auto.py": "auto/test_modeling_flax_auto.py",
     "models/auto/modeling_tf_auto.py": [
-        "test_modeling_tf_auto.py",
-        "test_modeling_tf_pytorch.py",
-        "test_modeling_tf_bort.py",
+        "auto/test_modeling_tf_auto.py",
+        "auto/test_modeling_tf_pytorch.py",
+        "bort/test_modeling_tf_bort.py",
     ],
-    "models/blenderbot_small/tokenization_blenderbot_small.py": "test_tokenization_small_blenderbot.py",
-    "models/blenderbot_small/tokenization_blenderbot_small_fast.py": "test_tokenization_small_blenderbot.py",
-    "models/gpt2/modeling_gpt2.py": ["test_modeling_gpt2.py", "test_modeling_megatron_gpt2.py"],
-    "pipelines/base.py": "test_pipelines_*.py",
+    "models/gpt2/modeling_gpt2.py": ["gpt2/test_modeling_gpt2.py", "megatron_gpt2/test_modeling_megatron_gpt2.py"],
+    "optimization.py": "optimization/test_optimization.py",
+    "optimization_tf.py": "optimization/test_optimization_tf.py",
+    "pipelines/base.py": "pipelines/test_pipelines_*.py",
     "pipelines/text2text_generation.py": [
-        "test_pipelines_text2text_generation.py",
-        "test_pipelines_summarization.py",
-        "test_pipelines_translation.py",
+        "pipelines/test_pipelines_text2text_generation.py",
+        "pipelines/test_pipelines_summarization.py",
+        "pipelines/test_pipelines_translation.py",
+    ],
+    "pipelines/zero_shot_classification.py": "pipelines/test_pipelines_zero_shot.py",
+    "testing_utils.py": "utils/test_skip_decorators.py",
+    "tokenization_utils.py": ["test_tokenization_common.py", "tokenization/test_tokenization_utils.py"],
+    "tokenization_utils_base.py": ["test_tokenization_common.py", "tokenization/test_tokenization_utils.py"],
+    "tokenization_utils_fast.py": [
+        "test_tokenization_common.py",
+        "tokenization/test_tokenization_utils.py",
+        "tokenization/test_tokenization_fast.py",
     ],
-    "pipelines/zero_shot_classification.py": "test_pipelines_zero_shot.py",
-    "testing_utils.py": "test_skip_decorators.py",
-    "tokenization_utils.py": "test_tokenization_common.py",
-    "tokenization_utils_base.py": "test_tokenization_common.py",
-    "tokenization_utils_fast.py": "test_tokenization_fast.py",
     "trainer.py": [
-        "test_trainer.py",
+        "trainer/test_trainer.py",
         "extended/test_trainer_ext.py",
-        "test_trainer_distributed.py",
-        "test_trainer_tpu.py",
+        "trainer/test_trainer_distributed.py",
+        "trainer/test_trainer_tpu.py",
     ],
-    "train_pt_utils.py": "test_trainer_utils.py",
-    "utils/versions.py": "test_versions_utils.py",
+    "train_pt_utils.py": "trainer/test_trainer_utils.py",
+    "utils/versions.py": "utils/test_versions_utils.py",
 }
 
 
@@ -311,21 +333,27 @@ def module_to_test_file(module_fname):
 
     # Special case for pipelines submodules
     if len(splits) >= 2 and splits[-2] == "pipelines":
-        default_test_file = f"tests/test_pipelines_{module_name}"
+        default_test_file = f"tests/pipelines/test_pipelines_{module_name}"
     # Special case for benchmarks submodules
     elif len(splits) >= 2 and splits[-2] == "benchmark":
-        return ["tests/test_benchmark.py", "tests/test_benchmark_tf.py"]
+        return ["tests/benchmark/test_benchmark.py", "tests/benchmark/test_benchmark_tf.py"]
     # Special case for commands submodules
     elif len(splits) >= 2 and splits[-2] == "commands":
-        return "tests/test_cli.py"
+        return "tests/utils/test_cli.py"
     # Special case for onnx submodules
     elif len(splits) >= 2 and splits[-2] == "onnx":
-        return ["tests/test_onnx.py", "tests/test_onnx_v2.py"]
+        return ["tests/onnx/test_onnx.py", "tests/onnx/test_onnx_v2.py"]
     # Special case for utils (not the one in src/transformers, the ones at the root of the repo).
     elif len(splits) > 0 and splits[0] == "utils":
-        default_test_file = f"tests/test_utils_{module_name}"
+        default_test_file = f"tests/utils/test_utils_{module_name}"
+    elif len(splits) > 4 and splits[2] == "models":
+        default_test_file = f"tests/{splits[3]}/test_{module_name}"
+    elif len(splits) > 2 and splits[2].startswith("generation"):
+        default_test_file = f"tests/generation/test_{module_name}"
+    elif len(splits) > 2 and splits[2].startswith("trainer"):
+        default_test_file = f"tests/trainer/test_{module_name}"
     else:
-        default_test_file = f"tests/test_{module_name}"
+        default_test_file = f"tests/utils/test_{module_name}"
 
     if os.path.isfile(default_test_file):
         return default_test_file
@@ -340,8 +368,8 @@ def module_to_test_file(module_fname):
 # This list contains the list of test files we expect never to be launched from a change in a module/util. Those are
 # launched separately.
 EXPECTED_TEST_FILES_NEVER_TOUCHED = [
-    "tests/test_doc_samples.py",  # Doc tests
-    "tests/test_pipelines_common.py",  # Actually checked by the pipeline based file
+    "tests/utils/test_doc_samples.py",  # Doc tests
+    "tests/pipelines/test_pipelines_common.py",  # Actually checked by the pipeline based file
     "tests/sagemaker/test_single_node_gpu.py",  # SageMaker test
     "tests/sagemaker/test_multi_node_model_parallel.py",  # SageMaker test
     "tests/sagemaker/test_multi_node_data_parallel.py",  # SageMaker test