Only run Travis tests for plugins either added or modified by PR (#36)

* add option to specify more than one test file

* run tests of plugins changed by pr

* run all test files in plugin dir

* cleanup

* add notification that tests are running

* get_scoring_info -> get_plugin_info

* Documentation

Co-authored-by: Martin Schrimpf <[email protected]>

* Documentation

Co-authored-by: Martin Schrimpf <[email protected]>

* cleanup

* Documentation

Co-authored-by: Martin Schrimpf <[email protected]>

* Documentation

Co-authored-by: Martin Schrimpf <[email protected]>

* Refactor

Co-authored-by: Martin Schrimpf <[email protected]>

* Refactor

Co-authored-by: Martin Schrimpf <[email protected]>

* specify path for git cmd

* add plugin change parsing tests

* travis TEST

* change test SHAs

* change test SHAs

* Update brainscore_core/plugin_management/parse_plugin_changes.py

Co-authored-by: Martin Schrimpf <[email protected]>

* Update brainscore_core/plugin_management/parse_plugin_changes.py

Co-authored-by: Martin Schrimpf <[email protected]>

* Update brainscore_core/plugin_management/parse_plugin_changes.py

Co-authored-by: Martin Schrimpf <[email protected]>

* Update brainscore_core/plugin_management/parse_plugin_changes.py

Co-authored-by: Martin Schrimpf <[email protected]>

* Update brainscore_core/plugin_management/parse_plugin_changes.py

Co-authored-by: Martin Schrimpf <[email protected]>

* Update brainscore_core/plugin_management/parse_plugin_changes.py

Co-authored-by: Martin Schrimpf <[email protected]>

* Update brainscore_core/plugin_management/parse_plugin_changes.py

Co-authored-by: Martin Schrimpf <[email protected]>

* Update brainscore_core/plugin_management/parse_plugin_changes.py

Co-authored-by: Martin Schrimpf <[email protected]>

* Update tests/test_plugin_management/test_parse_plugin_changes.py

Co-authored-by: Martin Schrimpf <[email protected]>

* Update tests/test_plugin_management/test_parse_plugin_changes.py

Co-authored-by: Martin Schrimpf <[email protected]>

* Update tests/test_plugin_management/test_parse_plugin_changes.py

Co-authored-by: Martin Schrimpf <[email protected]>

* Update tests/test_plugin_management/test_parse_plugin_changes__brainscore_dummy/benchmarks/dummy_benchmark/__init__.py

Co-authored-by: Martin Schrimpf <[email protected]>

* Update tests/test_plugin_management/test_parse_plugin_changes__brainscore_dummy/models/dummy_model/__init__.py

Co-authored-by: Martin Schrimpf <[email protected]>

* Update tests/test_plugin_management/test_parse_plugin_changes__brainscore_dummy/benchmarks/dummy_benchmark/__init__.py

* Update brainscore_core/plugin_management/test_plugins.py

Co-authored-by: Martin Schrimpf <[email protected]>

* add comment re: printing output

* var naming

* test git access

* test git access

* exclude pr_only tests

* change  conditional to type

* split jobs

* format

* format

* stage jobs

* YAML Additions (#38)

* YAML changes

* YAML changes

* mark test_get_all_changed_files pr_only

* add pr_only to markers

---------

Co-authored-by: Katherine Fairchild <[email protected]>
Co-authored-by: Martin Schrimpf <[email protected]>
Co-authored-by: Qasim Abdullah <[email protected]>

.travis.yml

@@ -1,8 +1,4 @@
 language: python
-python:
-  - "3.7"
-  - "3.8"
-  - "3.9"
 before_install:
   - pip install poetry
 install:
@@ -16,5 +12,42 @@ install:
   - conda update -q conda
   - conda info -a
   - pip list
-script:
-  - pytest -m "not requires_gpu and not memory_intense and not slow and not travis_slow"
+jobs:
+  include:
+    - stage: "PR Build"
+      name: "Python 3.7"
+      python: 3.7
+      script:
+        - pytest -m "not requires_gpu and not memory_intense and not slow and not travis_slow"
+      if: type = pull_request
+    - stage: "PR Build"
+      name: "Python 3.8"
+      python: 3.8
+      script:
+        - pytest -m "not requires_gpu and not memory_intense and not slow and not travis_slow"
+      if: type = pull_request
+    - stage: "PR Build"
+      name: "Python 3.9"
+      python: 3.9
+      script:
+        - pytest -m "not requires_gpu and not memory_intense and not slow and not travis_slow"
+      if: type = pull_request
+
+    - stage: "Branch Build"
+      name: "Python 3.7"
+      python: 3.7
+      script:
+        - pytest -m "not pr_only and not requires_gpu and not memory_intense and not slow and not travis_slow"
+      if: type = push
+    - stage: "Branch Build"
+      name: "Python 3.8"
+      python: 3.8
+      script:
+        - pytest -m "not pr_only and not requires_gpu and not memory_intense and not slow and not travis_slow"
+      if: type = push
+    - stage: "Branch Build"
+      name: "Python 3.9"
+      python: 3.9
+      script:
+        - pytest -m "not pr_only and not requires_gpu and not memory_intense and not slow and not travis_slow"
+      if: type = push

brainscore_core/plugin_management/parse_plugin_changes.py

@@ -0,0 +1,142 @@
+import logging
+import re
+import subprocess
+import sys
+from pathlib import Path
+from typing import List, Tuple, Dict
+
+from .test_plugins import run_args
+
+_logger = logging.getLogger(__name__)
+
+PLUGIN_DIRS = ['models', 'benchmarks', 'data', 'metrics']
+
+
+def get_all_changed_files(commit_SHA: str, comparison_branch='main') -> List[str]:
+	"""
+	:return: a list of file paths, relative to the library root directory, e.g. `['models/mymodel/__init__.py', 'models/mymodel/model.py', 'models/mymodel/test.py']`
+	"""
+	core_dir = Path(__file__).parents[2]
+	cmd = f'git diff --name-only {comparison_branch} {commit_SHA} -C {core_dir}'
+	files_changed_bytes = subprocess.run(cmd, shell=True, stdout=subprocess.PIPE).stdout.splitlines()
+	files_changed = [f.decode() for f in files_changed_bytes]
+
+	return files_changed
+
+
+def separate_plugin_files(files: List[str]) -> Tuple[List[str], List[str]]:
+	"""
+	:return: one list of files that are located inside a plugin, and one list of files that are located outside of all plugins, 
+		e.g. `['models/mymodel/__init__.py', 'models/mymodel/model.py', 'models/mymodel/test.py'], ['model_helpers/make_model_brainlike.py']`
+	"""
+	plugin_files = []
+	non_plugin_files = []
+
+	for f in files:
+		subdir = f.split('/')[1] if len(f.split('/')) > 1 else None
+		if not any(plugin_dir == subdir for plugin_dir in PLUGIN_DIRS):
+			non_plugin_files.append(f)
+		else:
+			plugin_files.append(f)
+
+	return plugin_files, non_plugin_files
+
+
+def _plugin_name_from_path(path_relative_to_library: str) -> str:
+    """
+    Returns the name of the plugin from the given path. 
+    E.g. `_plugin_name_from_path("brainscore_vision/models/mymodel")` will return `"mymodel"`.
+    """
+    return path_relative_to_library.split('/')[2]
+
+
+def get_plugin_paths(plugin_files: List[str], domain_root: str) -> Dict[str, List[str]]:
+	"""
+	Returns a dictionary `plugin_type -> plugin names` with the full names of all plugin directories for each plugin_type
+	"""
+	plugins = {}
+	for plugin_type in PLUGIN_DIRS:
+		plugin_type_path = f'{domain_root}/{plugin_type}/'
+		plugin_paths = [fpath for fpath in plugin_files if fpath.startswith(plugin_type_path)]
+		plugins[plugin_type] = list(set([_plugin_name_from_path(fname) 
+			for fname in plugin_paths if f'/{plugin_type}/' in fname]))
+	return plugins
+
+
+def get_plugin_ids(plugin_type: str, new_plugin_dirs: List[str], domain_root: str) -> List[str]:
+	"""
+	Searches all __init.py__ files in `new_plugin_dirs` of `plugin_type` for registered plugins.
+	Returns list of identifiers for each registered plugin.
+	"""
+	plugin_ids = []
+
+	for plugin_dirname in new_plugin_dirs:
+		init_file = Path(f'{domain_root}/{plugin_type}/{plugin_dirname}/__init__.py')
+		with open(init_file) as f:
+			registry_name = plugin_type.strip(
+				's') + '_registry'  # remove plural and determine variable name, e.g. "models" -> "model_registry"
+			plugin_registrations = [line for line in f if f"{registry_name}["
+									in line.replace('\"', '\'')]
+			for line in plugin_registrations:
+				result = re.search(f'{registry_name}\[.*\]', line)
+				identifier = result.group(0)[len(registry_name) + 2:-2] # remove brackets and quotes
+				plugin_ids.append(identifier)
+
+	return plugin_ids
+
+
+def parse_plugin_changes(commit_SHA: str, domain_root: str) -> dict:
+	"""
+	Return information about which files changed by the invoking PR (compared against main) belong to plugins
+
+	:param commit_SHA: SHA of the invoking PR
+	:param domain_root: the root package directory of the repo where the PR originates, either 'brainscore' (vision) or 'brainscore_language' (language)
+	"""
+	plugin_info_dict = {}
+	changed_files = get_all_changed_files(commit_SHA)
+	changed_plugin_files, changed_non_plugin_files = separate_plugin_files(changed_files)	
+
+	plugin_info_dict["changed_plugins"] = get_plugin_paths(changed_plugin_files, domain_root)
+	plugin_info_dict["is_automergeable"] = str(num_changed_non_plugin_files > 0)
+
+	return plugin_info_dict
+
+
+def get_plugin_info(commit_SHA: str, domain_root: str):
+	"""
+	If any model or benchmark files changed, get plugin ids and set run_score to "True".
+	Otherwise set to "False".
+	Print all collected information about plugins.
+	"""
+	plugin_info_dict = parse_plugin_changes(commit_SHA, domain_root)
+
+	scoring_plugin_types = ("models", "benchmarks")
+	plugins_to_score = [plugin_info_dict["changed_plugins"][plugin_type] for plugin_type in scoring_plugin_types]
+
+	if len(plugins_to_score) > 0:
+		plugin_info_dict["run_score"] = "True"
+		for plugin_type in scoring_plugin_types:
+			scoring_plugin_ids = get_plugin_ids(plugin_type, plugin_info_dict["changed_plugins"][plugin_type], domain_root)
+			plugin_info_dict[f'new_{plugin_type}'] = ' '.join(scoring_plugin_ids)
+	else:
+		plugin_info_dict["run_score"] = "False"
+
+	print(plugin_info_dict) # output is accessed via print!
+
+
+def run_changed_plugin_tests(commit_SHA: str, domain_root: str):
+	"""
+	Initiates run of all tests in each changed plugin directory
+	"""
+	plugin_info_dict = parse_plugin_changes(commit_SHA, domain_root)
+
+	tests_to_run = []
+	for plugin_type in plugin_info_dict["changed_plugins"]:
+		changed_plugins = plugin_info_dict["changed_plugins"][plugin_type]
+		for plugin_dirname in changed_plugins:
+			root = Path(f'{domain_root}/{plugin_type}/{plugin_dirname}')
+			for filepath in root.rglob(r'test*.py'):
+				tests_to_run.append(str(filepath))
+
+	_logger.info("Running tests for new or modified plugins...")
+	run_args('brainscore_language', tests_to_run)

brainscore_core/plugin_management/test_plugins.py

@@ -3,7 +3,7 @@
 import warnings
 import yaml
 from pathlib import Path
-from typing import Dict, Union
+from typing import Dict, Union, List
 
 from .environment_manager import EnvironmentManager
 
@@ -108,21 +108,21 @@ def run_all_tests(root_directory: Path, results: Dict):
                 plugin_test_runner()
 
 
-def run_args(root_directory: Union[Path, str], test_file: Union[None, str] = None, test: Union[None, str] = None):
+def run_args(root_directory: Union[Path, str], test_files: Union[None, List[str]] = None, test: Union[None, str] = None):
     """
     Run single specified test or all tests for each plugin.
 
     :param root_directory: the directory containing all plugin types, e.g. `/local/brain-score_language/`
-    :param test_file: path of target test file (optional)
+    :param test_files: List of paths of target test files. If this is `None`, run all tests
     :param test: name of test to run (optional)
     """
     results = {}
-    if not test_file:
+    if not test_files:
         run_all_tests(root_directory=Path(root_directory), results=results)
-    elif test_file and Path(test_file).exists():
-        run_specified_tests(root_directory=Path(root_directory), test_file=test_file, results=results, test=test)
     else:
-        warnings.warn("Test file not found.")
+        for test_file in test_files:
+            assert Path(test_file).exists()
+            run_specified_tests(root_directory=Path(root_directory), test_file=test_file, results=results, test=test)
 
     plugins_with_errors = {k: v for k, v in results.items() if (v != 0) and (v != 5)}
     num_plugins_failed = len(plugins_with_errors)

tests/README.md

@@ -6,7 +6,8 @@ They are automatically downloaded by executing `bash download_test_files.sh`.
 ## Markers
 Unit tests have various markers that denote possible issues in the travis build:
 
-* **private_access**: tests that require access to a private ressource, such as assemblies on S3 (travis pull request builds can not have private access)
+* **pr_only**: tests that should only be run on pull requests
+* **private_access**: tests that require access to a private resource, such as assemblies on S3 (travis pull request builds can not have private access)
 * **memory_intense**: tests requiring more memory than is available in the travis sandbox (currently 3 GB, https://docs.travis-ci.com/user/common-build-problems/#my-build-script-is-killed-without-any-error)
 * **requires_gpu**: tests requiring a GPU to run or to run in a reasonable time (travis does not support GPUs/CUDA)
 * **slow**: tests leading to runtimes that are not possible on the openmind cluster (>1 hour per test) 

tests/test_plugin_management/test_parse_plugin_changes.py

@@ -0,0 +1,68 @@
+import pytest
+import shutil
+import sys
+import tempfile
+from pathlib import Path
+
+from brainscore_core.plugin_management.parse_plugin_changes import get_all_changed_files, separate_plugin_files, get_plugin_paths, get_plugin_ids
+
+DUMMY_FILES_CHANGED = ['brainscore_core/models/dummy_model/model.py', 
+                'brainscore_core/models/dummy_model/test.py', 
+                'brainscore_core/models/dummy_model/__init__.py',
+                'brainscore_core/benchmarks/dummy_benchmark/__init__.py',
+                'brainscore_core/__init__.py',
+                'brainscore_core/README.md']
+
+
+@pytest.mark.pr_only
+def test_git_access():
+
+    import subprocess
+    cmd = f'git diff --name-only 1ee0923234bd40126cff0d995d56c608a4a803a1 b55f3f3c5b4f30c0d1963e59f4a65432dfc90c31'
+    files_changed_bytes = subprocess.run(cmd, shell=True, stdout=subprocess.PIPE).stdout.splitlines()
+    files_changed = [f.decode() for f in files_changed_bytes]
+    assert set(['.travis.yml', 'README.md', 'pyproject.toml', 'setup.py']) == set(files_changed)
+    core_dir = Path(__file__).parents[2]
+    cmd = f'git diff --name-only 1ee0923234bd40126cff0d995d56c608a4a803a1 b55f3f3c5b4f30c0d1963e59f4a65432dfc90c31 -C {core_dir}'
+    files_changed_bytes = subprocess.run(cmd, shell=True, stdout=subprocess.PIPE).stdout.splitlines()
+    files_changed = [f.decode() for f in files_changed_bytes]
+    print(core_dir)
+    assert set(['.travis.yml', 'README.md', 'pyproject.toml', 'setup.py']) == set(files_changed)
+
+@pytest.mark.pr_only
+def test_get_all_changed_files():
+
+    commit_sha = '1ee0923234bd40126cff0d995d56c608a4a803a1'
+    comparison_branch = 'b55f3f3c5b4f30c0d1963e59f4a65432dfc90c31'
+    files_changed = get_all_changed_files(commit_sha, comparison_branch)
+    assert set(['.travis.yml', 'README.md', 'pyproject.toml', 'setup.py']) == set(files_changed)
+
+
+def test_separate_plugin_files():
+    plugin_files, non_plugin_files = separate_plugin_files(DUMMY_FILES_CHANGED)
+    assert set(['brainscore_core/models/dummy_model/model.py', 
+        'brainscore_core/models/dummy_model/test.py', 
+        'brainscore_core/models/dummy_model/__init__.py', 
+        'brainscore_core/benchmarks/dummy_benchmark/__init__.py']) == set(plugin_files)
+    assert set(['brainscore_core/__init__.py', 
+        'brainscore_core/README.md']) == set(non_plugin_files)
+
+
+def test_get_plugin_paths():
+    changed_plugins = get_plugin_paths(DUMMY_FILES_CHANGED, 'brainscore_core')
+    assert changed_plugins['models'][0] == 'dummy_model'
+    assert changed_plugins['benchmarks'][0] == 'dummy_benchmark'
+    assert len(changed_plugins['data']) + len(changed_plugins['metrics']) == 0
+
+
+def test_get_plugin_ids():
+
+    dummy_root = str(Path(__file__).parent / 'test_parse_plugin_changes__brainscore_dummy')
+    plugin_types = ['models', 'benchmarks']
+    for plugin_type in plugin_types:
+        plugin_id = f'dummy_{plugin_type}'.strip('s')
+        plugin_ids = get_plugin_ids(plugin_type, [plugin_id], dummy_root)
+        assert plugin_id == plugin_id
+
+
+

tests/test_plugin_management/test_parse_plugin_changes__brainscore_dummy/benchmarks/dummy_benchmark/__init__.py

@@ -0,0 +1,5 @@
+from brainscore_dummy import benchmark_registry
+
+from .benchmark import DummyBenchmark  # doesn't exist, but we only parse and never run this file
+
+benchmark_registry['dummy-benchmark'] = DummyBenchmark

tests/test_plugin_management/test_parse_plugin_changes__brainscore_dummy/models/dummy_model/__init__.py

@@ -0,0 +1,5 @@
+from brainscore_dummy import model_registry
+
+from .model import DummyModel  # doesn't exist, but we only parse and never run this file
+
+model_registry['dummy-model'] = DummyModel