From 9332cb1f637d295f6c90da25e3f53c205185b4c7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 20:00:19 +0000 Subject: [PATCH 1/6] =?UTF-8?q?tests:=20=D7=94=D7=A9=D7=94=D7=99=D7=95?= =?UTF-8?q?=D7=AA=20=D7=A9=D7=9C=20=D7=A7=D7=95=D7=93=20=D7=94=D7=99=D7=99?= =?UTF-8?q?=D7=A6=D7=95=D7=A8=20=D7=9E=D7=AA=D7=90=D7=A4=D7=A1=D7=95=D7=AA?= =?UTF-8?q?=20=D7=A4=D7=A2=D7=9D=20=D7=90=D7=97=D7=AA=20=D7=91-tests/conft?= =?UTF-8?q?est.py,=20=D7=95=D7=9C=D7=90=20=D7=91=D7=9B=D7=9C=20=D7=A7?= =?UTF-8?q?=D7=95=D7=91=D7=A5=20=D7=98=D7=A1=D7=98=D7=99=D7=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - tests/conftest.py: אפס ל-GITHUB_API_BASE_DELAY ול-GITHUB_BACKOFF_DELAY. אליו עבר גם בלוק ה-env של ci.yml (HTTP_RESILIENCE_*, REQUESTS_RETRIES, REQUESTS_RETRY_BACKOFF, AIOHTTP_TIMEOUT_TOTAL), באותם ערכים, כך שהם חלים גם על deploy.yml ועל הרצה מקומית. הבלוק יושב לפני ייבוא ה-stubs, כי resilience.py קורא את הערכים שלו בזמן ייבוא. - טסטי הורדת ה-ZIP המתינו באמת ל-apply_rate_limit_delay: כ-98 שנ' בכל מסלול Unit Tests, לפי דוחות unit-durations של ci.yml מ-4.10. מקומית הקובץ יורד מ-92 שנ' ל-0.27. - נמחקו שלושת ה-_no_delay הידניים, בשני קבצים. - test_apply_rate_limit_delay_respects_backoff קובע בעצמו את ההשהיה הבסיסית. היא הייתה ארוכה מה-backoff שהוא קובע, והטסט עבר גם כשה-backoff בוטל (מוטציה). - טסט חדש: שתי קריאות צמודות, והשנייה ממתינה את ההשהיה הבסיסית. אחרי האיפוס זה הטסט שעובר דרך ההמתנה האמיתית. - test_backup_id_is_unique_within_the_same_second בודק עכשיו באמת שתי אריזות באותה שנייה. עם ההשהיה הוא עבר גם בלי הסיומת האקראית של המזהה (מוטציה). - test_http_sync_adapter_retries.py: ה-docstring מפנה ל-conftest ולא ל-CI. - docs: סעיף "השהיות של קוד הייצור בטסטים" ב-testing.rst, ורשומה ב-whats-new. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3jHDFecUvpZMMo7czCNQ8 --- .github/workflows/ci.yml | 11 ++----- docs/testing.rst | 9 ++++++ docs/whats-new.rst | 1 + tests/conftest.py | 21 +++++++++++++ tests/test_github_menu_backoff_delay.py | 31 ++++++++++++++++++- ...hub_menu_show_repos_loading_edit_raises.py | 5 --- .../test_github_rate_limit_more_resources.py | 9 ------ tests/test_http_sync_adapter_retries.py | 2 +- 8 files changed, 65 insertions(+), 24 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fa1d561ac..c82c0053e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -283,14 +283,9 @@ jobs: GITHUB_TOKEN: ${{ secrets.TEST_GITHUB_TOKEN }} PASTEBIN_API_KEY: ${{ secrets.TEST_PASTEBIN_KEY }} FEATURE_MY_COLLECTIONS: '1' - # Minimize retry/backoff delays during tests to avoid long waits - HTTP_RESILIENCE_MAX_ATTEMPTS: '1' - HTTP_RESILIENCE_BACKOFF_BASE: '0' - HTTP_RESILIENCE_BACKOFF_MAX: '0' - HTTP_RESILIENCE_JITTER: '0' - REQUESTS_RETRIES: '0' - REQUESTS_RETRY_BACKOFF: '0' - AIOHTTP_TIMEOUT_TOTAL: '6' + # ערכי הבדיקה של ההמתנות שקוד הייצור עושה בכוונה — rate limit, retry, backoff — + # נקבעים ב-tests/conftest.py ולא כאן, כדי שיחולו על כל הרצה של הטסטים: גם + # deploy.yml וגם הרצה מקומית. ראו docs/testing.rst. # ``-m`` בוחר את הטסטים של המסלול (``matrix.marker``). מסלול שלא נבחר בו אף # טסט נכשל ולא עובר בשקט: pytest יוצא בקוד 5 כשלא נאסף אף טסט, גם כשכולם # סוננו ב-``-m`` (נמדד ב-pytest 8.4.2, עם ``-n`` ובלעדיו). כך שם סימון ששונה diff --git a/docs/testing.rst b/docs/testing.rst index c06eb3e48..dcd3de64e 100644 --- a/docs/testing.rst +++ b/docs/testing.rst @@ -127,6 +127,15 @@ Mocking HTTP ב‑github_menu_handler monkeypatch.setattr(gh, "_http_sync_request", fake_req) assert gh.http_request("POST", "https://example.com", data=b"x") is sentinel +השהיות של קוד הייצור בטסטים +--------------------------- + +**קוד הייצור ממתין בכוונה, והטסטים לא ממתינים איתו.** ה-rate limit בין קריאות ל-GitHub (``apply_rate_limit_delay`` ב-``github_menu_handler.py``), ה-retry עם backoff (``resilience.py``, ``http_sync.py``) ותקרת ההמתנה לתשובה ב-``http_async.py`` ממתינים באמת, וטסט שעובר בהם ממתין ועובר בלי שאיש יראה. לכן ``tests/conftest.py`` קובע להם ערכי בדיקה פעם אחת, בבלוק משתני הסביבה שבראש הקובץ. הערכים חלים על כל הרצה: ``ci.yml``, ``deploy.yml`` והרצה מקומית, ולכן הם לא נקבעים ב-env של workflow. + +- **טסט שהנושא שלו הוא ההשהיה עצמה קובע אותה בעצמו** ב-``monkeypatch.setenv``, ולא נשען על ברירת המחדל של הייצור או של ``tests/conftest.py``. כך עושים ``tests/test_github_menu_backoff_delay.py`` ו-``tests/test_http_sync_adapter_retries.py``. +- **השהיה חדשה בקוד הייצור מקבלת ערך בדיקה באותו בלוק, באותו PR.** לא מנטרלים אותה בקובץ הטסטים שבמקרה שם לב אליה, כי הקובץ הבא לא יידע על כך. +- **המתנה אמיתית גם מסתירה באגים, ולא רק עולה זמן.** טסט שבודק התנהגות "באותה שנייה" לא מגיע אליה כשכל קריאה ממתינה. ``test_backup_id_is_unique_within_the_same_second`` עבר גם כשההגנה שהוא בודק הוסרה, כל עוד ההשהיה רצה. + רישום Blueprint בסביבת טסטים ------------------------------ diff --git a/docs/whats-new.rst b/docs/whats-new.rst index dc1a85478..39280500a 100644 --- a/docs/whats-new.rst +++ b/docs/whats-new.rst @@ -4,6 +4,7 @@ What's New 2026-10-04 ---------- +- **tests: השהיות של קוד הייצור מתאפסות פעם אחת לכל הטסטים, ב-``tests/conftest.py``.** ``apply_rate_limit_delay`` ממתינה בין קריאות ל-GitHub, וטסטי הורדת ה-ZIP עברו בה באמת: כ-98 שניות של המתנה בכל מסלול ``Unit Tests``, לפי דוחות ``unit-durations-*`` של ריצת ``ci.yml`` מ-4.10. שני קבצים אחרים כבר נטרלו אותה, כל אחד לעצמו, והקבצים האלה לא. עכשיו ``tests/conftest.py`` מאפס את ``GITHUB_API_BASE_DELAY`` ואת ``GITHUB_BACKOFF_DELAY``, והנטרולים הידניים נמחקו. אליו עברו גם ערכי ה-retry וה-backoff שהיו ב-env של ``ci.yml`` (``HTTP_RESILIENCE_*``, ``REQUESTS_RETRIES``, ``REQUESTS_RETRY_BACKOFF``, ``AIOHTTP_TIMEOUT_TOTAL``), כך שהם חלים עכשיו גם על ``deploy.yml`` ועל הרצה מקומית. **ההמתנה גם הסתירה שני טסטים, וזה נמדד במוטציות:** ``test_backup_id_is_unique_within_the_same_second`` לא הגיע אף פעם לשתי אריזות באותה שנייה, ועבר גם בלי הסיומת האקראית של המזהה. ו-``test_apply_rate_limit_delay_respects_backoff`` עבר גם כשה-backoff לא הופעל, כי ההשהיה הבסיסית הייתה ארוכה ממנו. עכשיו הראשון בודק את מה שהוא מבטיח, השני קובע את ההשהיה הבסיסית בעצמו, וטסט חדש עובר דרך ההמתנה הבסיסית האמיתית, בערך קטן שהוא קובע. ראו :doc:`testing`. - **ci: טסטי ה-Markdown הכבדים רצים במסלול נפרד, במקביל לשאר טסטי היחידה.** בחודש שעבר זמן הטסטים ב-CI עלה מכשלוש דקות וחצי לכשבע וחצי (שלב ה-pytest בג'וב של 3.12, לפי דוחות ``unit-durations-*`` של ריצות ``ci.yml`` מ-9.9 ומ-4.10). הטסטים שהיו קיימים לא הואטו בסך הכול; את התוספת הביאו טסטים חדשים, ואת החלק הגדול שלה מעט טסטים כבדים. ביניהם טסטי פרסר ה-Markdown שעיקר העבודה שלהם הוא החישוב עצמו: השוואה לאורקל cmark-gfm, קלט ענק בתקרות של הפרסר, והטסטים של סקריפטי המדידה. הם מסומנים עכשיו ``md_heavy`` (הרישום ב-``pytest.ini``), והג'וב ``unit-tests`` ב-``.github/workflows/ci.yml`` רץ בשני מסלולים לכל גרסת פייתון: ``Unit Tests`` מריץ את כל השאר, ו-``Unit Tests md-heavy`` מריץ רק אותם, במקביל ובלי MongoDB ו-Redis שהוא אינו צריך. כל מסלול מדווח סטטוס חובה משלו. **את שני הסטטוסים החדשים צריך לסמן כבדיקות חובה בהגדרות של GitHub**, ועד שזה נעשה כשל בהם מופיע ב-PR אך אינו חוסם מיזוג. ושמות הסטטוסים, שכתובים ביד בכמה קבצים, נבדקים עכשיו מול המטריצה ב-``tests/test_required_checks_are_listed.py`` — והוא נכשל גם כשעותק של הרשימה מופיע בקובץ שהוא לא מכיר. :doc:`testing` ו-:doc:`branch-protection-and-pr-rules` מפנים עכשיו לרשימה שב-:doc:`ci-cd` במקום להחזיק עותק משלהם. - **fix (שירות ה-MCP): ריפו שהוסר בממשק נמחק גם מהדיסק של שירות ה-MCP.** הסרת ריפו בוובאפ מחקה את המראה שבדיסק של הוובאפ ואת הרשומות ב-``repo_metadata``, אבל לדיסק של שירות ה-MCP אף אחד לא נגע: המראה שם נשארה, וכלי הקריאה המשיכו להגיש אותה כאילו כלום לא קרה (כך קרה ל-``codekeeper-plugin``). עכשיו בסוף כל מעבר של ה-autosync, כל מראה בדיסק של השירות שאין לה רשומה ב-``repo_metadata`` נמחקת, דרך ``delete_mirror``. שום דבר לא נמחק כשהשאילתה נכשלה, או כשאף שם ב-``repo_metadata`` אינו שם של מראה שבדיסק (כולל קריאה ריקה). כשיש בדיסק מראות שנחסכו כך, נרשמת אזהרה. וגם לא נמחקים תיקייה שאינה מראה שהשירות יוצר (שם לא תקין, קישור סמלי) או ריפו שנמצא באמצע clone או fetch. מחיקה שנכשלה, או תיקיית מראות שאי אפשר לקרוא, נספרות ב-``errors``, והמראה נשארת עד המעבר הבא. הקריאה של ``repo_metadata`` נעשית מה-primary במפורש. בשורת הלוג ``repo autosync pass:`` נוסף המונה ``pruned``. ובאותה מנייה של המראות, גם ניקוי ה-credentials בעלייה מדווח עכשיו על תיקיית מראות שאי אפשר לקרוא ככשל, ולא כ-``checked=0 failed=0``. ראו :ref:`mcp-repo-autosync`. - **fix (מראות): טוקן ה-GitHub לא נשמר יותר בקובץ ה-config של המראות, ולא עובר בשורת הפקודה (#3480); וזיהוי הענף הראשי בייבוא ריפו (#3479).** עד עכשיו ``init_mirror`` שכפל מ-URL שהטוקן בתוכו, ו-git שמר אותו בטקסט גלוי ב-``remote.origin.url`` — על הדיסקים של הוובאפ ושל שירות ה-MCP, ובצילומים היומיים של Render; הוא גם הופיע בשורת הפקודה של ``git-remote-http`` בכל clone ו-fetch. עכשיו ה-URL השמור נקי, והטוקן עובר ככותרת דרך משתני סביבה של git, ורק לריפו שדורש הזדהות — ריפו ציבורי לא מקבל טוקן. מראות קיימות מנוקות לפני כל fetch ובעליית כל שירות, עם שורת לוג אחת שסופרת מה נמצא ומה נוקה, ואומרת לכל מראה מאיפה הטוקן שלה (``map``/``global``). fetch על מראה שלא אומת שהיא נקייה לא רץ (``mirror_url_not_clean``). ובנפרד: ``initial_import`` ניסה לזהות את הענף הראשי בפקודות ש-``_run_git_command`` דוחה, ולכן תמיד נפל ל-``main`` — וריפו שהענף הראשי שלו ``master`` לא יובא. עכשיו הזיהוי הוא מ-HEAD של המראה, וכשאין ענף מאומת מוחזר ``default_branch_undetected`` במקום ניחוש. ראו :ref:`mcp-mirror-credentials`. diff --git a/tests/conftest.py b/tests/conftest.py index c0d1b4956..633b4037a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -40,6 +40,27 @@ os.environ.setdefault('REDIS_CONNECT_TIMEOUT', '0.25') os.environ.setdefault('REDIS_SOCKET_TIMEOUT', '0.25') +# אותו עיקרון להמתנות שקוד הייצור עושה בכוונה: rate limit בין קריאות ל-GitHub +# (``apply_rate_limit_delay`` ב-``github_menu_handler.py``), retry עם backoff +# (``resilience.py``, ``http_sync.py``), ותקרת ההמתנה לתשובה ב-``http_async.py``. טסט שעובר +# בהן ממתין באמת ועובר, ואיש לא רואה — עד שמישהו קורא דוח זמנים. הערכים נקבעים כאן, ולא ב-env של +# workflow, כדי שיחולו על כל הרצה: ``ci.yml``, ``deploy.yml`` והרצה מקומית. הם לפני ייבוא +# ה-stubs, כי ``resilience.py`` קורא את שלו כבר בזמן הייבוא (``DEFAULT_RETRY_POLICY``). +# +# המתנה אמיתית גם מסתירה באגים, ולא רק עולה זמן: טסט שבודק התנהגות "באותה שנייה" לא מגיע +# אליה כשכל קריאה ממתינה. טסט שהנושא שלו הוא ההשהיה עצמה קובע אותה בעצמו +# (``monkeypatch.setenv``), כמו ב-``tests/test_github_menu_backoff_delay.py``. והשהיה חדשה +# בקוד הייצור מקבלת כאן ערך בדיקה באותו PR, ולא נטרול בקובץ הטסטים שבמקרה שם לב אליה. +os.environ.setdefault('GITHUB_API_BASE_DELAY', '0') +os.environ.setdefault('GITHUB_BACKOFF_DELAY', '0') +os.environ.setdefault('HTTP_RESILIENCE_MAX_ATTEMPTS', '1') +os.environ.setdefault('HTTP_RESILIENCE_BACKOFF_BASE', '0') +os.environ.setdefault('HTTP_RESILIENCE_BACKOFF_MAX', '0') +os.environ.setdefault('HTTP_RESILIENCE_JITTER', '0') +os.environ.setdefault('REQUESTS_RETRIES', '0') +os.environ.setdefault('REQUESTS_RETRY_BACKOFF', '0') +os.environ.setdefault('AIOHTTP_TIMEOUT_TOTAL', '6') + # Import stubs so any import of `telegram` succeeds in tests try: import tests._telegram_stubs # noqa: F401 diff --git a/tests/test_github_menu_backoff_delay.py b/tests/test_github_menu_backoff_delay.py index 897e89529..ce01a471c 100644 --- a/tests/test_github_menu_backoff_delay.py +++ b/tests/test_github_menu_backoff_delay.py @@ -25,11 +25,40 @@ def get(self): import github_menu_handler as mod monkeypatch.setattr(mod, "github_backoff_state", _State(), raising=False) + # ההשהיה הבסיסית מאופסת כאן במפורש, כדי שכל ההמתנה תבוא מה-backoff. כשהיא רצה + # בברירת המחדל של הייצור, היא ארוכה מה-backoff שנקבע כאן, והטסט עבר גם כשה-backoff + # לא הופעל בכלל (נמדד במוטציה שמבטלת אותו). + monkeypatch.setenv("GITHUB_API_BASE_DELAY", "0") # Reduce backoff delay to keep test fast, while still asserting >=1s monkeypatch.setenv("GITHUB_BACKOFF_DELAY", "1.0") start = time.time() await h.apply_rate_limit_delay(user_id) elapsed = time.time() - start - # Under backoff, delay should be ~5s minus already elapsed since last call; allow slack + # Under backoff, the wait is GITHUB_BACKOFF_DELAY minus the time since the seeded call assert elapsed >= 1.0 # at least some wait was applied + + +@pytest.mark.asyncio +async def test_apply_rate_limit_delay_waits_the_base_delay_between_two_calls(monkeypatch): + """שתי קריאות צמודות של אותו משתמש: הראשונה עוברת מיד, והשנייה ממתינה את ההשהיה הבסיסית. + + ``tests/conftest.py`` מאפס את ``GITHUB_API_BASE_DELAY`` לכל הטסטים, ולכן זה הטסט שעובר + דרך ההמתנה האמיתית. הוא קובע ערך קטן משלו, ומנטרל את מצב ה-backoff הגלובלי, כדי שההמתנה + תבוא רק מההשהיה הבסיסית. + """ + import github_menu_handler as mod + + delay = 0.3 + monkeypatch.setattr(mod, "github_backoff_state", None, raising=False) + monkeypatch.setenv("GITHUB_API_BASE_DELAY", str(delay)) + h = GitHubMenuHandler() + + start = time.monotonic() + await h.apply_rate_limit_delay(7) + first = time.monotonic() - start + await h.apply_rate_limit_delay(7) + second = time.monotonic() - start - first + + assert first < delay / 2, f"הקריאה הראשונה לא אמורה להמתין, והמתינה {first:.3f} שנ'" + assert second >= delay * 0.8, f"הקריאה השנייה הייתה אמורה להמתין כ-{delay} שנ', והמתינה {second:.3f}" diff --git a/tests/test_github_menu_show_repos_loading_edit_raises.py b/tests/test_github_menu_show_repos_loading_edit_raises.py index ef748911f..eefdf06ad 100644 --- a/tests/test_github_menu_show_repos_loading_edit_raises.py +++ b/tests/test_github_menu_show_repos_loading_edit_raises.py @@ -9,11 +9,6 @@ async def test_show_repos_loading_edit_raises_then_final_succeeds(monkeypatch): handler = gh.GitHubMenuHandler() monkeypatch.setattr(handler, "get_user_token", lambda uid: "token") - # avoid real delay - async def _no_delay(user_id: int): - return None - monkeypatch.setattr(handler, "apply_rate_limit_delay", _no_delay) - class _RateCore: remaining = 5000 limit = 5000 diff --git a/tests/test_github_rate_limit_more_resources.py b/tests/test_github_rate_limit_more_resources.py index 180e1290f..aeff06f7c 100644 --- a/tests/test_github_rate_limit_more_resources.py +++ b/tests/test_github_rate_limit_more_resources.py @@ -7,11 +7,6 @@ async def test_show_repos_resources_high_rate_loads_and_lists(monkeypatch): gh = __import__('github_menu_handler') handler = gh.GitHubMenuHandler() - # speed up - async def _no_delay(uid: int): - return None - monkeypatch.setattr(handler, "apply_rate_limit_delay", _no_delay) - # ensure token exists monkeypatch.setattr(handler, "get_user_token", lambda uid: "t") @@ -68,10 +63,6 @@ async def test_show_repos_unknown_rate_structure_still_proceeds(monkeypatch): gh = __import__('github_menu_handler') handler = gh.GitHubMenuHandler() - # speed up - async def _no_delay(uid: int): - return None - monkeypatch.setattr(handler, "apply_rate_limit_delay", _no_delay) monkeypatch.setattr(handler, "get_user_token", lambda uid: "t") class _User: diff --git a/tests/test_http_sync_adapter_retries.py b/tests/test_http_sync_adapter_retries.py index b585081a9..45dc9bd0d 100644 --- a/tests/test_http_sync_adapter_retries.py +++ b/tests/test_http_sync_adapter_retries.py @@ -55,7 +55,7 @@ def failing_server(): def _fresh_sessions(monkeypatch): """Sessions טריים, ושכבת adapter פעילה שאפשר למדוד. - ה-CI מגדיר ``REQUESTS_RETRIES=0`` כדי שהטסטים ירוצו מהר. עם אפס, שני + ``tests/conftest.py`` מגדיר ``REQUESTS_RETRIES=0`` כדי שהטסטים ירוצו מהר. עם אפס, שני המסלולים — עם הדגל ובלעדיו — מייצרים בדיוק אותו מספר בקשות, והטסטים כאן היו עוברים בלי לבדוק דבר. לכן הם **קובעים בעצמם** את הערך שהם מודדים, במקום לקוות שהסביבה תספק אותו. From bb34d172e56cca20bf70a5efaca86ab19a6218ef Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 20:01:35 +0000 Subject: [PATCH 2/6] =?UTF-8?q?ci:=20coverage=20=D7=A0=D7=9E=D7=93=D7=93?= =?UTF-8?q?=20=D7=A8=D7=A7=20=D7=91=D7=9E=D7=A1=D7=9C=D7=95=D7=9C=D7=99=20?= =?UTF-8?q?3.11=20=D7=A9=D7=9C=20unit-tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ci.yml: רשומת include במטריצה מוסיפה coverage: true לשני המסלולים של 3.11. לפי הכלל של GitHub, רשומה כזו מתווספת לכל צירוף שהיא לא דורסת בו ערך מקורי של המטריצה. דגלי --cov עוברים ל-pytest רק דרך ${{ matrix.coverage && '...' || '' }}, וההעלאה ל-Codecov רצה רק כש-matrix.coverage דלוק. שמות הסטטוסים לא השתנו. - למה: coverage הוסיף לטסטים של 3.12 כ-30% מהזמן שלהם. זו הערכה ממדידה מקומית עם coverage ובלעדיו, על זמני ה-CI מ-4.10. באותה ריצה התוצאות של שתי הגרסאות היו זהות, טסט מול טסט. נבחרה 3.11 כי עליה רץ הפרודקשן. - tests/test_required_checks_are_listed.py: _matrix_combinations פורש include לפי הכלל של GitHub. הוא נכשל על רשומה שהייתה יוצרת צירוף חדש, ועל exclude. - טסט חדש: לכל suite יש גרסה שמודדת coverage, הדגלים נמצאים רק בביטוי של matrix.coverage, וההעלאה תלויה בו. Codecov אינו בדיקת חובה, ולכן בלי הטסט כיסוי שנעלם לא היה מפיל דבר. - docs: ci-cd.rst, testing.rst, whats-new. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3jHDFecUvpZMMo7czCNQ8 --- .github/workflows/ci.yml | 18 ++++- docs/ci-cd.rst | 2 +- docs/testing.rst | 1 + docs/whats-new.rst | 1 + tests/test_required_checks_are_listed.py | 83 +++++++++++++++++++----- 5 files changed, 84 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c82c0053e..c851d088e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -188,6 +188,13 @@ jobs: - suite: md-heavy label: 'Unit Tests md-heavy' marker: 'md_heavy' + # coverage נמדד בגרסה אחת בלבד, כדי לא לשלם פעמיים על ההאטה שהוא מוסיף לטסטים. + # 3.11, כי זו הגרסה שהפרודקשן רץ עליה (Dockerfile). הרשומה מוסיפה את + # ``coverage: true`` לכל המסלולים של הגרסה הזו: רשומת include מתווספת לכל צירוף + # שהיא לא דורסת בו ערך מקורי של המטריצה + # (``jobs..strategy.matrix.include`` בתיעוד של GitHub Actions). + - python-version: '3.11' + coverage: true services: # המסלול ``md-heavy`` אינו צריך מסד: ``image`` ריק אומר ל-runner לא להרים את # השירות (``jobs..services..image`` בתיעוד של GitHub @@ -292,13 +299,20 @@ jobs: # כאן ולא בטסטים, או בכל הטסטים ולא כאן, מפיל את המסלול ``md-heavy``. טסט בודד # שהסימון שלו חסר או שגוי פשוט רץ במסלול הרגיל. את קוד היציאה, ואת זה ששני # הביטויים משלימים זה את זה, מקבע ``tests/test_required_checks_are_listed.py``. + # + # דגלי ה-coverage עוברים רק במסלולים ש-``matrix.coverage`` דלוק בהם. בשאר המסלולים + # המאפיין לא מוגדר, ומאפיין שלא קיים מחזיר מחרוזת ריקה (contexts בתיעוד של GitHub + # Actions); ``&&`` ו-``||`` מחזירים את הערך עצמו ולא true/false (``And.cs`` ו-``Or.cs`` + # ב-actions/runner), ולכן שם הביטוי ריק. אותו טסט בודק שאין ``--cov`` מחוץ לביטוי. run: | pytest -n auto --dist=loadscope -v -o addopts="" -m "${{ matrix.marker }}" \ - --cov=. --cov-report=xml --cov-report=term \ + ${{ matrix.coverage && '--cov=. --cov-report=xml --cov-report=term' || '' }} \ --durations=0 --json-report --json-report-file=unit-durations.json - name: Upload coverage to Codecov - if: always() + # רק במסלולים שמדדו coverage. Codecov ממזג את ההעלאות של אותו קומיט, ובלי + # ``after_n_builds`` ב-codecov.yml הוא לא מחכה למספר מסוים שלהן. + if: ${{ always() && matrix.coverage }} uses: codecov/codecov-action@v4 with: files: coverage.xml diff --git a/docs/ci-cd.rst b/docs/ci-cd.rst index 371cab156..0d43e5f8f 100644 --- a/docs/ci-cd.rst +++ b/docs/ci-cd.rst @@ -27,7 +27,7 @@ CI/CD Guide -------------------- - **Code Quality & Security** – בדיקות סטטיות ואבטחה -- **Unit Tests (3.11/3.12)** ו-**Unit Tests md-heavy (3.11/3.12)** – טסטי היחידה, בשני מסלולים לכל גרסת פייתון שרצים במקביל. ``Unit Tests`` מריץ את כל הטסטים חוץ מאלה שמסומנים ``md_heavy``, ו-``Unit Tests md-heavy`` מריץ רק אותם: טסטי פרסר ה-Markdown הכבדים (ראו :doc:`testing`). כל מסלול מדווח סטטוס חובה משלו, והמסלול ``md-heavy`` אינו מרים את MongoDB ואת Redis (``services`` של ``unit-tests``). +- **Unit Tests (3.11/3.12)** ו-**Unit Tests md-heavy (3.11/3.12)** – טסטי היחידה, בשני מסלולים לכל גרסת פייתון שרצים במקביל. ``Unit Tests`` מריץ את כל הטסטים חוץ מאלה שמסומנים ``md_heavy``, ו-``Unit Tests md-heavy`` מריץ רק אותם: טסטי פרסר ה-Markdown הכבדים (ראו :doc:`testing`). כל מסלול מדווח סטטוס חובה משלו, והמסלול ``md-heavy`` אינו מרים את MongoDB ואת Redis (``services`` של ``unit-tests``). coverage נמדד ונשלח ל-Codecov רק במסלולים שהמטריצה מסמנת ב-``coverage`` (רשומת ``include`` לפי גרסת פייתון), כדי לא לשלם פעמיים על ההאטה שהוא מוסיף לטסטים, וההעלאות של אותו קומיט מתמזגות ב-Codecov לדוח אחד. - **JS Tests (node)** – טסטי הצד-לקוח שב-``tests/*.test.js``. כל קובץ הוא סקריפט עצמאי שמריץ את עצמו ויוצא עם קוד שגיאה בכשל, בלי רץ טסטים חיצוני. ``repo-history.test.js`` מדולג במפורש: הוא כתוב בסגנון ``describe``/``it`` ואין בפרויקט רץ שמספק אותם. **אינו סטטוס נדרש** – הכשל מופיע ב-PR אך אינו חוסם מיזוג. - **Performance Tests** – טסטי ביצועים (ברירת מחדל: הכל; Draft + ``perf-light``: רק קלים). דוחות זמני ריצה נשמרים כארטיפקטים. diff --git a/docs/testing.rst b/docs/testing.rst index dcd3de64e..079694c30 100644 --- a/docs/testing.rst +++ b/docs/testing.rst @@ -221,6 +221,7 @@ Mocking HTTP ב‑github_menu_handler -------------------------- - הפרויקט מגדיר ``pytest-cov`` ב-``pytest.ini``. אם חסר, התקינו: ``pip install pytest-cov``. +- ב-CI הוא נמדד רק בחלק מהמסלולים של ``unit-tests`` — ראו :doc:`ci-cd`. - דוחות: .. code-block:: bash diff --git a/docs/whats-new.rst b/docs/whats-new.rst index 39280500a..66bcaefd6 100644 --- a/docs/whats-new.rst +++ b/docs/whats-new.rst @@ -4,6 +4,7 @@ What's New 2026-10-04 ---------- +- **ci: coverage נמדד רק במסלולי 3.11 של ``unit-tests``.** הוא הוסיף לטסטים של 3.12 כ-30% מהזמן שלהם (הערכה: מדידה מקומית עם coverage ובלעדיו, על זמני הטסטים מדוחות ``unit-durations-*`` של ריצת ``ci.yml`` מ-4.10), ובאותה ריצה התוצאות של 3.11 ושל 3.12 היו זהות, טסט מול טסט. עכשיו רשומת ``include`` במטריצה מוסיפה ``coverage: true`` לגרסה אחת, והדגלים ``--cov`` וההעלאה ל-Codecov תלויים בה. הגרסה היא 3.11, זו שהפרודקשן רץ עליה (``Dockerfile``). שמות הסטטוסים לא השתנו. ``tests/test_required_checks_are_listed.py`` פורש עכשיו את ``include`` לפי הכלל של GitHub, ובודק שלכל מסלול יש גרסה שמודדת coverage ושהדגלים וההעלאה תלויים במפתח הזה, כי Codecov אינו בדיקת חובה, וכיסוי שנעלם לא היה מפיל דבר. ראו :doc:`ci-cd`. - **tests: השהיות של קוד הייצור מתאפסות פעם אחת לכל הטסטים, ב-``tests/conftest.py``.** ``apply_rate_limit_delay`` ממתינה בין קריאות ל-GitHub, וטסטי הורדת ה-ZIP עברו בה באמת: כ-98 שניות של המתנה בכל מסלול ``Unit Tests``, לפי דוחות ``unit-durations-*`` של ריצת ``ci.yml`` מ-4.10. שני קבצים אחרים כבר נטרלו אותה, כל אחד לעצמו, והקבצים האלה לא. עכשיו ``tests/conftest.py`` מאפס את ``GITHUB_API_BASE_DELAY`` ואת ``GITHUB_BACKOFF_DELAY``, והנטרולים הידניים נמחקו. אליו עברו גם ערכי ה-retry וה-backoff שהיו ב-env של ``ci.yml`` (``HTTP_RESILIENCE_*``, ``REQUESTS_RETRIES``, ``REQUESTS_RETRY_BACKOFF``, ``AIOHTTP_TIMEOUT_TOTAL``), כך שהם חלים עכשיו גם על ``deploy.yml`` ועל הרצה מקומית. **ההמתנה גם הסתירה שני טסטים, וזה נמדד במוטציות:** ``test_backup_id_is_unique_within_the_same_second`` לא הגיע אף פעם לשתי אריזות באותה שנייה, ועבר גם בלי הסיומת האקראית של המזהה. ו-``test_apply_rate_limit_delay_respects_backoff`` עבר גם כשה-backoff לא הופעל, כי ההשהיה הבסיסית הייתה ארוכה ממנו. עכשיו הראשון בודק את מה שהוא מבטיח, השני קובע את ההשהיה הבסיסית בעצמו, וטסט חדש עובר דרך ההמתנה הבסיסית האמיתית, בערך קטן שהוא קובע. ראו :doc:`testing`. - **ci: טסטי ה-Markdown הכבדים רצים במסלול נפרד, במקביל לשאר טסטי היחידה.** בחודש שעבר זמן הטסטים ב-CI עלה מכשלוש דקות וחצי לכשבע וחצי (שלב ה-pytest בג'וב של 3.12, לפי דוחות ``unit-durations-*`` של ריצות ``ci.yml`` מ-9.9 ומ-4.10). הטסטים שהיו קיימים לא הואטו בסך הכול; את התוספת הביאו טסטים חדשים, ואת החלק הגדול שלה מעט טסטים כבדים. ביניהם טסטי פרסר ה-Markdown שעיקר העבודה שלהם הוא החישוב עצמו: השוואה לאורקל cmark-gfm, קלט ענק בתקרות של הפרסר, והטסטים של סקריפטי המדידה. הם מסומנים עכשיו ``md_heavy`` (הרישום ב-``pytest.ini``), והג'וב ``unit-tests`` ב-``.github/workflows/ci.yml`` רץ בשני מסלולים לכל גרסת פייתון: ``Unit Tests`` מריץ את כל השאר, ו-``Unit Tests md-heavy`` מריץ רק אותם, במקביל ובלי MongoDB ו-Redis שהוא אינו צריך. כל מסלול מדווח סטטוס חובה משלו. **את שני הסטטוסים החדשים צריך לסמן כבדיקות חובה בהגדרות של GitHub**, ועד שזה נעשה כשל בהם מופיע ב-PR אך אינו חוסם מיזוג. ושמות הסטטוסים, שכתובים ביד בכמה קבצים, נבדקים עכשיו מול המטריצה ב-``tests/test_required_checks_are_listed.py`` — והוא נכשל גם כשעותק של הרשימה מופיע בקובץ שהוא לא מכיר. :doc:`testing` ו-:doc:`branch-protection-and-pr-rules` מפנים עכשיו לרשימה שב-:doc:`ci-cd` במקום להחזיק עותק משלהם. - **fix (שירות ה-MCP): ריפו שהוסר בממשק נמחק גם מהדיסק של שירות ה-MCP.** הסרת ריפו בוובאפ מחקה את המראה שבדיסק של הוובאפ ואת הרשומות ב-``repo_metadata``, אבל לדיסק של שירות ה-MCP אף אחד לא נגע: המראה שם נשארה, וכלי הקריאה המשיכו להגיש אותה כאילו כלום לא קרה (כך קרה ל-``codekeeper-plugin``). עכשיו בסוף כל מעבר של ה-autosync, כל מראה בדיסק של השירות שאין לה רשומה ב-``repo_metadata`` נמחקת, דרך ``delete_mirror``. שום דבר לא נמחק כשהשאילתה נכשלה, או כשאף שם ב-``repo_metadata`` אינו שם של מראה שבדיסק (כולל קריאה ריקה). כשיש בדיסק מראות שנחסכו כך, נרשמת אזהרה. וגם לא נמחקים תיקייה שאינה מראה שהשירות יוצר (שם לא תקין, קישור סמלי) או ריפו שנמצא באמצע clone או fetch. מחיקה שנכשלה, או תיקיית מראות שאי אפשר לקרוא, נספרות ב-``errors``, והמראה נשארת עד המעבר הבא. הקריאה של ``repo_metadata`` נעשית מה-primary במפורש. בשורת הלוג ``repo autosync pass:`` נוסף המונה ``pruned``. ובאותה מנייה של המראות, גם ניקוי ה-credentials בעלייה מדווח עכשיו על תיקיית מראות שאי אפשר לקרוא ככשל, ולא כ-``checked=0 failed=0``. ראו :ref:`mcp-repo-autosync`. diff --git a/tests/test_required_checks_are_listed.py b/tests/test_required_checks_are_listed.py index 0edea2eeb..b24349227 100644 --- a/tests/test_required_checks_are_listed.py +++ b/tests/test_required_checks_are_listed.py @@ -12,7 +12,7 @@ ``test_every_file_that_names_a_unit_tests_status_is_checked``, שמחפש שמות של סטטוסים בכל הקבצים שבמעקב של git — כי גם ``LISTS`` נכתבת ביד, ועותק שנשכח ממנה לא היה נבדק. -ובנוסף, שני דברים שהמסלולים עצמם נשענים עליהם: +ובנוסף, מה שהמסלולים עצמם נשענים עליו: - **הביטויים של המסלולים משלימים זה את זה** (``marker`` במטריצה). טסט שאף מסלול לא בוחר אינו רץ ב-CI בכלל, ואיש לא מקבל על כך הודעה. @@ -20,6 +20,8 @@ הטסטים, או בכל הטסטים בלי ``ci.yml``: המסלול הכבד היה בוחר אפס טסטים. pytest יוצא אז בקוד ``ExitCode.NO_TESTS_COLLECTED``, וה-CI נשען על זה, ולכן זה מקובע כאן ולא רק כתוב. טסט בודד שהסימון שלו חסר או שגוי אינו נתפס כך, וגם לא צריך: הוא רץ במסלול הרגיל. +- **לכל מסלול יש גרסה שמודדת coverage**, ודגלי ה-coverage וההעלאה ל-Codecov תלויים שניהם + במפתח ``coverage`` של המטריצה. Codecov אינו בדיקת חובה, ולכן כיסוי שנעלם לא היה מפיל דבר. """ from __future__ import annotations @@ -66,6 +68,9 @@ #: ביטוי ``-m`` שהוא שם של סימון אחד, בלי ``not``, ``and`` או סוגריים. _BARE_MARKER = re.compile(r"\w+") +#: הביטוי בפקודת ה-pytest שמוסיף את דגלי ה-coverage רק כש-``matrix.coverage`` דלוק. +_COVERAGE_FLAGS = re.compile(r"\$\{\{\s*matrix\.coverage\s*&&\s*'([^']*)'\s*\|\|\s*''\s*\}\}") + def _unit_tests_job() -> dict: workflow = yaml.safe_load(CI_WORKFLOW.read_text(encoding="utf-8")) @@ -106,25 +111,28 @@ def _status_template(job: dict) -> str: def _matrix_combinations(job: dict) -> list[dict]: """כל צירוף של המטריצה, עם המפתחות ש-``include`` מוסיף לו. - ב-``include`` כל רשומה נקשרת למסלול אחד לפי ``suite`` ומוסיפה לו מפתחות. זה - המקרה היחיד שהטסט יודע לפרוש, ולכן הוא מוודא שזה המבנה ולא מנחש. + לפי הכלל של GitHub (``jobs..strategy.matrix.include`` ב-``workflow-syntax.md``): + רשומה מתווספת לכל צירוף שאף זוג מפתח-ערך שלה אינו דורס בו ערך מקורי של המטריצה, ומפתח + שרשומה קודמת הוסיפה מותר לדרוס. רשומה שאינה מתאימה לאף צירוף יוצרת אצל GitHub צירוף + חדש — מסלול שלם, שאין לו שם ברשימות של בדיקות החובה — ולכן כאן היא נכשלת ולא נפרשת + בניחוש. מאותה סיבה הטסט גם אינו פורש ``exclude``, ונכשל עליו. """ matrix = job["strategy"]["matrix"] - versions = matrix["python-version"] - suites = matrix["suite"] - by_suite = {} - for entry in matrix["include"]: - assert entry.get("suite") in suites, f"רשומת include בלי suite מוכר: {entry!r}" - assert "python-version" not in entry, f"רשומת include שמשנה גרסה: {entry!r}" - assert entry["suite"] not in by_suite, f"שתי רשומות include לאותו suite: {entry!r}" - by_suite[entry["suite"]] = entry - assert set(by_suite) == set(suites), ( - f"לכל suite צריכה להיות רשומת include: יש {sorted(by_suite)}, צריך {sorted(suites)}" - ) - return [ - {**by_suite[suite], "python-version": version, "suite": suite} - for version, suite in itertools.product(versions, suites) - ] + assert "exclude" not in matrix, "המטריצה של unit-tests משתמשת ב-exclude, והטסט אינו יודע לפרוש אותו" + axes = {key: values for key, values in matrix.items() if key != "include"} + for key, values in axes.items(): + assert isinstance(values, list), f"ציר המטריצה {key!r} אינו רשימה של ערכים: {values!r}" + combinations = [dict(zip(axes, values)) for values in itertools.product(*axes.values())] + for entry in matrix.get("include", []): + targets = [ + combination + for combination in combinations + if all(combination[key] == value for key, value in entry.items() if key in axes) + ] + assert targets, f"רשומת include שאינה מתאימה לאף צירוף, ולכן הייתה יוצרת מסלול חדש: {entry!r}" + for combination in targets: + combination.update({key: value for key, value in entry.items() if key not in axes}) + return combinations def _expected_statuses() -> set[str]: @@ -132,6 +140,8 @@ def _expected_statuses() -> set[str]: template = _status_template(job) names = set() for combination in _matrix_combinations(job): + missing = sorted(set(_MATRIX_EXPRESSION.findall(template)) - set(combination)) + assert not missing, f"למסלול {combination!r} אין במטריצה את מה ששם הסטטוס בנוי ממנו: {missing}" name = _MATRIX_EXPRESSION.sub(lambda match: combination[match.group(1)], template) assert "${{" not in name, f"נשאר ביטוי שהטסט אינו יודע לפרוש: {name!r}" names.add(name) @@ -256,6 +266,43 @@ def test_the_lanes_split_the_suite_into_two_complementary_halves(): ) +def test_coverage_is_measured_in_every_suite_and_only_where_the_matrix_says(): + """coverage נמדד רק בחלק מהמסלולים, ו-Codecov אינו בדיקת חובה — כיסוי שנעלם לא מפיל דבר. + + המפתח ``coverage`` במטריצה קובע איפה הוא נמדד, ושני צעדים קוראים אותו: פקודת ה-pytest + (הדגלים ``--cov``) וההעלאה ל-Codecov. הטסט בודק את שלושתם יחד: שלכל suite יש מסלול + שמודד coverage, אחרת הטסטים שלו אינם בדוח; שהדגלים נמצאים רק בתוך הביטוי שתלוי + ב-``matrix.coverage``; ושההעלאה תלויה בו. ערך שאינו ``true``/``false`` נכשל גם הוא: מחרוזת + כמו ``'false'`` אינה בין הערכים ש-GitHub מחשיב כשקר (expressions בתיעוד של GitHub + Actions), ולכן הייתה מדליקה את ה-coverage. + """ + job = _unit_tests_job() + combinations = _matrix_combinations(job) + for combination in combinations: + assert isinstance(combination.get("coverage", False), bool), ( + f"coverage במטריצה אינו true/false: {combination!r}" + ) + suites = {combination["suite"] for combination in combinations} + covered = {combination["suite"] for combination in combinations if combination.get("coverage")} + assert covered == suites, f"מסלולים שאין להם אף גרסה שמודדת coverage: {sorted(suites - covered)}" + + runs = [str(step["run"]) for step in job["steps"] if "pytest" in str(step.get("run", ""))] + assert len(runs) == 1, f"צפוי צעד אחד שמריץ pytest ב-unit-tests, נמצאו {len(runs)}" + flags = _COVERAGE_FLAGS.findall(runs[0]) + assert len(flags) == 1 and "--cov=" in flags[0] and "--cov-report=xml" in flags[0], ( + f"דגלי ה-coverage אינם בביטוי אחד שתלוי ב-matrix.coverage ומפיק coverage.xml:\n{runs[0]}" + ) + assert "--cov" not in _COVERAGE_FLAGS.sub("", runs[0]), ( + f"--cov מחוץ לביטוי של matrix.coverage, ולכן נמדד בכל המסלולים:\n{runs[0]}" + ) + + uploads = [step for step in job["steps"] if str(step.get("uses", "")).startswith("codecov/codecov-action")] + assert len(uploads) == 1, f"צפוי צעד העלאה אחד ל-Codecov ב-unit-tests, נמצאו {len(uploads)}" + assert "matrix.coverage" in str(uploads[0].get("if", "")), ( + f"ההעלאה ל-Codecov אינה תלויה ב-matrix.coverage: {uploads[0].get('if')!r}" + ) + + #: ``1 deselected`` בסיכום של ריצה בלי ``-n`` מראה שהטסט נאסף ואז סונן, ולכן קוד היציאה #: בא מהסינון ולא מקובץ שאין בו טסטים. תחת ``-n`` ה-controller מסכם ``no tests ran`` בלי #: לספור את מה שסונן ב-workers (נמדד ב-pytest-xdist 3.8.0), ושם אותו קובץ ואותו ביטוי From 24973f8891ec7dc3d915b1a0c7b6f592068570dc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 21:10:05 +0000 Subject: [PATCH 3/6] =?UTF-8?q?tests:=20=D7=98=D7=A1=D7=98=20=D7=94-backof?= =?UTF-8?q?f=20=D7=9C=D7=90=20=D7=A0=D7=9B=D7=A9=D7=9C=20=D7=99=D7=95?= =?UTF-8?q?=D7=AA=D7=A8=20=D7=9C=D7=A4=D7=99=20=D7=A1=D7=93=D7=A8=20=D7=94?= =?UTF-8?q?=D7=A8=D7=99=D7=A6=D7=94,=20=D7=95=D7=A2=D7=A8=D7=9B=D7=99=20?= =?UTF-8?q?=D7=94=D7=94=D7=A9=D7=94=D7=99=D7=94=20=D7=A9=D7=9C=20conftest?= =?UTF-8?q?=20=D7=92=D7=95=D7=91=D7=A8=D7=99=D7=9D=20=D7=A2=D7=9C=20=D7=94?= =?UTF-8?q?=D7=9E=D7=A2=D7=98=D7=A4=D7=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit שורש הכשל ב-CI: test_telegram_retry_after_and_message_not_modified הוציא את github_menu_handler מ-sys.modules וייבא אותו מחדש, בלי להחזיר את המודול הקודם. טסט ה-backoff, שרץ אחריו באותו worker, החזיק מחלקה מהמודול הקודם ופאץ' את github_backoff_state במודול החדש. לכן ה-backoff לא הופעל וההמתנה הייתה אפס. לפני ה-PR זה לא נראה, כי ההשהיה הבסיסית של הייצור (2 שניות) הסתירה את ה-backoff. - התיקון בשורש: monkeypatch.delitem מחזיר את המודול הקודם ל-sys.modules בסוף הטסט. - טסט ה-backoff בונה את GitHubMenuHandler מאותו מודול שהוא מפאץ', ולא ממחלקה שיובאה בזמן האיסוף. - tests/conftest.py: השמה במקום setdefault לתשעת ערכי ההשהיה. כך ערך שהמעטפת מייצאת לא מחזיר את ההמתנה האמיתית. - docs/testing.rst: מתאר את ההשמה, בלי הטענה "בראש הקובץ". - טסט ההשהיה הבסיסית: סף אחד (delay * 0.8) לשתי הקריאות. - הודעת ה-exclude ב-test_required_checks_are_listed אומרת מה להוסיף לפני שמשתמשים בו. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3jHDFecUvpZMMo7czCNQ8 --- docs/testing.rst | 2 +- tests/conftest.py | 22 ++++++++++-------- tests/test_error_recovery_db_and_telegram.py | 8 ++++++- tests/test_github_menu_backoff_delay.py | 24 +++++++++++++------- tests/test_required_checks_are_listed.py | 6 ++++- 5 files changed, 42 insertions(+), 20 deletions(-) diff --git a/docs/testing.rst b/docs/testing.rst index 079694c30..b0735219f 100644 --- a/docs/testing.rst +++ b/docs/testing.rst @@ -130,7 +130,7 @@ Mocking HTTP ב‑github_menu_handler השהיות של קוד הייצור בטסטים --------------------------- -**קוד הייצור ממתין בכוונה, והטסטים לא ממתינים איתו.** ה-rate limit בין קריאות ל-GitHub (``apply_rate_limit_delay`` ב-``github_menu_handler.py``), ה-retry עם backoff (``resilience.py``, ``http_sync.py``) ותקרת ההמתנה לתשובה ב-``http_async.py`` ממתינים באמת, וטסט שעובר בהם ממתין ועובר בלי שאיש יראה. לכן ``tests/conftest.py`` קובע להם ערכי בדיקה פעם אחת, בבלוק משתני הסביבה שבראש הקובץ. הערכים חלים על כל הרצה: ``ci.yml``, ``deploy.yml`` והרצה מקומית, ולכן הם לא נקבעים ב-env של workflow. +**קוד הייצור ממתין בכוונה, והטסטים לא ממתינים איתו.** ה-rate limit בין קריאות ל-GitHub (``apply_rate_limit_delay`` ב-``github_menu_handler.py``), ה-retry עם backoff (``resilience.py``, ``http_sync.py``) ותקרת ההמתנה לתשובה ב-``http_async.py`` ממתינים באמת, וטסט שעובר בהם ממתין ועובר בלי שאיש יראה. לכן ``tests/conftest.py`` קובע להם ערכי בדיקה פעם אחת, בבלוק משתני הסביבה שלו. הוא קובע אותם בהשמה ולא ב-``setdefault``, ולכן הם חלים על כל הרצה — ``ci.yml``, ``deploy.yml`` והרצה מקומית — גם כשהמעטפת מייצאת ערך אחר, ומאותה סיבה הם לא נקבעים ב-env של workflow. - **טסט שהנושא שלו הוא ההשהיה עצמה קובע אותה בעצמו** ב-``monkeypatch.setenv``, ולא נשען על ברירת המחדל של הייצור או של ``tests/conftest.py``. כך עושים ``tests/test_github_menu_backoff_delay.py`` ו-``tests/test_http_sync_adapter_retries.py``. - **השהיה חדשה בקוד הייצור מקבלת ערך בדיקה באותו בלוק, באותו PR.** לא מנטרלים אותה בקובץ הטסטים שבמקרה שם לב אליה, כי הקובץ הבא לא יידע על כך. diff --git a/tests/conftest.py b/tests/conftest.py index 633b4037a..ebd2e6242 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -51,15 +51,19 @@ # אליה כשכל קריאה ממתינה. טסט שהנושא שלו הוא ההשהיה עצמה קובע אותה בעצמו # (``monkeypatch.setenv``), כמו ב-``tests/test_github_menu_backoff_delay.py``. והשהיה חדשה # בקוד הייצור מקבלת כאן ערך בדיקה באותו PR, ולא נטרול בקובץ הטסטים שבמקרה שם לב אליה. -os.environ.setdefault('GITHUB_API_BASE_DELAY', '0') -os.environ.setdefault('GITHUB_BACKOFF_DELAY', '0') -os.environ.setdefault('HTTP_RESILIENCE_MAX_ATTEMPTS', '1') -os.environ.setdefault('HTTP_RESILIENCE_BACKOFF_BASE', '0') -os.environ.setdefault('HTTP_RESILIENCE_BACKOFF_MAX', '0') -os.environ.setdefault('HTTP_RESILIENCE_JITTER', '0') -os.environ.setdefault('REQUESTS_RETRIES', '0') -os.environ.setdefault('REQUESTS_RETRY_BACKOFF', '0') -os.environ.setdefault('AIOHTTP_TIMEOUT_TOTAL', '6') +# +# השמה ולא ``setdefault`` כמו בשאר הבלוק: כאן אין ערך מבחוץ שהטסטים צריכים לכבד, ו- +# ``setdefault`` היה משאיר ערך שהמעטפת מייצאת — למשל כשמריצים באותה מעטפת גם את הבוט — +# ומחזיר בשקט את ההמתנה האמיתית. +os.environ['GITHUB_API_BASE_DELAY'] = '0' +os.environ['GITHUB_BACKOFF_DELAY'] = '0' +os.environ['HTTP_RESILIENCE_MAX_ATTEMPTS'] = '1' +os.environ['HTTP_RESILIENCE_BACKOFF_BASE'] = '0' +os.environ['HTTP_RESILIENCE_BACKOFF_MAX'] = '0' +os.environ['HTTP_RESILIENCE_JITTER'] = '0' +os.environ['REQUESTS_RETRIES'] = '0' +os.environ['REQUESTS_RETRY_BACKOFF'] = '0' +os.environ['AIOHTTP_TIMEOUT_TOTAL'] = '6' # Import stubs so any import of `telegram` succeeds in tests try: diff --git a/tests/test_error_recovery_db_and_telegram.py b/tests/test_error_recovery_db_and_telegram.py index bf28f4ee1..59c725a4e 100644 --- a/tests/test_error_recovery_db_and_telegram.py +++ b/tests/test_error_recovery_db_and_telegram.py @@ -42,7 +42,13 @@ def get_latest_version(self, uid, name): async def test_telegram_retry_after_and_message_not_modified(monkeypatch): # Use a small slice of handler that edits messages via safe edit path import sys - sys.modules.pop('github_menu_handler', None) + # ייבוא טרי של המודול לטסט הזה. אם המודול כבר היה טעון, ``monkeypatch.delitem`` רושם אותו + # ומחזיר אותו ל-``sys.modules`` בסוף הטסט (pytest 8.4.2: ``delitem`` ו-``undo`` ב- + # ``_pytest/monkeypatch.py``). ה-``sys.modules.pop`` שהיה כאן השאיר אחריו מודול אחר, וטסט + # שרץ אחריו באותו תהליך והחזיק מחלקה מהמודול הקודם פאץ' משתנה במודול שהקוד שלו לא קורא — + # כך נכשל ``test_apply_rate_limit_delay_respects_backoff`` שב- + # ``tests/test_github_menu_backoff_delay.py``, ב-CI של PR #3524. + monkeypatch.delitem(sys.modules, 'github_menu_handler', raising=False) import github_menu_handler as gh handler = gh.GitHubMenuHandler() diff --git a/tests/test_github_menu_backoff_delay.py b/tests/test_github_menu_backoff_delay.py index ce01a471c..3dc136f03 100644 --- a/tests/test_github_menu_backoff_delay.py +++ b/tests/test_github_menu_backoff_delay.py @@ -3,12 +3,18 @@ import types import pytest -from github_menu_handler import GitHubMenuHandler +# הטסטים כאן משנים את ``github_backoff_state`` במודול, ולכן בונים את ``GitHubMenuHandler`` +# מאותו אובייקט מודול שהם משנים, ולא ממחלקה שיובאה בזמן האיסוף. טסט שמייבא את +# ``github_menu_handler`` מחדש ולא מחזיר את הקודם משאיר ב-``sys.modules`` מודול אחר, והשינוי +# נוחת אז במודול שהקוד שרץ לא קורא ממנו. כך היה עם ``tests/test_error_recovery_db_and_telegram.py`` +# עד PR #3524. @pytest.mark.asyncio async def test_apply_rate_limit_delay_respects_backoff(monkeypatch): - h = GitHubMenuHandler() + import github_menu_handler as mod + + h = mod.GitHubMenuHandler() user_id = 123 # Seed last call now @@ -22,7 +28,6 @@ class _State: def get(self): return _Info() # Patch services.github_backoff_state reference inside module - import github_menu_handler as mod monkeypatch.setattr(mod, "github_backoff_state", _State(), raising=False) # ההשהיה הבסיסית מאופסת כאן במפורש, כדי שכל ההמתנה תבוא מה-backoff. כשהיא רצה @@ -43,8 +48,8 @@ def get(self): async def test_apply_rate_limit_delay_waits_the_base_delay_between_two_calls(monkeypatch): """שתי קריאות צמודות של אותו משתמש: הראשונה עוברת מיד, והשנייה ממתינה את ההשהיה הבסיסית. - ``tests/conftest.py`` מאפס את ``GITHUB_API_BASE_DELAY`` לכל הטסטים, ולכן זה הטסט שעובר - דרך ההמתנה האמיתית. הוא קובע ערך קטן משלו, ומנטרל את מצב ה-backoff הגלובלי, כדי שההמתנה + ``tests/conftest.py`` מאפס את ``GITHUB_API_BASE_DELAY`` לכל הטסטים, ולכן הטסט הזה קובע ערך + קטן משלו, כדי לעבור דרך ההמתנה האמיתית. הוא גם מנטרל את מצב ה-backoff הגלובלי, כדי שההמתנה תבוא רק מההשהיה הבסיסית. """ import github_menu_handler as mod @@ -52,7 +57,7 @@ async def test_apply_rate_limit_delay_waits_the_base_delay_between_two_calls(mon delay = 0.3 monkeypatch.setattr(mod, "github_backoff_state", None, raising=False) monkeypatch.setenv("GITHUB_API_BASE_DELAY", str(delay)) - h = GitHubMenuHandler() + h = mod.GitHubMenuHandler() start = time.monotonic() await h.apply_rate_limit_delay(7) @@ -60,5 +65,8 @@ async def test_apply_rate_limit_delay_waits_the_base_delay_between_two_calls(mon await h.apply_rate_limit_delay(7) second = time.monotonic() - start - first - assert first < delay / 2, f"הקריאה הראשונה לא אמורה להמתין, והמתינה {first:.3f} שנ'" - assert second >= delay * 0.8, f"הקריאה השנייה הייתה אמורה להמתין כ-{delay} שנ', והמתינה {second:.3f}" + # סף אחד בין "המתינה" ל"לא המתינה": קריאה שממתינה את ההשהיה עוברת אותו, וקריאה שלא + # ממתינה נשארת רחוק מתחתיו גם במכונה עמוסה. + waited = delay * 0.8 + assert first < waited, f"הקריאה הראשונה לא אמורה להמתין, והמתינה {first:.3f} שנ'" + assert second >= waited, f"הקריאה השנייה הייתה אמורה להמתין כ-{delay} שנ', והמתינה {second:.3f}" diff --git a/tests/test_required_checks_are_listed.py b/tests/test_required_checks_are_listed.py index b24349227..2eefcc55b 100644 --- a/tests/test_required_checks_are_listed.py +++ b/tests/test_required_checks_are_listed.py @@ -118,7 +118,11 @@ def _matrix_combinations(job: dict) -> list[dict]: בניחוש. מאותה סיבה הטסט גם אינו פורש ``exclude``, ונכשל עליו. """ matrix = job["strategy"]["matrix"] - assert "exclude" not in matrix, "המטריצה של unit-tests משתמשת ב-exclude, והטסט אינו יודע לפרוש אותו" + assert "exclude" not in matrix, ( + "המטריצה של unit-tests משתמשת ב-exclude, והטסט אינו יודע לפרוש אותו. לפני שמוסיפים " + "exclude, הוסיפו ל-_matrix_combinations את הכלל של GitHub: הצירופים ש-exclude מתאר מוסרים, " + "ורק אחר כך include מעובד" + ) axes = {key: values for key, values in matrix.items() if key != "include"} for key, values in axes.items(): assert isinstance(values, list), f"ציר המטריצה {key!r} אינו רשימה של ערכים: {values!r}" From 273b48d7d9c7da3e0cb80fbe20fdf709f2e17260 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 21:14:02 +0000 Subject: [PATCH 4/6] =?UTF-8?q?docs(whats-new):=20=D7=94=D7=93=D7=9C=D7=99?= =?UTF-8?q?=D7=A4=D7=94=20=D7=91-sys.modules=20=D7=A9=D7=98=D7=A1=D7=98=20?= =?UTF-8?q?=D7=94-backoff=20=D7=97=D7=A9=D7=A3,=20=D7=95=D7=94=D7=94=D7=A9?= =?UTF-8?q?=D7=9E=D7=94=20=D7=91-conftest?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit הרשומה של השהיות הייצור מתארת עכשיו גם את מה שתוקן בסבב הסקירה: טסט שהחליף את github_menu_handler ב-sys.modules ולא החזיר אותו, ושינה כך את התוצאה של טסט ה-backoff לפי סדר הריצה; וערכי ההשהיה ב-tests/conftest.py שנקבעים בהשמה ולא ב-setdefault. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3jHDFecUvpZMMo7czCNQ8 --- docs/whats-new.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/whats-new.rst b/docs/whats-new.rst index 66bcaefd6..d7093cf4f 100644 --- a/docs/whats-new.rst +++ b/docs/whats-new.rst @@ -5,7 +5,7 @@ What's New 2026-10-04 ---------- - **ci: coverage נמדד רק במסלולי 3.11 של ``unit-tests``.** הוא הוסיף לטסטים של 3.12 כ-30% מהזמן שלהם (הערכה: מדידה מקומית עם coverage ובלעדיו, על זמני הטסטים מדוחות ``unit-durations-*`` של ריצת ``ci.yml`` מ-4.10), ובאותה ריצה התוצאות של 3.11 ושל 3.12 היו זהות, טסט מול טסט. עכשיו רשומת ``include`` במטריצה מוסיפה ``coverage: true`` לגרסה אחת, והדגלים ``--cov`` וההעלאה ל-Codecov תלויים בה. הגרסה היא 3.11, זו שהפרודקשן רץ עליה (``Dockerfile``). שמות הסטטוסים לא השתנו. ``tests/test_required_checks_are_listed.py`` פורש עכשיו את ``include`` לפי הכלל של GitHub, ובודק שלכל מסלול יש גרסה שמודדת coverage ושהדגלים וההעלאה תלויים במפתח הזה, כי Codecov אינו בדיקת חובה, וכיסוי שנעלם לא היה מפיל דבר. ראו :doc:`ci-cd`. -- **tests: השהיות של קוד הייצור מתאפסות פעם אחת לכל הטסטים, ב-``tests/conftest.py``.** ``apply_rate_limit_delay`` ממתינה בין קריאות ל-GitHub, וטסטי הורדת ה-ZIP עברו בה באמת: כ-98 שניות של המתנה בכל מסלול ``Unit Tests``, לפי דוחות ``unit-durations-*`` של ריצת ``ci.yml`` מ-4.10. שני קבצים אחרים כבר נטרלו אותה, כל אחד לעצמו, והקבצים האלה לא. עכשיו ``tests/conftest.py`` מאפס את ``GITHUB_API_BASE_DELAY`` ואת ``GITHUB_BACKOFF_DELAY``, והנטרולים הידניים נמחקו. אליו עברו גם ערכי ה-retry וה-backoff שהיו ב-env של ``ci.yml`` (``HTTP_RESILIENCE_*``, ``REQUESTS_RETRIES``, ``REQUESTS_RETRY_BACKOFF``, ``AIOHTTP_TIMEOUT_TOTAL``), כך שהם חלים עכשיו גם על ``deploy.yml`` ועל הרצה מקומית. **ההמתנה גם הסתירה שני טסטים, וזה נמדד במוטציות:** ``test_backup_id_is_unique_within_the_same_second`` לא הגיע אף פעם לשתי אריזות באותה שנייה, ועבר גם בלי הסיומת האקראית של המזהה. ו-``test_apply_rate_limit_delay_respects_backoff`` עבר גם כשה-backoff לא הופעל, כי ההשהיה הבסיסית הייתה ארוכה ממנו. עכשיו הראשון בודק את מה שהוא מבטיח, השני קובע את ההשהיה הבסיסית בעצמו, וטסט חדש עובר דרך ההמתנה הבסיסית האמיתית, בערך קטן שהוא קובע. ראו :doc:`testing`. +- **tests: השהיות של קוד הייצור מתאפסות פעם אחת לכל הטסטים, ב-``tests/conftest.py``.** ``apply_rate_limit_delay`` ממתינה בין קריאות ל-GitHub, וטסטי הורדת ה-ZIP עברו בה באמת: כ-98 שניות של המתנה בכל מסלול ``Unit Tests``, לפי דוחות ``unit-durations-*`` של ריצת ``ci.yml`` מ-4.10. שני קבצים אחרים כבר נטרלו אותה, כל אחד לעצמו, והקבצים האלה לא. עכשיו ``tests/conftest.py`` מאפס את ``GITHUB_API_BASE_DELAY`` ואת ``GITHUB_BACKOFF_DELAY``, והנטרולים הידניים נמחקו. אליו עברו גם ערכי ה-retry וה-backoff שהיו ב-env של ``ci.yml`` (``HTTP_RESILIENCE_*``, ``REQUESTS_RETRIES``, ``REQUESTS_RETRY_BACKOFF``, ``AIOHTTP_TIMEOUT_TOTAL``), כך שהם חלים עכשיו גם על ``deploy.yml`` ועל הרצה מקומית. **ההמתנה גם הסתירה שני טסטים, וזה נמדד במוטציות:** ``test_backup_id_is_unique_within_the_same_second`` לא הגיע אף פעם לשתי אריזות באותה שנייה, ועבר גם בלי הסיומת האקראית של המזהה. ו-``test_apply_rate_limit_delay_respects_backoff`` עבר גם כשה-backoff לא הופעל, כי ההשהיה הבסיסית הייתה ארוכה ממנו. עכשיו הראשון בודק את מה שהוא מבטיח, השני קובע את ההשהיה הבסיסית בעצמו, וטסט חדש עובר דרך ההמתנה הבסיסית האמיתית, בערך קטן שהוא קובע. **ומשטסט ה-backoff התחיל לבדוק באמת, הוא נכשל ב-CI לפי סדר הריצה:** ``test_telegram_retry_after_and_message_not_modified`` הוציא את ``github_menu_handler`` מ-``sys.modules`` וייבא אותו מחדש בלי להחזיר את המודול הקודם, וטסט ה-backoff, שרץ אחריו באותו תהליך, שינה את מצב ה-backoff במודול שהקוד שרץ לא קורא ממנו. עכשיו ``monkeypatch.delitem`` מחזיר את המודול בסוף הטסט, וטסט ה-backoff בונה את ``GitHubMenuHandler`` מאותו מודול שהוא משנה. ובאותו סבב, ``tests/conftest.py`` קובע את ערכי ההשהיה בהשמה ולא ב-``setdefault``, כדי שערך שהמעטפת מייצאת לא יחזיר את ההמתנה. ראו :doc:`testing`. - **ci: טסטי ה-Markdown הכבדים רצים במסלול נפרד, במקביל לשאר טסטי היחידה.** בחודש שעבר זמן הטסטים ב-CI עלה מכשלוש דקות וחצי לכשבע וחצי (שלב ה-pytest בג'וב של 3.12, לפי דוחות ``unit-durations-*`` של ריצות ``ci.yml`` מ-9.9 ומ-4.10). הטסטים שהיו קיימים לא הואטו בסך הכול; את התוספת הביאו טסטים חדשים, ואת החלק הגדול שלה מעט טסטים כבדים. ביניהם טסטי פרסר ה-Markdown שעיקר העבודה שלהם הוא החישוב עצמו: השוואה לאורקל cmark-gfm, קלט ענק בתקרות של הפרסר, והטסטים של סקריפטי המדידה. הם מסומנים עכשיו ``md_heavy`` (הרישום ב-``pytest.ini``), והג'וב ``unit-tests`` ב-``.github/workflows/ci.yml`` רץ בשני מסלולים לכל גרסת פייתון: ``Unit Tests`` מריץ את כל השאר, ו-``Unit Tests md-heavy`` מריץ רק אותם, במקביל ובלי MongoDB ו-Redis שהוא אינו צריך. כל מסלול מדווח סטטוס חובה משלו. **את שני הסטטוסים החדשים צריך לסמן כבדיקות חובה בהגדרות של GitHub**, ועד שזה נעשה כשל בהם מופיע ב-PR אך אינו חוסם מיזוג. ושמות הסטטוסים, שכתובים ביד בכמה קבצים, נבדקים עכשיו מול המטריצה ב-``tests/test_required_checks_are_listed.py`` — והוא נכשל גם כשעותק של הרשימה מופיע בקובץ שהוא לא מכיר. :doc:`testing` ו-:doc:`branch-protection-and-pr-rules` מפנים עכשיו לרשימה שב-:doc:`ci-cd` במקום להחזיק עותק משלהם. - **fix (שירות ה-MCP): ריפו שהוסר בממשק נמחק גם מהדיסק של שירות ה-MCP.** הסרת ריפו בוובאפ מחקה את המראה שבדיסק של הוובאפ ואת הרשומות ב-``repo_metadata``, אבל לדיסק של שירות ה-MCP אף אחד לא נגע: המראה שם נשארה, וכלי הקריאה המשיכו להגיש אותה כאילו כלום לא קרה (כך קרה ל-``codekeeper-plugin``). עכשיו בסוף כל מעבר של ה-autosync, כל מראה בדיסק של השירות שאין לה רשומה ב-``repo_metadata`` נמחקת, דרך ``delete_mirror``. שום דבר לא נמחק כשהשאילתה נכשלה, או כשאף שם ב-``repo_metadata`` אינו שם של מראה שבדיסק (כולל קריאה ריקה). כשיש בדיסק מראות שנחסכו כך, נרשמת אזהרה. וגם לא נמחקים תיקייה שאינה מראה שהשירות יוצר (שם לא תקין, קישור סמלי) או ריפו שנמצא באמצע clone או fetch. מחיקה שנכשלה, או תיקיית מראות שאי אפשר לקרוא, נספרות ב-``errors``, והמראה נשארת עד המעבר הבא. הקריאה של ``repo_metadata`` נעשית מה-primary במפורש. בשורת הלוג ``repo autosync pass:`` נוסף המונה ``pruned``. ובאותה מנייה של המראות, גם ניקוי ה-credentials בעלייה מדווח עכשיו על תיקיית מראות שאי אפשר לקרוא ככשל, ולא כ-``checked=0 failed=0``. ראו :ref:`mcp-repo-autosync`. - **fix (מראות): טוקן ה-GitHub לא נשמר יותר בקובץ ה-config של המראות, ולא עובר בשורת הפקודה (#3480); וזיהוי הענף הראשי בייבוא ריפו (#3479).** עד עכשיו ``init_mirror`` שכפל מ-URL שהטוקן בתוכו, ו-git שמר אותו בטקסט גלוי ב-``remote.origin.url`` — על הדיסקים של הוובאפ ושל שירות ה-MCP, ובצילומים היומיים של Render; הוא גם הופיע בשורת הפקודה של ``git-remote-http`` בכל clone ו-fetch. עכשיו ה-URL השמור נקי, והטוקן עובר ככותרת דרך משתני סביבה של git, ורק לריפו שדורש הזדהות — ריפו ציבורי לא מקבל טוקן. מראות קיימות מנוקות לפני כל fetch ובעליית כל שירות, עם שורת לוג אחת שסופרת מה נמצא ומה נוקה, ואומרת לכל מראה מאיפה הטוקן שלה (``map``/``global``). fetch על מראה שלא אומת שהיא נקייה לא רץ (``mirror_url_not_clean``). ובנפרד: ``initial_import`` ניסה לזהות את הענף הראשי בפקודות ש-``_run_git_command`` דוחה, ולכן תמיד נפל ל-``main`` — וריפו שהענף הראשי שלו ``master`` לא יובא. עכשיו הזיהוי הוא מ-HEAD של המראה, וכשאין ענף מאומת מוחזר ``default_branch_undetected`` במקום ניחוש. ראו :ref:`mcp-mirror-credentials`. From 5312672c2e534cbbe2478ef807cab579b768d294 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 21:38:49 +0000 Subject: [PATCH 5/6] =?UTF-8?q?tests:=20=D7=98=D7=A1=D7=98=20=D7=94-backof?= =?UTF-8?q?f=20=D7=9E=D7=95=D7=93=D7=93=20=D7=90=D7=AA=20=D7=94=D7=94?= =?UTF-8?q?=D7=9E=D7=AA=D7=A0=D7=94=20=D7=9E=D7=94=D7=A7=D7=A8=D7=99=D7=90?= =?UTF-8?q?=D7=94=20=D7=94=D7=A7=D7=95=D7=93=D7=9E=D7=AA=20=D7=A9=D7=A0?= =?UTF-8?q?=D7=96=D7=A8=D7=A2=D7=94,=20=D7=95=D7=9C=D7=90=20=D7=9E=D7=AA?= =?UTF-8?q?=D7=97=D7=99=D7=9C=D7=AA=20=D7=94=D7=A7=D7=A8=D7=99=D7=90=D7=94?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit apply_rate_limit_delay מבטיח שהקריאה חוזרת רק אחרי ש-GITHUB_BACKOFF_DELAY עבר מהקריאה הקודמת. הטסט מדד מתחילת הקריאה, ולכן החסיר את הזמן שעבר בין הזריעה לקריאה. כשהפער הזה גדול מהמרווח ש-asyncio.sleep מוסיף (מכונה איטית, עצירה של GC), הטסט נכשל גם כשההמתנה נכונה. עם עצירה מלאכותית של 50ms אחרי הזריעה, המדידה הישנה יצאה 0.952 שנ' ונכשלה, והחדשה עוברת. מוטציה שמבטלת את ה-backoff עדיין מכשילה את הטסט (0.000 שנ'). הממצא: CodeRabbit (הערה מחוץ לדיף, tests/test_github_menu_backoff_delay.py). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3jHDFecUvpZMMo7czCNQ8 --- tests/test_github_menu_backoff_delay.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_github_menu_backoff_delay.py b/tests/test_github_menu_backoff_delay.py index 3dc136f03..82bb7d57f 100644 --- a/tests/test_github_menu_backoff_delay.py +++ b/tests/test_github_menu_backoff_delay.py @@ -18,7 +18,8 @@ async def test_apply_rate_limit_delay_respects_backoff(monkeypatch): user_id = 123 # Seed last call now - h.last_api_call[user_id] = time.time() + seeded = time.time() + h.last_api_call[user_id] = seeded # Stub github_backoff_state.get().is_active() -> True class _Info: @@ -36,12 +37,13 @@ def get(self): monkeypatch.setenv("GITHUB_API_BASE_DELAY", "0") # Reduce backoff delay to keep test fast, while still asserting >=1s monkeypatch.setenv("GITHUB_BACKOFF_DELAY", "1.0") - start = time.time() await h.apply_rate_limit_delay(user_id) - elapsed = time.time() - start + since_seed = time.time() - seeded - # Under backoff, the wait is GITHUB_BACKOFF_DELAY minus the time since the seeded call - assert elapsed >= 1.0 # at least some wait was applied + # תחת backoff הקריאה חוזרת רק אחרי ש-GITHUB_BACKOFF_DELAY עבר מהקריאה הקודמת, כלומר מהרגע + # שנזרע כאן, ולכן מודדים ממנו. מדידה מתחילת הקריאה מפספסת את הזמן שעבר מאז הזריעה, ובמכונה + # איטית, או בעצירה של GC באמצע, היא יוצאת קצרה משנייה גם כשההמתנה נכונה. + assert since_seed >= 1.0, f"הקריאה הייתה אמורה לחזור שנייה אחרי הקריאה הקודמת, וחזרה אחרי {since_seed:.3f} שנ'" @pytest.mark.asyncio From c0b8d6e523251936105aaa50b376fdf6ceab29d2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 22:00:26 +0000 Subject: [PATCH 6/6] =?UTF-8?q?tests:=20test=5Fuser=5Fstats=5Funit=20?= =?UTF-8?q?=D7=9B=D7=91=D7=A8=20=D7=9C=D7=90=20=D7=A2=D7=95=D7=A9=D7=94=20?= =?UTF-8?q?reload=20=D7=9C-user=5Fstats,=20=D7=95=D7=98=D7=A1=D7=98=D7=99?= =?UTF-8?q?=20log=5Fuser=5Factivity=20=D7=9C=D7=90=20=D7=A0=D7=9B=D7=A9?= =?UTF-8?q?=D7=9C=D7=99=D7=9D=20=D7=9C=D7=A4=D7=99=20=D7=A1=D7=93=D7=A8=20?= =?UTF-8?q?=D7=94=D7=A8=D7=99=D7=A6=D7=94?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ב-CI על 24973f8 נכשלו ב-3.11 test_log_user_activity_fallback_without_jobqueue ו-test_log_user_activity_weight_typeerror_fallback (assert 0 == 1). השורש: tests/test_user_stats_unit.py עשה importlib.reload ל-user_stats. ה-reload בנה מופע חדש של user_stats.user_stats, בזמן ש-main.py ממשיך להחזיק את הקודם (from user_stats import user_stats). טסט שרץ אחריו באותו worker והחליף את log_user במופע החדש לא הגיע לקוד של main. - ה-reload היה מיותר: _get_files_facade_or_none ב-user_stats.py מייבא את ה-facade בכל קריאה, ולכן ה-stub שב-sys.modules נקלט גם בלעדיו. הוא הוסר משלושת הטסטים, והערה בראש הקובץ מסבירה למה. - נבדק: הטסט שטוען את main, אחריו test_user_stats_unit, ואחריו כל הטסטים שמחליפים את log_user במופע מ-user_stats, באותו תהליך. עם ה-reload נכשלו שלושה, ובלעדיו כולם עברו, ב-3.11 וב-3.12. אותו סדר נכשל גם על main (ae4fd1f). - docs/whats-new.rst: משפט ברשומת השהיות הייצור. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3jHDFecUvpZMMo7czCNQ8 --- docs/whats-new.rst | 2 +- tests/test_user_stats_unit.py | 13 +++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/whats-new.rst b/docs/whats-new.rst index d7093cf4f..4021a1a42 100644 --- a/docs/whats-new.rst +++ b/docs/whats-new.rst @@ -5,7 +5,7 @@ What's New 2026-10-04 ---------- - **ci: coverage נמדד רק במסלולי 3.11 של ``unit-tests``.** הוא הוסיף לטסטים של 3.12 כ-30% מהזמן שלהם (הערכה: מדידה מקומית עם coverage ובלעדיו, על זמני הטסטים מדוחות ``unit-durations-*`` של ריצת ``ci.yml`` מ-4.10), ובאותה ריצה התוצאות של 3.11 ושל 3.12 היו זהות, טסט מול טסט. עכשיו רשומת ``include`` במטריצה מוסיפה ``coverage: true`` לגרסה אחת, והדגלים ``--cov`` וההעלאה ל-Codecov תלויים בה. הגרסה היא 3.11, זו שהפרודקשן רץ עליה (``Dockerfile``). שמות הסטטוסים לא השתנו. ``tests/test_required_checks_are_listed.py`` פורש עכשיו את ``include`` לפי הכלל של GitHub, ובודק שלכל מסלול יש גרסה שמודדת coverage ושהדגלים וההעלאה תלויים במפתח הזה, כי Codecov אינו בדיקת חובה, וכיסוי שנעלם לא היה מפיל דבר. ראו :doc:`ci-cd`. -- **tests: השהיות של קוד הייצור מתאפסות פעם אחת לכל הטסטים, ב-``tests/conftest.py``.** ``apply_rate_limit_delay`` ממתינה בין קריאות ל-GitHub, וטסטי הורדת ה-ZIP עברו בה באמת: כ-98 שניות של המתנה בכל מסלול ``Unit Tests``, לפי דוחות ``unit-durations-*`` של ריצת ``ci.yml`` מ-4.10. שני קבצים אחרים כבר נטרלו אותה, כל אחד לעצמו, והקבצים האלה לא. עכשיו ``tests/conftest.py`` מאפס את ``GITHUB_API_BASE_DELAY`` ואת ``GITHUB_BACKOFF_DELAY``, והנטרולים הידניים נמחקו. אליו עברו גם ערכי ה-retry וה-backoff שהיו ב-env של ``ci.yml`` (``HTTP_RESILIENCE_*``, ``REQUESTS_RETRIES``, ``REQUESTS_RETRY_BACKOFF``, ``AIOHTTP_TIMEOUT_TOTAL``), כך שהם חלים עכשיו גם על ``deploy.yml`` ועל הרצה מקומית. **ההמתנה גם הסתירה שני טסטים, וזה נמדד במוטציות:** ``test_backup_id_is_unique_within_the_same_second`` לא הגיע אף פעם לשתי אריזות באותה שנייה, ועבר גם בלי הסיומת האקראית של המזהה. ו-``test_apply_rate_limit_delay_respects_backoff`` עבר גם כשה-backoff לא הופעל, כי ההשהיה הבסיסית הייתה ארוכה ממנו. עכשיו הראשון בודק את מה שהוא מבטיח, השני קובע את ההשהיה הבסיסית בעצמו, וטסט חדש עובר דרך ההמתנה הבסיסית האמיתית, בערך קטן שהוא קובע. **ומשטסט ה-backoff התחיל לבדוק באמת, הוא נכשל ב-CI לפי סדר הריצה:** ``test_telegram_retry_after_and_message_not_modified`` הוציא את ``github_menu_handler`` מ-``sys.modules`` וייבא אותו מחדש בלי להחזיר את המודול הקודם, וטסט ה-backoff, שרץ אחריו באותו תהליך, שינה את מצב ה-backoff במודול שהקוד שרץ לא קורא ממנו. עכשיו ``monkeypatch.delitem`` מחזיר את המודול בסוף הטסט, וטסט ה-backoff בונה את ``GitHubMenuHandler`` מאותו מודול שהוא משנה. ובאותו סבב, ``tests/conftest.py`` קובע את ערכי ההשהיה בהשמה ולא ב-``setdefault``, כדי שערך שהמעטפת מייצאת לא יחזיר את ההמתנה. ראו :doc:`testing`. +- **tests: השהיות של קוד הייצור מתאפסות פעם אחת לכל הטסטים, ב-``tests/conftest.py``.** ``apply_rate_limit_delay`` ממתינה בין קריאות ל-GitHub, וטסטי הורדת ה-ZIP עברו בה באמת: כ-98 שניות של המתנה בכל מסלול ``Unit Tests``, לפי דוחות ``unit-durations-*`` של ריצת ``ci.yml`` מ-4.10. שני קבצים אחרים כבר נטרלו אותה, כל אחד לעצמו, והקבצים האלה לא. עכשיו ``tests/conftest.py`` מאפס את ``GITHUB_API_BASE_DELAY`` ואת ``GITHUB_BACKOFF_DELAY``, והנטרולים הידניים נמחקו. אליו עברו גם ערכי ה-retry וה-backoff שהיו ב-env של ``ci.yml`` (``HTTP_RESILIENCE_*``, ``REQUESTS_RETRIES``, ``REQUESTS_RETRY_BACKOFF``, ``AIOHTTP_TIMEOUT_TOTAL``), כך שהם חלים עכשיו גם על ``deploy.yml`` ועל הרצה מקומית. **ההמתנה גם הסתירה שני טסטים, וזה נמדד במוטציות:** ``test_backup_id_is_unique_within_the_same_second`` לא הגיע אף פעם לשתי אריזות באותה שנייה, ועבר גם בלי הסיומת האקראית של המזהה. ו-``test_apply_rate_limit_delay_respects_backoff`` עבר גם כשה-backoff לא הופעל, כי ההשהיה הבסיסית הייתה ארוכה ממנו. עכשיו הראשון בודק את מה שהוא מבטיח, השני קובע את ההשהיה הבסיסית בעצמו, וטסט חדש עובר דרך ההמתנה הבסיסית האמיתית, בערך קטן שהוא קובע. **ומשטסט ה-backoff התחיל לבדוק באמת, הוא נכשל ב-CI לפי סדר הריצה:** ``test_telegram_retry_after_and_message_not_modified`` הוציא את ``github_menu_handler`` מ-``sys.modules`` וייבא אותו מחדש בלי להחזיר את המודול הקודם, וטסט ה-backoff, שרץ אחריו באותו תהליך, שינה את מצב ה-backoff במודול שהקוד שרץ לא קורא ממנו. עכשיו ``monkeypatch.delitem`` מחזיר את המודול בסוף הטסט, וטסט ה-backoff בונה את ``GitHubMenuHandler`` מאותו מודול שהוא משנה. ובאותו סבב, ``tests/conftest.py`` קובע את ערכי ההשהיה בהשמה ולא ב-``setdefault``, כדי שערך שהמעטפת מייצאת לא יחזיר את ההמתנה. **ובריצה אחרת של ה-CI נכשלו לפי סדר הריצה טסטים של** ``log_user_activity``\ **, מאותה משפחה:** ``tests/test_user_stats_unit.py`` עשה ``importlib.reload`` ל-``user_stats``, וה-reload בנה מופע חדש של ``user_stats.user_stats`` בזמן ש-``main.py`` ממשיך להחזיק את הקודם, ולכן טסט שהחליף את ``log_user`` במופע החדש לא הגיע לקוד של ``main``. ה-reload היה מיותר, כי ``user_stats`` מייבא את ה-facade בכל קריאה, והוא הוסר. ראו :doc:`testing`. - **ci: טסטי ה-Markdown הכבדים רצים במסלול נפרד, במקביל לשאר טסטי היחידה.** בחודש שעבר זמן הטסטים ב-CI עלה מכשלוש דקות וחצי לכשבע וחצי (שלב ה-pytest בג'וב של 3.12, לפי דוחות ``unit-durations-*`` של ריצות ``ci.yml`` מ-9.9 ומ-4.10). הטסטים שהיו קיימים לא הואטו בסך הכול; את התוספת הביאו טסטים חדשים, ואת החלק הגדול שלה מעט טסטים כבדים. ביניהם טסטי פרסר ה-Markdown שעיקר העבודה שלהם הוא החישוב עצמו: השוואה לאורקל cmark-gfm, קלט ענק בתקרות של הפרסר, והטסטים של סקריפטי המדידה. הם מסומנים עכשיו ``md_heavy`` (הרישום ב-``pytest.ini``), והג'וב ``unit-tests`` ב-``.github/workflows/ci.yml`` רץ בשני מסלולים לכל גרסת פייתון: ``Unit Tests`` מריץ את כל השאר, ו-``Unit Tests md-heavy`` מריץ רק אותם, במקביל ובלי MongoDB ו-Redis שהוא אינו צריך. כל מסלול מדווח סטטוס חובה משלו. **את שני הסטטוסים החדשים צריך לסמן כבדיקות חובה בהגדרות של GitHub**, ועד שזה נעשה כשל בהם מופיע ב-PR אך אינו חוסם מיזוג. ושמות הסטטוסים, שכתובים ביד בכמה קבצים, נבדקים עכשיו מול המטריצה ב-``tests/test_required_checks_are_listed.py`` — והוא נכשל גם כשעותק של הרשימה מופיע בקובץ שהוא לא מכיר. :doc:`testing` ו-:doc:`branch-protection-and-pr-rules` מפנים עכשיו לרשימה שב-:doc:`ci-cd` במקום להחזיק עותק משלהם. - **fix (שירות ה-MCP): ריפו שהוסר בממשק נמחק גם מהדיסק של שירות ה-MCP.** הסרת ריפו בוובאפ מחקה את המראה שבדיסק של הוובאפ ואת הרשומות ב-``repo_metadata``, אבל לדיסק של שירות ה-MCP אף אחד לא נגע: המראה שם נשארה, וכלי הקריאה המשיכו להגיש אותה כאילו כלום לא קרה (כך קרה ל-``codekeeper-plugin``). עכשיו בסוף כל מעבר של ה-autosync, כל מראה בדיסק של השירות שאין לה רשומה ב-``repo_metadata`` נמחקת, דרך ``delete_mirror``. שום דבר לא נמחק כשהשאילתה נכשלה, או כשאף שם ב-``repo_metadata`` אינו שם של מראה שבדיסק (כולל קריאה ריקה). כשיש בדיסק מראות שנחסכו כך, נרשמת אזהרה. וגם לא נמחקים תיקייה שאינה מראה שהשירות יוצר (שם לא תקין, קישור סמלי) או ריפו שנמצא באמצע clone או fetch. מחיקה שנכשלה, או תיקיית מראות שאי אפשר לקרוא, נספרות ב-``errors``, והמראה נשארת עד המעבר הבא. הקריאה של ``repo_metadata`` נעשית מה-primary במפורש. בשורת הלוג ``repo autosync pass:`` נוסף המונה ``pruned``. ובאותה מנייה של המראות, גם ניקוי ה-credentials בעלייה מדווח עכשיו על תיקיית מראות שאי אפשר לקרוא ככשל, ולא כ-``checked=0 failed=0``. ראו :ref:`mcp-repo-autosync`. - **fix (מראות): טוקן ה-GitHub לא נשמר יותר בקובץ ה-config של המראות, ולא עובר בשורת הפקודה (#3480); וזיהוי הענף הראשי בייבוא ריפו (#3479).** עד עכשיו ``init_mirror`` שכפל מ-URL שהטוקן בתוכו, ו-git שמר אותו בטקסט גלוי ב-``remote.origin.url`` — על הדיסקים של הוובאפ ושל שירות ה-MCP, ובצילומים היומיים של Render; הוא גם הופיע בשורת הפקודה של ``git-remote-http`` בכל clone ו-fetch. עכשיו ה-URL השמור נקי, והטוקן עובר ככותרת דרך משתני סביבה של git, ורק לריפו שדורש הזדהות — ריפו ציבורי לא מקבל טוקן. מראות קיימות מנוקות לפני כל fetch ובעליית כל שירות, עם שורת לוג אחת שסופרת מה נמצא ומה נוקה, ואומרת לכל מראה מאיפה הטוקן שלה (``map``/``global``). fetch על מראה שלא אומת שהיא נקייה לא רץ (``mirror_url_not_clean``). ובנפרד: ``initial_import`` ניסה לזהות את הענף הראשי בפקודות ש-``_run_git_command`` דוחה, ולכן תמיד נפל ל-``main`` — וריפו שהענף הראשי שלו ``master`` לא יובא. עכשיו הזיהוי הוא מ-HEAD של המראה, וכשאין ענף מאומת מוחזר ``default_branch_undetected`` במקום ניחוש. ראו :ref:`mcp-mirror-credentials`. diff --git a/tests/test_user_stats_unit.py b/tests/test_user_stats_unit.py index 7f930bf39..4b856a9ec 100644 --- a/tests/test_user_stats_unit.py +++ b/tests/test_user_stats_unit.py @@ -3,6 +3,12 @@ import importlib from datetime import datetime, timezone, timedelta +# הטסטים כאן לא עושים ``importlib.reload`` ל-``user_stats``. ``_get_files_facade_or_none`` ב-``user_stats.py`` +# מייבא את ה-facade בכל קריאה, ולכן ה-stub שהטסט שם ב-``sys.modules`` נקלט גם בלי reload. ו-reload +# היה בונה מופע חדש של ``user_stats.user_stats``, בזמן ש-``main.py`` ממשיך להחזיק את הקודם +# (``from user_stats import user_stats``): טסט שרץ אחר כך באותו תהליך ומחליף את ``log_user`` במופע +# החדש לא היה מגיע לקוד של ``main``. כך נכשלו ``test_log_user_activity_*`` ב-CI של PR #3524. + def _make_fake_mongo(docs_holder): class FakeUsersCollection: @@ -69,8 +75,7 @@ async def test_log_user_updates_and_increments(monkeypatch): types.SimpleNamespace(get_files_facade=lambda: facade), ) - # Import module under test (reload to bind our mocked database) - us_mod = importlib.reload(importlib.import_module("user_stats")) + us_mod = importlib.import_module("user_stats") stats = us_mod.UserStats() today = datetime.now(timezone.utc).strftime("%Y-%m-%d") @@ -110,7 +115,7 @@ def test_get_weekly_stats_filters_and_sorts(monkeypatch): ) import importlib - us_mod = importlib.reload(importlib.import_module("user_stats")) + us_mod = importlib.import_module("user_stats") stats = us_mod.UserStats() out = stats.get_weekly_stats() @@ -133,7 +138,7 @@ def test_get_all_time_stats_counts(monkeypatch): types.SimpleNamespace(get_files_facade=lambda: facade), ) - us_mod = importlib.reload(importlib.import_module("user_stats")) + us_mod = importlib.import_module("user_stats") stats = us_mod.UserStats() out = stats.get_all_time_stats()