From 546d8b047b975f3f3f798cf8ad83d8d4a6424613 Mon Sep 17 00:00:00 2001 From: kvfairchild Date: Thu, 10 Aug 2023 14:36:26 -0400 Subject: [PATCH] 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 * Documentation Co-authored-by: Martin Schrimpf * cleanup * Documentation Co-authored-by: Martin Schrimpf * Documentation Co-authored-by: Martin Schrimpf * Refactor Co-authored-by: Martin Schrimpf * Refactor Co-authored-by: Martin Schrimpf * 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 * Update brainscore_core/plugin_management/parse_plugin_changes.py Co-authored-by: Martin Schrimpf * Update brainscore_core/plugin_management/parse_plugin_changes.py Co-authored-by: Martin Schrimpf * Update brainscore_core/plugin_management/parse_plugin_changes.py Co-authored-by: Martin Schrimpf * Update brainscore_core/plugin_management/parse_plugin_changes.py Co-authored-by: Martin Schrimpf * Update brainscore_core/plugin_management/parse_plugin_changes.py Co-authored-by: Martin Schrimpf * Update brainscore_core/plugin_management/parse_plugin_changes.py Co-authored-by: Martin Schrimpf * Update brainscore_core/plugin_management/parse_plugin_changes.py Co-authored-by: Martin Schrimpf * Update tests/test_plugin_management/test_parse_plugin_changes.py Co-authored-by: Martin Schrimpf * Update tests/test_plugin_management/test_parse_plugin_changes.py Co-authored-by: Martin Schrimpf * Update tests/test_plugin_management/test_parse_plugin_changes.py Co-authored-by: Martin Schrimpf * Update tests/test_plugin_management/test_parse_plugin_changes__brainscore_dummy/benchmarks/dummy_benchmark/__init__.py Co-authored-by: Martin Schrimpf * Update tests/test_plugin_management/test_parse_plugin_changes__brainscore_dummy/models/dummy_model/__init__.py Co-authored-by: Martin Schrimpf * 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 * 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 Co-authored-by: Martin Schrimpf Co-authored-by: Qasim Abdullah <89213175+qasim-at-tci@users.noreply.github.com> --- .travis.yml | 45 +++++- .../plugin_management/parse_plugin_changes.py | 142 ++++++++++++++++++ .../plugin_management/test_plugins.py | 14 +- tests/README.md | 3 +- .../test_parse_plugin_changes.py | 68 +++++++++ .../benchmarks/dummy_benchmark/__init__.py | 5 + .../models/dummy_model/__init__.py | 5 + 7 files changed, 268 insertions(+), 14 deletions(-) create mode 100644 brainscore_core/plugin_management/parse_plugin_changes.py create mode 100644 tests/test_plugin_management/test_parse_plugin_changes.py create mode 100644 tests/test_plugin_management/test_parse_plugin_changes__brainscore_dummy/benchmarks/dummy_benchmark/__init__.py create mode 100644 tests/test_plugin_management/test_parse_plugin_changes__brainscore_dummy/models/dummy_model/__init__.py diff --git a/.travis.yml b/.travis.yml index 5f64aa94..0a746611 100644 --- a/.travis.yml +++ b/.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 diff --git a/brainscore_core/plugin_management/parse_plugin_changes.py b/brainscore_core/plugin_management/parse_plugin_changes.py new file mode 100644 index 00000000..889c53fc --- /dev/null +++ b/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) diff --git a/brainscore_core/plugin_management/test_plugins.py b/brainscore_core/plugin_management/test_plugins.py index 3aeab15e..d4651ca2 100644 --- a/brainscore_core/plugin_management/test_plugins.py +++ b/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) diff --git a/tests/README.md b/tests/README.md index 4a6de692..d3bed472 100644 --- a/tests/README.md +++ b/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) diff --git a/tests/test_plugin_management/test_parse_plugin_changes.py b/tests/test_plugin_management/test_parse_plugin_changes.py new file mode 100644 index 00000000..9d1e63fc --- /dev/null +++ b/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 + + + diff --git a/tests/test_plugin_management/test_parse_plugin_changes__brainscore_dummy/benchmarks/dummy_benchmark/__init__.py b/tests/test_plugin_management/test_parse_plugin_changes__brainscore_dummy/benchmarks/dummy_benchmark/__init__.py new file mode 100644 index 00000000..f2e6df48 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/tests/test_plugin_management/test_parse_plugin_changes__brainscore_dummy/models/dummy_model/__init__.py b/tests/test_plugin_management/test_parse_plugin_changes__brainscore_dummy/models/dummy_model/__init__.py new file mode 100644 index 00000000..1167c35b --- /dev/null +++ b/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 \ No newline at end of file