From 7e21ff2af938e99536ba86485720dbbeef94b1c5 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Fri, 7 Aug 2026 15:12:30 +0500 Subject: [PATCH] =?UTF-8?q?test(ci):=20=D1=80=D0=B0=D0=B7=D0=B2=D0=B5?= =?UTF-8?q?=D1=81=D1=82=D0=B8=20=C2=AB=D0=BD=D0=B0=D0=B3=D1=80=D1=83=D0=B7?= =?UTF-8?q?=D0=BA=D1=83=20=D0=BD=D0=B5=20=D1=81=D0=BE=D0=B7=D0=B4=D0=B0?= =?UTF-8?q?=D1=82=D1=8C=C2=BB=20=D0=B8=20=C2=AB=D0=B7=D0=B0=D1=89=D0=B8?= =?UTF-8?q?=D1=82=D0=B0=20=D1=81=D0=BB=D0=BE=D0=BC=D0=B0=D0=BD=D0=B0=C2=BB?= =?UTF-8?q?=20=D0=B2=20=D0=B4=D0=B2=D1=83=D1=85=20=D0=B3=D0=B5=D0=B9=D1=82?= =?UTF-8?q?=D0=B0=D1=85=20(#2783)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Оба теста красили ЧУЖИЕ PR-ы по таймингу раннера, а на правиле «зелёный CI = можно мержить» здесь держится весь самомерж. Диагнозы разные, поэтому и правки разные. 1) tradein auth-флуд (`test_login_flood_capped_by_rate_…`). Утверждение про потолок верное — подводило ПРЕДУСЛОВИЕ `len(codes) > connections`: «каждое из ста соединений успело сходить дважды за секунду» мерит скорость раннера, а не нагрузку. На занятом первый круг из ста запросов сам съедает всю секунду, и невыполненное предусловие давало красный, неотличимый от настоящей поломки потолка. Замер: свободная машина — 0 падений из 20 (~800 ответов/с), под load average ~185 — 10 из 15, и в каждом флуд всё равно предлагал 55-80 запросов/с против порога 30/с, то есть мерить БЫЛО на чем. Теперь предусловие содержательное и с тем же порогом, с которым сверяется вердикт: пока флуд предлагает меньше `ceiling × 1.5`, следующее утверждение зелено даже без потолка вовсе — значит измерения нет, и красный говорит «раннер не потянул», а не «потолок сломан». Остальные четыре утверждения не тронуты. 2) backend weather_cache. Здесь подводило САМО утверждение: тест требовал «ровно один сетевой вызов на 16 потоков», а код (#1370) прямым текстом выносит вызов за lock и обещает обратное — «может породить несколько параллельных запросов… приемлемо». Зелёным он был по везению GIL: 200 штормов дали 2 вызова один раз (~0.5% — этим и покрасило #2781), а при `setswitchinterval(1e-6)` больше одного вызова дали 197 штормов из 200. Тест приведён к тому, что код правда гарантирует: значение у всех потоков одно, сетевых вызовов не больше числа участников, ключ один, и ПОСЛЕ шторма кэш отвечает без сети. Код не тронут: поведение объявлено приемлемым в #1370 с обоснованием; настоящий single-flight (per-key lock) — отдельная задача с отдельным обоснованием, а не побочный эффект правки теста. Оба сторожа проверены на способность краснеть: снятый гейт насыщения + пул мимо настройки → «957 сверок/с при потолке 20/с» (и под нагрузкой тоже краснеет, на том же прогоне, где раньше падало предусловие); потерянная запись в кэш / чужой ключ / мёртвый TTL → три разных красных сообщения. Refs #2783 --- backend/tests/services/test_weather_cache.py | 66 +++++++++++++++++--- tradein-mvp/backend/tests/test_auth_api.py | 30 +++++++-- 2 files changed, 80 insertions(+), 16 deletions(-) diff --git a/backend/tests/services/test_weather_cache.py b/backend/tests/services/test_weather_cache.py index f04c2322..c5cab6f1 100644 --- a/backend/tests/services/test_weather_cache.py +++ b/backend/tests/services/test_weather_cache.py @@ -10,8 +10,10 @@ DNS-fail повторяет timeout на каждый analyze. 3. ИЗОЛЯЦИЯ ДВУХ КЭШЕЙ: forecast-вызов не отравляет climate-кэш и наоборот (две раздельные таблицы внутри модуля). - 4. SINGLE-FLIGHT под конкурентностью: 16 потоков на ОДИН ключ при cold-start → - ровно ОДИН реальный httpx-вызов (lock + check-then-fetch-then-store). + 4. ШТОРМ НА COLD-START: 16 потоков на ОДИН ключ → сеть зовётся не больше раза на + поток, все получают одно и то же значение, и шторм заканчивается сложившимся + кэшем. Не «ровно один вызов»: single-flight'а тут нет и он снят сознательно + (#1370, см. сам тест). 5. ИСТЕЧЕНИЕ TTL: подменяем `weather_cache._now`, проталкиваем время за expires_at → следующий вызов идёт по сети заново (а не из устаревшего кэша). @@ -23,6 +25,7 @@ from __future__ import annotations import os import threading +import time from collections.abc import Iterator from typing import Any from unittest.mock import MagicMock, patch @@ -242,9 +245,37 @@ class TestSeparateCachesForForecastAndClimate: class TestConcurrencySafe: - def test_single_flight_cold_start_one_network_call(self) -> None: - """16 потоков на ОДИН ключ при cold-start → ровно один реальный httpx-вызов.""" - # GET имитирует медленный ответ, чтобы потоки реально гонялись за один lock. + def test_cold_start_storm_bounded_and_cache_converges(self) -> None: + """16 потоков на ОДИН ключ при cold-start: сеть зовут не больше раза на поток, + все получают одно и то же значение, и после шторма кэш отвечает без сети. + + ЗДЕСЬ СТОЯЛО `get_call_count == 1` («single-flight под lock'ом»), и это + было требование, которого код НЕ выполняет и выполнять не собирается: + сетевой вызов вынесен ЗА lock сознательно (#1370 — иначе все analyze + сериализуются на время httpx-вызова даже для разных координат), а рядом с + ним написано, что cold-start на один ключ «может породить несколько + параллельных запросов… приемлемо». Тест зеленел не потому, что защита + работает, а потому что при GIL первый поток обычно успевал сложить + результат раньше остальных. + + Замер 2026-08-07, 200 штормов подряд: при дефолтном + `sys.getswitchinterval()` 199 раз вышел 1 вызов и один раз 2 — те самые + ~0.5%, которыми гейт красил ЧУЖИЕ PR-ы (#2781: «ожидался 1 сетевой вызов, + было 2» в диффе про парсер КРТ). При `setswitchinterval(1e-6)`, когда + потоки реально чередуются, больше одного вызова дали 197 штормов из 200, + и в 173 из них вызовов было все 16. То есть утверждение ложно почти + всегда, когда гонка вообще случается, — чинить надо было тест. + + Менять КОД (per-key lock ради настоящего single-flight) сознательно НЕ + стали: поведение объявлено приемлемым в #1370 с обоснованием, лишние + запросы бывают только на cold-start одного ключа и они идемпотентны. + Понадобится — это отдельная задача с отдельным обоснованием, а не + побочный эффект правки теста. + + `time.sleep` в ответе делает гонку НЕслучайной: все 16 успевают пройти + промах кэша до первой записи. Так тест мерит худший случай той самой + уступки, а не везение планировщика. + """ start_barrier = threading.Barrier(16) get_call_count = 0 get_lock = threading.Lock() @@ -253,8 +284,7 @@ class TestConcurrencySafe: nonlocal get_call_count with get_lock: get_call_count += 1 - # Микро-задержка — окно для других потоков добраться до lock'а. - # Не делаем sleep большим, чтобы тест не висел. + time.sleep(0.05) # окно, в котором остальные потоки видят промах return _make_httpx_response(_make_forecast_response()) client_ctx = MagicMock() @@ -276,11 +306,27 @@ class TestConcurrencySafe: t.start() for t in threads: t.join() + storm_calls = get_call_count + # Шторм закончился — кэш обязан отвечать сам. Патч ещё активен, так что + # поход в сеть был бы виден счётчиком, а не отказом коннекта. + after_storm = weather_cache.get_weather_cached(56.84, 60.59) assert len(results) == 16 - assert all(r is not None for r in results) - # Single-flight под lock'ом + check-then-fetch — РОВНО один реальный вызов. - assert get_call_count == 1, f"ожидался 1 сетевой вызов, было {get_call_count}" + assert results[0] is not None + assert all(r == results[0] for r in results), "потоки увидели РАЗНЫЕ значения" + # Потолок — число участников: в сеть идут только промахнувшиеся, по разу + # каждый. Больше — значит кто-то фетчит повторно (retry-петля, потерянная + # запись в кэш); меньше единицы невозможно, кэш был пуст. + assert 1 <= storm_calls <= 16, f"сетевых вызовов {storm_calls} при 16 участниках" + # Ключ ОДИН на всех (last-write wins), и цена шторма платится один раз: + # следующий вызов идёт из кэша. Это и есть то, что #1370 обещает взамен + # снятого single-flight — без этого уступка превращается в дыру. + assert list(weather_cache._FORECAST_CACHE) == [weather_cache._round_key(56.84, 60.59)] + assert after_storm == results[0] + assert get_call_count == storm_calls, ( + f"после шторма кэш обязан отвечать без сети, а вызовов стало " + f"{get_call_count} против {storm_calls}" + ) # ────────────────────────────────────────────────────────────────────────────── diff --git a/tradein-mvp/backend/tests/test_auth_api.py b/tradein-mvp/backend/tests/test_auth_api.py index 17b0930e..9cd7b5cd 100644 --- a/tradein-mvp/backend/tests/test_auth_api.py +++ b/tradein-mvp/backend/tests/test_auth_api.py @@ -839,13 +839,31 @@ async def test_login_flood_capped_by_rate_while_api_stays_responsive( len(probe_latencies) >= 10 ), f"проба успела всего {len(probe_latencies)} раз за {elapsed:.2f}с — цикл был занят" - # 2. ТЕМП ограничен. Флуд предлагал тысячи попыток в секунду — до bcrypt их - # доехало не больше потолка (запас ×1.5 на планировщик). - assert len(codes) > connections, ( - "флуд не состоялся: на каждое соединение вышло не больше одного ответа — " - "мерить потолок не на чем" + # 2. ТЕМП ограничен. Флуд предлагал больше попыток в секунду, чем разрешает + # потолок — до bcrypt их доехало не больше него (запас ×1.5 на планировщик). + max_attempts_per_s = ceiling_per_s * 1.5 + offered_per_s = len(codes) / elapsed + + # ПРЕДУСЛОВИЕ, и оно отделено от вердикта намеренно: «нагрузку создать не + # удалось» и «потолок не работает» — разные новости, и красный обязан их + # различать. Порог здесь ТОТ ЖЕ, с которым сверяется вердикт ниже, и это не + # совпадение: пока флуд предлагает меньше, следующее утверждение зелено даже + # на системе вовсе без потолка, то есть измерения нет. + # + # Раньше условием было `len(codes) > connections` — «каждое соединение + # успело сходить хотя бы дважды за секунду». Это мерило скорости РАННЕРА, а + # не нагрузки: на занятом первый круг из ста запросов сам съедал всю секунду, + # и сторож краснел неотличимо от настоящей поломки потолка (замер 2026-08-07: + # свободная машина — 0 падений из 20, ~800 ответов/с; под load average ~185 — + # 10 падений из 15, и в каждом флуд всё равно предлагал 55-80 запросов/с + # против порога 30/с, то есть мерить было на чем). + assert offered_per_s > max_attempts_per_s, ( + f"нагрузку создать не удалось: флуд предложил {offered_per_s:.0f} запросов/с " + f"({len(codes)} за {elapsed:.2f}с) — не больше порога следующей проверки " + f"({max_attempts_per_s:.0f}/с), она прошла бы и без потолка. Это «раннер не " + f"потянул», НЕ «потолок сломан»" ) - assert attempts_per_s <= ceiling_per_s * 1.5, ( + assert attempts_per_s <= max_attempts_per_s, ( f"{attempts_per_s:.0f} сверок/с при потолке {ceiling_per_s:.0f}/с " f"({len(attempts)} за {elapsed:.2f}с) — потолок темпа не работает" )