From b33c1f28f77f1f72d8d3e2f48bab6597b559a8b1 Mon Sep 17 00:00:00 2001 From: Lucain Date: Mon, 28 Nov 2022 16:55:25 +0100 Subject: [PATCH] [RFC] Contrib test suite + tests for timm and sentence_transformers (#1200) * First draft for a contrib test suite + test for timm contrib * run only Python 3.8 * remove commented code * Run contrib tests in separate environments * fix ci * fix ci again * and now ? * stupid me * this time ? * Refactor how to run contrib tests locally * add tests for sentence_transformers * amke style * Update contrib/README.md Co-authored-by: Omar Sanseviero * ADapt timm tests * Include feedback form osanseviero * script to check contrib list is accurate * Use [testing] requirements as contrib common dependencies * add check_contrib_list in github workflow * code qualiry Co-authored-by: Omar Sanseviero --- .github/workflows/contrib-tests.yml | 45 +++++++ .github/workflows/python-quality.yml | 1 + Makefile | 47 +++++++- contrib/README.md | 70 +++++++++++ contrib/__init__.py | 0 contrib/conftest.py | 57 +++++++++ contrib/sentence_transformers/__init__.py | 0 .../sentence_transformers/requirements.txt | 1 + .../test_sentence_transformers.py | 34 ++++++ contrib/timm/__init__.py | 0 contrib/timm/requirements.txt | 2 + contrib/timm/test_timm.py | 20 +++ contrib/utils.py | 59 +++++++++ utils/check_contrib_list.py | 114 ++++++++++++++++++ utils/check_static_imports.py | 12 +- 15 files changed, 453 insertions(+), 9 deletions(-) create mode 100644 .github/workflows/contrib-tests.yml create mode 100644 contrib/README.md create mode 100644 contrib/__init__.py create mode 100644 contrib/conftest.py create mode 100644 contrib/sentence_transformers/__init__.py create mode 100644 contrib/sentence_transformers/requirements.txt create mode 100644 contrib/sentence_transformers/test_sentence_transformers.py create mode 100644 contrib/timm/__init__.py create mode 100644 contrib/timm/requirements.txt create mode 100644 contrib/timm/test_timm.py create mode 100644 contrib/utils.py create mode 100644 utils/check_contrib_list.py diff --git a/.github/workflows/contrib-tests.yml b/.github/workflows/contrib-tests.yml new file mode 100644 index 0000000000..22d7762bc8 --- /dev/null +++ b/.github/workflows/contrib-tests.yml @@ -0,0 +1,45 @@ +name: Contrib tests + +on: + workflow_dispatch: + schedule: + - cron: '0 0 * * 6' # Run once a week, Saturday midnight + push: + branches: + - ci_contrib_* + +jobs: + build: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + contrib: [ + "sentence_transformers", + "timm", + ] + + steps: + - uses: actions/checkout@v2 + - name: Set up Python 3.8 + uses: actions/setup-python@v2 + with: + python-version: 3.8 + + # Install pip + - name: Install pip + run: pip install --upgrade pip + + # Install downstream library and its specific dependencies + - name: Install ${{ matrix.contrib }} + run: pip install -r contrib/${{ matrix.contrib }}/requirements.txt + + # Install huggingface_hub from source code + testing extras + - name: Install `huggingface_hub` + run: | + pip uninstall -y huggingface_hub + pip install .[testing] + + # Run tests + - name: Run tests + run: pytest contrib/${{ matrix.contrib }} diff --git a/.github/workflows/python-quality.yml b/.github/workflows/python-quality.yml index 7586153929..69a1b58729 100644 --- a/.github/workflows/python-quality.yml +++ b/.github/workflows/python-quality.yml @@ -30,6 +30,7 @@ jobs: - run: black --check tests src - run: isort --check-only tests src - run: flake8 tests src + - run: python utils/check_contrib_list.py - run: python utils/check_static_imports.py # Run type checking at least on huggingface_hub root file to check all modules diff --git a/Makefile b/Makefile index 5ba1b48f22..9da02e4346 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ -.PHONY: quality style test +.PHONY: contrib quality style test -check_dirs := tests src utils setup.py +check_dirs := contrib src tests utils setup.py quality: @@ -9,12 +9,53 @@ quality: isort --check-only $(check_dirs) flake8 $(check_dirs) mypy src + python utils/check_contrib_list.py python utils/check_static_imports.py style: black $(check_dirs) isort $(check_dirs) - python utils/check_static_imports.py --update-file + python utils/check_contrib_list.py --update + python utils/check_static_imports.py --update test: pytest ./tests/ + +# Taken from https://stackoverflow.com/a/12110773 +# Commands: +# make contrib_setup_timm : setup tests for timm +# make contrib_test_timm : run tests for timm +# make contrib_timm : setup and run tests for timm +# make contrib_clear_timm : delete timm virtual env +# +# make contrib_setup : setup ALL tests +# make contrib_test : run ALL tests +# make contrib : setup and run ALL tests +# make contrib_clear : delete all virtual envs +# Use -j4 flag to run jobs in parallel. +CONTRIB_LIBS := sentence_transformers timm +CONTRIB_JOBS := $(addprefix contrib_,${CONTRIB_LIBS}) +CONTRIB_CLEAR_JOBS := $(addprefix contrib_clear_,${CONTRIB_LIBS}) +CONTRIB_SETUP_JOBS := $(addprefix contrib_setup_,${CONTRIB_LIBS}) +CONTRIB_TEST_JOBS := $(addprefix contrib_test_,${CONTRIB_LIBS}) + +contrib_clear_%: + rm -rf contrib/$*/.venv + +contrib_setup_%: + python3 -m venv contrib/$*/.venv + ./contrib/$*/.venv/bin/pip install -r contrib/$*/requirements.txt + ./contrib/$*/.venv/bin/pip uninstall -y huggingface_hub + ./contrib/$*/.venv/bin/pip install -e .[testing] + +contrib_test_%: + ./contrib/$*/.venv/bin/python -m pytest contrib/$* + +contrib_%: + make contrib_setup_$* + make contrib_test_$* + +contrib: ${CONTRIB_JOBS}; +contrib_clear: ${CONTRIB_CLEAR_JOBS}; echo "Successful contrib tests." +contrib_setup: ${CONTRIB_SETUP_JOBS}; echo "Successful contrib setup." +contrib_test: ${CONTRIB_TEST_JOBS}; echo "Successful contrib tests." \ No newline at end of file diff --git a/contrib/README.md b/contrib/README.md new file mode 100644 index 0000000000..2b051bc341 --- /dev/null +++ b/contrib/README.md @@ -0,0 +1,70 @@ +# Contrib test suite + +The contrib folder contains simple end-to-end scripts to test integration of `huggingface_hub` in downstream libraries. The main goal is to proactively notice breaking changes and deprecation warnings. + +## Add tests for a new library + +To add another contrib lib, one must: +1. Create a subfolder with the lib name. Example: `./contrib/transformers` +2. Create a `requirements.txt` file specific to this lib. Example `./contrib/transformers/requirements.txt` +3. Implements tests for this lib. Example: `./contrib/transformers/test_push_to_hub.py` +4. Run `make style`. This will edit both `makefile` and `.github/workflows/contrib-tests.yml` to add the lib to list of libs to test. Make sure changes are accurate before committing. + +## Run contrib tests on CI + +Contrib tests can be [manually triggered in GitHub](https://github.com/huggingface/huggingface_hub/actions) with the `Contrib tests` workflow. + +Tests are not run in the default test suite (for each PR) as this would slow down development process. The goal is to notice breaking changes, not to avoid them. In particular, it is interesting to trigger it before a release to make sure it will not cause too much friction. + +## Run contrib tests locally + +Tests must be ran individually for each dependent library. Here is an example to run +`timm` tests. Tests are separated to avoid conflicts between version dependencies. + +### Run all contrib tests + +Before running tests, a virtual env must be setup for each contrib library. To do so, run: + +```sh +# Run setup in parallel to save time +make contrib_setup -j4 +``` + +Then tests can be run + +```sh +# Optional: -j4 to run in parallel. Output will be messy in that case. +make contrib_test -j4 +``` + +Optionally, it is possible to setup and run all tests in a single command. However this +take more time as you don't need to setup the venv each time you run tests. + +```sh +make contrib -j4 +``` + +Finally, it is possible to delete all virtual envs to get a fresh start for contrib tests. +After running this command, `contrib_setup` will have to re-download/re-install all dependencies. + +``` +make contrib_clear +``` + +### Run contrib tests for a single lib + +Instead of running tests for all contrib libraries, you can run a specific lib: + +```sh +# Setup timm tests +make contrib_setup_timm + +# Run timm tests +make contrib_test_timm + +# (or) Setup and run timm tests at once +make contrib_timm + +# Delete timm virtualenv if corrupted +make contrib_clear_timm +``` diff --git a/contrib/__init__.py b/contrib/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/contrib/conftest.py b/contrib/conftest.py new file mode 100644 index 0000000000..f438cba61c --- /dev/null +++ b/contrib/conftest.py @@ -0,0 +1,57 @@ +import time +import uuid +from typing import Generator + +import pytest + +from huggingface_hub import HfFolder, delete_repo + + +@pytest.fixture(scope="session") +def token() -> str: + # Not critical, only usable on the sandboxed CI instance. + return "hf_94wBhPGp6KrrTH3KDchhKpRxZwd6dmHWLL" + + +@pytest.fixture(scope="session") +def user() -> str: + return "__DUMMY_TRANSFORMERS_USER__" + + +@pytest.fixture(autouse=True, scope="session") +def login_as_dummy_user(token: str) -> Generator: + """Login with dummy user token on machine + + Once all tests are completed, set back previous token.""" + # Remove registered token + old_token = HfFolder().get_token() + HfFolder().save_token(token) + + yield # Run all tests + + # Set back token once all tests have passed + if old_token is not None: + HfFolder().save_token(old_token) + + +@pytest.fixture +def repo_name(request) -> None: + """ + Return a readable pseudo-unique repository name for tests. + + Example: "repo-2fe93f-16599646671840" + """ + prefix = request.module.__name__ # example: `test_timm` + id = uuid.uuid4().hex[:6] + ts = int(time.time() * 10e3) + return f"repo-{prefix}-{id}-{ts}" + + +@pytest.fixture +def cleanup_repo(user: str, repo_name: str) -> None: + """Delete the repo at the end of the tests. + + TODO: Adapt to handle `repo_type` as well + """ + yield # run test + delete_repo(repo_id=f"{user}/{repo_name}") diff --git a/contrib/sentence_transformers/__init__.py b/contrib/sentence_transformers/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/contrib/sentence_transformers/requirements.txt b/contrib/sentence_transformers/requirements.txt new file mode 100644 index 0000000000..128abfc4be --- /dev/null +++ b/contrib/sentence_transformers/requirements.txt @@ -0,0 +1 @@ +git+https://github.com/UKPLab/sentence-transformers.git#egg=sentence-transformers \ No newline at end of file diff --git a/contrib/sentence_transformers/test_sentence_transformers.py b/contrib/sentence_transformers/test_sentence_transformers.py new file mode 100644 index 0000000000..62d1593b5f --- /dev/null +++ b/contrib/sentence_transformers/test_sentence_transformers.py @@ -0,0 +1,34 @@ +import pytest + +from sentence_transformers import SentenceTransformer, util + +from ..utils import production_endpoint + + +@pytest.fixture(scope="module") +def multi_qa_model() -> SentenceTransformer: + with production_endpoint(): + return SentenceTransformer("multi-qa-MiniLM-L6-cos-v1") + + +def test_from_pretrained(multi_qa_model: SentenceTransformer) -> None: + # Example taken from https://www.sbert.net/docs/hugging_face.html#using-hugging-face-models. + query_embedding = multi_qa_model.encode("How big is London") + passage_embedding = multi_qa_model.encode( + [ + "London has 9,787,426 inhabitants at the 2011 census", + "London is known for its financial district", + ] + ) + print("Similarity:", util.dot_score(query_embedding, passage_embedding)) + + +@pytest.mark.xfail( + reason=( + "Production endpoint is hardcoded in sentence_transformers when pushing to Hub." + ) +) +def test_push_to_hub( + multi_qa_model: SentenceTransformer, repo_name: str, cleanup_repo: None +) -> None: + multi_qa_model.save_to_hub(repo_name) diff --git a/contrib/timm/__init__.py b/contrib/timm/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/contrib/timm/requirements.txt b/contrib/timm/requirements.txt new file mode 100644 index 0000000000..8455ddbef6 --- /dev/null +++ b/contrib/timm/requirements.txt @@ -0,0 +1,2 @@ +# Timm +git+https://github.com/rwightman/pytorch-image-models.git#egg=timm \ No newline at end of file diff --git a/contrib/timm/test_timm.py b/contrib/timm/test_timm.py new file mode 100644 index 0000000000..85e65d8dde --- /dev/null +++ b/contrib/timm/test_timm.py @@ -0,0 +1,20 @@ +import timm + +from ..utils import production_endpoint + + +MODEL_ID = "nateraw/timm-resnet50-beans" + + +@production_endpoint() +def test_load_from_hub() -> None: + # Test load only config + _ = timm.models.hub.load_model_config_from_hf(MODEL_ID) + + # Load entire model from Hub + _ = timm.create_model("hf_hub:" + MODEL_ID, pretrained=True) + + +def test_push_to_hub(repo_name: str, cleanup_repo: None) -> None: + model = timm.create_model("resnet18") + timm.models.hub.push_to_hf_hub(model, repo_name) diff --git a/contrib/utils.py b/contrib/utils.py new file mode 100644 index 0000000000..396c87e206 --- /dev/null +++ b/contrib/utils.py @@ -0,0 +1,59 @@ +import contextlib +from typing import Generator +from unittest.mock import patch + + +@contextlib.contextmanager +def production_endpoint() -> Generator: + """Patch huggingface_hub to connect to production server in a context manager. + + Ugly way to patch all constants at once. + TODO: refactor when https://github.com/huggingface/huggingface_hub/issues/1172 is fixed. + + Example: + ```py + def test_push_to_hub(): + # Pull from production Hub + with production_endpoint(): + model = ...from_pretrained("modelname") + + # Push to staging Hub + model.push_to_hub() + ``` + """ + PROD_ENDPOINT = "https://huggingface.co" + ENDPOINT_TARGETS = [ + "huggingface_hub.constants", + "huggingface_hub._commit_api", + "huggingface_hub.hf_api", + "huggingface_hub.lfs", + "huggingface_hub.commands.user", + "huggingface_hub.utils._git_credential", + ] + + PROD_URL_TEMPLATE = PROD_ENDPOINT + "/{repo_id}/resolve/{revision}/{filename}" + URL_TEMPLATE_TARGETS = [ + "huggingface_hub.constants", + "huggingface_hub.file_download", + ] + + from huggingface_hub.hf_api import api + + patchers = ( + [patch(target + ".ENDPOINT", PROD_ENDPOINT) for target in ENDPOINT_TARGETS] + + [ + patch(target + ".HUGGINGFACE_CO_URL_TEMPLATE", PROD_URL_TEMPLATE) + for target in URL_TEMPLATE_TARGETS + ] + + [patch.object(api, "endpoint", PROD_URL_TEMPLATE)] + ) + + # Start all patches + for patcher in patchers: + patcher.start() + + yield + + # Stop all patches + for patcher in patchers: + patcher.stop() diff --git a/utils/check_contrib_list.py b/utils/check_contrib_list.py new file mode 100644 index 0000000000..ea3d0a73f1 --- /dev/null +++ b/utils/check_contrib_list.py @@ -0,0 +1,114 @@ +# coding=utf-8 +# Copyright 2022-present, the HuggingFace Inc. team. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Contains a tool to list contrib test suites automatically.""" +import argparse +import re +from pathlib import Path +from typing import NoReturn + + +ROOT_DIR = Path(__file__).parent.parent +CONTRIB_PATH = ROOT_DIR / "contrib" +MAKEFILE_PATH = ROOT_DIR / "Makefile" +WORKFLOW_PATH = ROOT_DIR / ".github" / "workflows" / "contrib-tests.yml" + +MAKEFILE_REGEX = re.compile(r"^CONTRIB_LIBS := .*$", flags=re.MULTILINE) +WORKFLOW_REGEX = re.compile( + r""" + # First: match "contrib: [" + (?P^\s{8}contrib:\s\[\n) + # Match list of libs + (\s{10}\".*\",\n)* + # Finally: match trailing "]" + (?P^\s{8}\]) + """, + flags=re.MULTILINE | re.VERBOSE, +) + + +def check_contrib_list(update: bool) -> NoReturn: + """List `contrib` test suites. + + Make sure `Makefile` and `.github/workflows/contrib-tests.yml` are consistent with + the list.""" + # List contrib test suites + contrib_list = sorted( + path.name + for path in CONTRIB_PATH.glob("*") + if path.is_dir() and not path.name.startswith("_") + ) + + # Check Makefile is consistent with list + makefile_content = MAKEFILE_PATH.read_text() + makefile_expected_content = MAKEFILE_REGEX.sub( + f"CONTRIB_LIBS := {' '.join(contrib_list)}", makefile_content + ) + + # Check workflow is consistent with list + workflow_content = WORKFLOW_PATH.read_text() + _substitute = "\n".join(f'{" "*10}"{lib}",' for lib in contrib_list) + workflow_content_expected = WORKFLOW_REGEX.sub( + rf"\g{_substitute}\n\g", workflow_content + ) + + # + failed = False + if makefile_content != makefile_expected_content: + if update: + print( + "✅ Contrib libs have been updated in `Makefile`." + "\n Please make sure the changes are accurate and commit them." + ) + MAKEFILE_PATH.write_text(makefile_expected_content) + else: + print( + "❌ Expected content mismatch in `Makefile`.\n It is most likely that" + " you added a contrib test and did not update the Makefile.\n Please" + " run `make style` or `python utils/check_contrib_list.py --update`." + ) + failed = True + + if workflow_content != workflow_content_expected: + if update: + print( + f"✅ Contrib libs have been updated in `{WORKFLOW_PATH}`." + "\n Please make sure the changes are accurate and commit them." + ) + WORKFLOW_PATH.write_text(workflow_content_expected) + else: + print( + f"❌ Expected content mismatch in `{WORKFLOW_PATH}`.\n It is most" + " likely that you added a contrib test and did not update the github" + " workflow file.\n Please run `make style` or `python" + " utils/check_contrib_list.py --update`." + ) + failed = True + + if failed: + exit(1) + print("✅ All good! (contrib list)") + exit(0) + + +if __name__ == "__main__": + parser = argparse.ArgumentParser() + parser.add_argument( + "--update", + action="store_true", + help="Whether to fix Makefile and github workflow if a new lib is detected.", + ) + args = parser.parse_args() + + check_contrib_list(update=args.update) diff --git a/utils/check_static_imports.py b/utils/check_static_imports.py index 5107ad47d5..2d9a10e3bf 100644 --- a/utils/check_static_imports.py +++ b/utils/check_static_imports.py @@ -29,7 +29,7 @@ SUBMOD_ATTRS_PATTERN = re.compile("_SUBMOD_ATTRS = {[^}]+}") # match the all dict -def check_static_imports(update_file: bool) -> NoReturn: +def check_static_imports(update: bool) -> NoReturn: """Check all imports are made twice (1 in lazy-loading and 1 in static checks). For more explanations, see `./src/huggingface_hub/__init__.py`. @@ -85,7 +85,7 @@ def check_static_imports(update_file: bool) -> NoReturn: # If expected `__init__.py` content is different, test fails. If '--update-init-file' # is used, `__init__.py` file is updated before the test fails. if init_content != expected_init_content: - if update_file: + if update: with INIT_FILE_PATH.open("w") as f: f.write(expected_init_content) @@ -100,18 +100,18 @@ def check_static_imports(update_file: bool) -> NoReturn: " `./src/huggingface_hub/__init__.py`.\n It is most likely that you" " added a module/function to `_SUBMOD_ATTRS` and did not update the" " 'static import'-part.\n Please run `make style` or `python" - " utils/check_static_imports.py --update-file`." + " utils/check_static_imports.py --update`." ) exit(1) - print("✅ All good!") + print("✅ All good! (static imports)") exit(0) if __name__ == "__main__": parser = argparse.ArgumentParser() parser.add_argument( - "--update-file", + "--update", action="store_true", help=( "Whether to fix `./src/huggingface_hub/__init__.py` if a change is" @@ -120,4 +120,4 @@ def check_static_imports(update_file: bool) -> NoReturn: ) args = parser.parse_args() - check_static_imports(update_file=args.update_file) + check_static_imports(update=args.update)