テストコードのリファクタリング

.github/workflows/lint-test.yml

@@ -28,8 +28,11 @@ jobs:
           python -m pip install --upgrade pip "poetry<1.9"
           poetry install --only main,test --all-extras
       - name: Test
+        env:
+          # 以下の環境変数がないとテストに失敗するため、ダミーの値を設定する
+          ANNOFAB_PAT: "foo"
         run: |
-            poetry run pytest tests/test_local*.py
+            poetry run pytest -n auto -m "not access_webapi"
 
   lint:
     runs-on: ubuntu-latest

Makefile

@@ -12,6 +12,7 @@ format:
 	poetry run ruff check ${SOURCE_FILES} ${TEST_FILES} --fix-only --exit-zero
 
 lint:
+	poetry run ruff format ${SOURCE_FILES} ${TEST_FILES} --check
 	poetry run ruff check ${SOURCE_FILES} ${TEST_FILES} 
 	poetry run mypy ${SOURCE_FILES} ${TEST_FILES}
 	# テストコードはチェックを緩和するためpylintは実行しない

annofabapi/api.py

@@ -193,8 +193,16 @@ def _create_query_params_for_logger(params: dict[str, Any]) -> dict[str, Any]:
 def _should_retry_with_status(status_code: int) -> bool:
     """
     HTTP Status Codeからリトライすべきかどうかを返す。
+
+    Notes:
+        429(Too many requests)の場合も、`@my_backoff`ではリトライしません。
+        レスポンスヘッダーの`Retry-After`を参照するため、`@my_backoff`ではなく、直接リトライするコードを書いています。
+
+    Returns:
+        trueならばリトライする
     """
-    # 注意：429(Too many requests)の場合は、backoffモジュール外でリトライするため、このメソッドでは判定しない
+
+    # 501の場合は、未実装のためリトライしない
     if status_code == requests.codes.not_implemented:
         return False
     if 500 <= status_code < 600:  # noqa: SIM103
@@ -204,19 +212,14 @@ def _should_retry_with_status(status_code: int) -> bool:
 
 def my_backoff(function) -> Callable:  # noqa: ANN001
     """
-    HTTP Status Codeが429 or 5XXのときはリトライする. 最大5分間リトライする。
+    リトライした方が良い場合は、バックオフする
     """
 
     @wraps(function)
     def wrapped(*args, **kwargs):  # noqa: ANN202
-        def fatal_code(e):  # noqa: ANN001, ANN202
+        def should_give_up(e: Exception) -> bool:
             """
-            リトライするかどうか
-            status codeが5xxのとき、またはToo many Requests(429)のときはリトライする。429以外の4XXはリトライしない
-            https://requests.kennethreitz.org/en/master/user/quickstart/#errors-and-exceptions
-
-            Args:
-                e: exception
+            ギブアップ（リトライしない）かどうか
 
             Returns:
                 True: give up(リトライしない), False: リトライする
@@ -236,7 +239,7 @@ def fatal_code(e):  # noqa: ANN001, ANN202
             (requests.exceptions.HTTPError, requests.exceptions.ConnectionError, ConnectionError),
             jitter=backoff.full_jitter,
             max_time=300,
-            giveup=fatal_code,
+            giveup=should_give_up,
             # loggerの名前をbackoffからannofabapiに変更する
             logger=logger,
         )(function)(*args, **kwargs)

pytest.ini

@@ -4,6 +4,9 @@
 # Don't write `pytest-cov` Option
 addopts = --verbose --capture=no -rs --ignore=tests/test_sandbox.py
 
+markers =
+    access_webapi: WebAPIにアクセスするテスト
+
 [annofab]
 endpoint_url = https://annofab.com
 

tests/test_api.py

@@ -15,6 +15,7 @@
 from more_itertools import first_true
 
 import annofabapi
+from annofabapi.api import _create_request_body_for_logger, my_backoff
 from annofabapi.dataclass.annotation import AnnotationV2Output, SimpleAnnotation, SingleAnnotationV2
 from annofabapi.dataclass.annotation_specs import AnnotationSpecsV3
 from annofabapi.dataclass.comment import Comment
@@ -50,6 +51,63 @@
 test_wrapper = WrapperForTest(api)
 
 
+class TestMyBackoff:
+    @my_backoff
+    def access_api_with_retry(self, log: list[str], exception: Exception):
+        """何回実行されるかを確認する"""
+        if len(log) == 0:
+            log.append("foo")
+            raise exception
+
+    def test__connection_errorが発生したらリトライされる(self):
+        # ２回呼ばれることを確認する
+        log: list[str] = []
+        self.access_api_with_retry(log, requests.exceptions.ConnectionError)
+        assert len(log) == 1
+
+        log = []
+        self.access_api_with_retry(log, ConnectionError)
+        assert len(log) == 1
+
+    def test__http_errorのstatus_codeリトライされる(self):
+        # ２回呼ばれることを確認する
+
+        # リトライしない
+        response = requests.Response()
+        response.status_code = 400
+        log: list[str] = []
+        with pytest.raises(requests.exceptions.HTTPError):
+            self.access_api_with_retry(log, requests.exceptions.HTTPError(response=response))
+
+        response = requests.Response()
+        response.status_code = 501
+        log = []
+        with pytest.raises(requests.exceptions.HTTPError):
+            self.access_api_with_retry(log, requests.exceptions.HTTPError(response=response))
+
+        # リトライする
+        response = requests.Response()
+        response.status_code = 500
+        log = []
+        self.access_api_with_retry(log, requests.exceptions.HTTPError(response=response))
+        assert len(log) == 1
+
+
+class Test__create_request_body_for_logger:
+    def test_data_dict(self):
+        actual = _create_request_body_for_logger({"foo": "1", "password": "x", "new_password": "y", "old_password": "z"})
+        assert actual == {"foo": "1", "password": "***", "new_password": "***", "old_password": "***"}
+
+    def test_data_dict2(self):
+        actual = _create_request_body_for_logger({"foo": "1"})
+        assert actual == {"foo": "1"}
+
+    def test_data_list(self):
+        actual = _create_request_body_for_logger([{"foo": "1"}])
+        assert actual == [{"foo": "1"}]
+
+
+@pytest.mark.access_webapi
 class TestAnnotation:
     input_data_id: str
 
@@ -107,6 +165,7 @@ def test_wrapper_put_annotation_for_simple_annotation_json(self):
         )
 
 
+@pytest.mark.access_webapi
 class TestAnnotationSpecs:
     def test_get_annotation_specs(self):
         annotation_spec, _ = api.get_annotation_specs(project_id, query_params={"v": "3"})
@@ -136,6 +195,7 @@ def test_get_annotation_specs_relation(self):
         assert type(result) is annofabapi.wrapper.AnnotationSpecsRelation
 
 
+@pytest.mark.access_webapi
 class TestComment:
     task: dict[str, Any]
 
@@ -199,6 +259,7 @@ def teardown_class(cls):
         wrapper.change_task_status_to_break(project_id, task_id)
 
 
+@pytest.mark.access_webapi
 class TestInputData:
     input_data_id: str
 
@@ -232,6 +293,7 @@ def test_put_input_data_from_file_and_batch_update_inputs(self):
         assert type(api.batch_update_inputs(project_id, request_body=request_body)[0]) == list  # noqa: E721
 
 
+@pytest.mark.access_webapi
 class TestInstruction:
     def test_wrapper_get_latest_instruction(self):
         assert type(wrapper.get_latest_instruction(project_id)) == dict  # noqa: E721
@@ -261,6 +323,7 @@ def test_put_instruction(self):
         assert type(api.put_instruction(project_id, request_body=put_request_body)[0]) == dict  # noqa: E721
 
 
+@pytest.mark.access_webapi
 class TestJob:
     @pytest.mark.submitting_job
     def test_scenario(self):
@@ -292,6 +355,7 @@ def test_scenario(self):
         assert first_true(job_list, pred=lambda e: e["job_id"] == job_id) is None
 
 
+@pytest.mark.access_webapi
 class TestLogin:
     def test_login(self):
         # Exceptionをスローしないことの確認
@@ -300,6 +364,7 @@ def test_login(self):
         api.logout()
 
 
+@pytest.mark.access_webapi
 class TestMy:
     def test_get_my_account(self):
         my_account, _ = api.get_my_account()
@@ -322,6 +387,7 @@ def test_get_my_member_in_project(self):
         assert type(my_member_in_project) == dict  # noqa: E721
 
 
+@pytest.mark.access_webapi
 class TestOrganization:
     organization_name: str
 
@@ -342,6 +408,7 @@ def test_wrapper_get_all_projects_of_organization(self):
         assert len(wrapper.get_all_projects_of_organization(self.organization_name)) > 0
 
 
+@pytest.mark.access_webapi
 class TestOrganizationMember:
     organization_name: str
 
@@ -365,6 +432,7 @@ def test_update_organization_member_role(self):
         api.update_organization_member_role(self.organization_name, api.login_user_id, request_body=request_body)
 
 
+@pytest.mark.access_webapi
 class TestProject:
     def test_get_project(self):
         dict_project, _ = api.get_project(project_id)
@@ -395,6 +463,7 @@ def test_wrapper_download_project_comments_url(self):
         assert wrapper.download_project_comments_url(project_id, f"{out_dir}/comments.json").startswith("https://")
 
 
+@pytest.mark.access_webapi
 class TestProjectMember:
     def test_get_project_member(self):
         my_member = api.get_project_member(project_id, api.login_user_id)[0]
@@ -406,6 +475,7 @@ def test_wrapper_get_all_project_members(self):
         ProjectMember.schema().load(member_list, many=True)
 
 
+@pytest.mark.access_webapi
 class TestStatistics:
     def test_wrapper_get_account_daily_statistics(self):
         actual = wrapper.get_account_daily_statistics(project_id, from_date="2021-04-01", to_date="2021-06-30")
@@ -513,7 +583,8 @@ def test_get_statistics_available_dates(self):
         assert type(content) == list  # noqa: E721
 
 
-class Testsupplementary:
+@pytest.mark.access_webapi
+class TestSupplementary:
     input_data_id: str
 
     @classmethod
@@ -542,6 +613,7 @@ def test_supplementary(self):
         assert len([e for e in supplementary_data_list2 if e["supplementary_data_id"] == supplementary_data_id]) == 0
 
 
+@pytest.mark.access_webapi
 class TestTask:
     input_data_id: str
 
@@ -608,6 +680,7 @@ def test_patch_tasks_metadata(self):
         assert type(content) == dict  # noqa: E721
 
 
+@pytest.mark.access_webapi
 class TestWebhook:
     def test_scenario(self):
         """
@@ -643,6 +716,7 @@ def test_scenario(self):
         assert first_true(webhook_list, pred=lambda e: e["webhook_id"] == test_webhook_id) is None
 
 
+@pytest.mark.access_webapi
 class TestGetObjOrNone:
     """
     wrapper.get_xxx_or_none メソッドの確認
@@ -708,6 +782,7 @@ def test_get_supplementary_data_list_or_none(self):
         assert supplementary_data_list is None
 
 
+@pytest.mark.access_webapi
 class TestProtectedMethod:
     input_data_id: str
 
@@ -733,6 +808,7 @@ def test_request_get_with_cookie_failed(self):
             api._request_get_with_cookie(project_id, url)
 
 
+@pytest.mark.access_webapi
 class TestProperty:
     def test_account_id(self):
         account_id = api.account_id

tests/test_api2.py

@@ -5,9 +5,14 @@
 
 import configparser
 
+import pytest
+
 import annofabapi
 from tests.utils_for_test import WrapperForTest
 
+# webapiにアクセスするテストモジュール
+pytestmark = pytest.mark.access_webapi
+
 inifile = configparser.ConfigParser()
 inifile.read("./pytest.ini", "UTF-8")
 project_id = inifile["annofab"]["project_id"]

tests/test_pydantic_models.py

@@ -1,6 +1,8 @@
 import configparser
 import json
 
+import pytest
+
 import annofabapi
 from annofabapi.pydantic_models.annotation_specs_v3 import AnnotationSpecsV3
 from annofabapi.pydantic_models.input_data import InputData
@@ -11,6 +13,9 @@
 from annofabapi.pydantic_models.single_annotation import SingleAnnotation
 from annofabapi.pydantic_models.task import Task
 
+# webapiにアクセスするテストモジュール
+pytestmark = pytest.mark.access_webapi
+
 inifile = configparser.ConfigParser()
 inifile.read("./pytest.ini", "UTF-8")
 

tests/test_local_resource.py → tests/test_resource.py

@@ -24,6 +24,7 @@ def test_build_from_env_raise_CredentialsNotFoundError(self):
         with pytest.raises(annofabapi.exceptions.CredentialsNotFoundError):
             os.environ.pop("ANNOFAB_USER_ID", None)
             os.environ.pop("ANNOFAB_PASSWORD", None)
+            os.environ.pop("ANNOFAB_PAT", None)
             build_from_env()
 
     def test_build_from_env(self):

tests/test_sandbox.py

@@ -4,8 +4,13 @@
 
 import configparser
 
+import pytest
+
 import annofabapi
 
+# webapiにアクセスするテストモジュール
+pytestmark = pytest.mark.access_webapi
+
 inifile = configparser.ConfigParser()
 inifile.read("./pytest.ini", "UTF-8")