From f47629a0fa49ab7b19aaea1bab67190814f60414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20Kir=C3=A1ly?= Date: Wed, 13 Sep 2023 12:37:09 +0200 Subject: [PATCH] [MNT] Differential testing of estimators (#96) This PR introduces the differential testing from `sktime` to the PR CI, to test only estimators from modules that have changed. This does not affect the release CI, which runs the tests for all estimators in `sktime`. --- .github/workflows/test.yml | 18 ++++++- conftest.py | 30 +++++++++++ setup.cfg | 1 + skpro/tests/test_all_estimators.py | 22 +++++++- skpro/utils/git_diff.py | 80 ++++++++++++++++++++++++++++++ 5 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 conftest.py create mode 100644 skpro/utils/git_diff.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 20d40129a..5ab6c2130 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -72,7 +72,11 @@ jobs: os: [ubuntu-20.04, windows-latest, macOS-11] runs-on: ${{ matrix.os }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 + + - run: git remote set-branches origin 'main' + + - run: git fetch --depth 1 - name: Set up Python uses: actions/setup-python@v4 @@ -89,6 +93,9 @@ jobs: - name: Show dependencies run: python -m pip list + - name: Show available branches + run: git branch -a + - name: Run tests run: make test @@ -104,7 +111,11 @@ jobs: os: [ubuntu-20.04, windows-latest, macOS-11] runs-on: ${{ matrix.os }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 + + - run: git remote set-branches origin 'main' + + - run: git fetch --depth 1 - name: Set up Python uses: actions/setup-python@v4 @@ -121,6 +132,9 @@ jobs: - name: Show dependencies run: python -m pip list + - name: Show available branches + run: git branch -a + - name: Run tests run: make test diff --git a/conftest.py b/conftest.py new file mode 100644 index 000000000..53a23a7ac --- /dev/null +++ b/conftest.py @@ -0,0 +1,30 @@ +"""Main configuration file for pytest. + +Contents: +adds an --only_changed_modules option to pytest +this allows to turn on/off differential testing (for shorter runtime) +"on" condition ensures that only estimators are tested that have changed, + more precisely, only estimators whose class is in a module + that has changed compared to the main branch +by default, this is off, including for default local runs of pytest +""" +# copyright: skpro developers, BSD-3-Clause License (see LICENSE file) + +__author__ = ["fkiraly"] + + +def pytest_addoption(parser): + """Pytest command line parser options adder.""" + parser.addoption( + "--only_changed_modules", + default=False, + help="test only estimators from modules that have changed compared to main", + ) + + +def pytest_configure(config): + """Pytest configuration preamble.""" + from skpro.tests import test_all_estimators + + if config.getoption("--only_changed_modules") in [True, "True"]: + test_all_estimators.ONLY_CHANGED_MODULES = True diff --git a/setup.cfg b/setup.cfg index f4e22826e..0bee5cf63 100644 --- a/setup.cfg +++ b/setup.cfg @@ -18,6 +18,7 @@ addopts = --cov-report xml --cov-report html --showlocals + --only_changed_modules True -n auto filterwarnings = ignore::UserWarning diff --git a/skpro/tests/test_all_estimators.py b/skpro/tests/test_all_estimators.py index 7ae206337..3133a09de 100644 --- a/skpro/tests/test_all_estimators.py +++ b/skpro/tests/test_all_estimators.py @@ -8,6 +8,11 @@ from skbase.testing.utils.inspect import _get_args from skpro.registry import OBJECT_TAG_LIST +from skpro.utils.git_diff import is_class_changed + +# whether to test only estimators from modules that are changed w.r.t. main +# default is False, can be set to True by pytest --only_changed_modules True flag +ONLY_CHANGED_MODULES = False class PackageConfig: @@ -29,7 +34,22 @@ class PackageConfig: valid_tags = OBJECT_TAG_LIST -class TestAllObjects(PackageConfig, _TestAllObjects): +class BaseFixtureGenerator: + """Base class for fixture generation, overrides skbase object retrieval.""" + + def _all_objects(self): + """Retrieve list of all object classes of type self.object_type_filter.""" + obj_list = super()._all_objects() + + # this setting ensures that only estimators are tested that have changed + # in the sense that any line in the module is different from main + if ONLY_CHANGED_MODULES: + obj_list = [obj for obj in obj_list if is_class_changed(obj)] + + return obj_list + + +class TestAllObjects(PackageConfig, BaseFixtureGenerator, _TestAllObjects): """Generic tests for all objects in the mini package.""" # override this due to reserved_params index, columns, in the BaseDistribution class diff --git a/skpro/utils/git_diff.py b/skpro/utils/git_diff.py new file mode 100644 index 000000000..a88a7e9ac --- /dev/null +++ b/skpro/utils/git_diff.py @@ -0,0 +1,80 @@ +"""Git related utilities to identify changed modules.""" + +__author__ = ["fkiraly"] +__all__ = [] + +import importlib.util +import inspect +import subprocess + + +def get_module_from_class(cls): + """Get full parent module string from class. + + Parameters + ---------- + cls : class + class to get module string from, e.g., NaiveForecaster + + Returns + ------- + str : module string, e.g., sktime.forecasting.naive + """ + module = inspect.getmodule(cls) + return module.__name__ if module else None + + +def get_path_from_module(module_str): + r"""Get local path string from module string. + + Parameters + ---------- + module_str : str + module string, e.g., sktime.forecasting.naive + + Returns + ------- + str : local path string, e.g., sktime\forecasting\naive.py + """ + try: + module_spec = importlib.util.find_spec(module_str) + if module_spec is None: + raise ImportError( + f"Error in get_path_from_module, module '{module_str}' not found." + ) + return module_spec.origin + except Exception as e: + raise ImportError(f"Error finding module '{module_str}'") from e + + +def is_module_changed(module_str): + """Check if a module has changed compared to the main branch. + + Parameters + ---------- + module_str : str + module string, e.g., sktime.forecasting.naive + """ + module_file_path = get_path_from_module(module_str) + cmd = f"git diff remotes/origin/main -- {module_file_path}" + try: + output = subprocess.check_output(cmd, shell=True, text=True) + return bool(output) + except subprocess.CalledProcessError: + return True + + +def is_class_changed(cls): + """Check if a class' parent module has changed compared to the main branch. + + Parameters + ---------- + cls : class + class to get module string from, e.g., NaiveForecaster + + Returns + ------- + bool : True if changed, False otherwise + """ + module_str = get_module_from_class(cls) + return is_module_changed(module_str)