From ba474bd97f69a2119b59f50446598134960ac2bf Mon Sep 17 00:00:00 2001 From: CI Bot Date: Mon, 17 Aug 2026 19:44:29 +0800 Subject: [PATCH 1/2] fix(tests): resolve flaky test_batch_download_session_closed Root cause: - _run_with_fakes called session_maker() BEFORE patch context - Full test suite runs caused PromiseProxy cache staleness - Session mock tracked outside patch did not match actual session Fix: - Use patch.object on actual worker_app.db module (not string path) - Track session creation INSIDE patch context via wrapper - Make test_batch_download_session_closed self-contained - Apply same pattern to test_batch_download_closes_session_on_error This ensures proper isolation and avoids module-level cache issues. --- tests/unit/test_batch_download.py | 65 +++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 16 deletions(-) diff --git a/tests/unit/test_batch_download.py b/tests/unit/test_batch_download.py index 58c669bce..fd6813f31 100755 --- a/tests/unit/test_batch_download.py +++ b/tests/unit/test_batch_download.py @@ -108,21 +108,30 @@ def _run_with_fakes( session = MagicMock() session_maker = MagicMock(return_value=session) - captured = {"upload_calls": [], "session": session_maker()} + captured: dict = {"upload_calls": []} def _tracking_upload(local_path, storage_key): captured["upload_calls"].append((local_path, storage_key)) return upload_fn(local_path, storage_key) + # Wrap session_maker to capture the session INSIDE the patch context. + # This avoids stale PromiseProxy cache issues in full-suite runs. + _created_sessions: list = [] + _orig_sm = session_maker + + def _tracking_sm(*a, **kw): + s = _orig_sm(*a, **kw) + _created_sessions.append(s) + return s + bound_task = _make_bound_task() - with patch( - "packages.adapters.sqlalchemy_impl.generated_video_repository.SQLAlchemyGeneratedVideoRepository", - return_value=repo, - ): + import worker_app.db as _db_mod + + with patch.object(_db_mod, "SessionLocal", _tracking_sm): with patch( - "worker_app.db.SessionLocal", - session_maker, + "packages.adapters.sqlalchemy_impl.generated_video_repository.SQLAlchemyGeneratedVideoRepository", + return_value=repo, ): with patch( "video_processing.oss_helpers.upload_to_oss", @@ -134,6 +143,7 @@ def _run_with_fakes( ): result = _call_task(batch_download_videos, bound_task, [v.id for v in videos], user_id) + captured["session"] = _created_sessions[0] if _created_sessions else None captured["result"] = result return captured @@ -244,19 +254,43 @@ def test_batch_download_single_video(): def test_batch_download_session_closed(): - """DB session is always closed (via finally block).""" + """DB session is always closed (via finally block). + + Self-contained: uses patch.object on the actual worker_app.db module + to avoid stale PromiseProxy cache from prior tests in the suite. + """ + import worker_app.db as _db_mod + from apps.worker.worker_app.tasks.batch_download import batch_download_videos + videos = [_FakeVideo("v1", "v.mp4")] + repo = _FakeGeneratedVideoRepository(videos) session = MagicMock() - session_maker = MagicMock(return_value=session) - _run_with_fakes(videos, session_maker=session_maker) + def _noop_download(url, dest): + Path(dest).parent.mkdir(parents=True, exist_ok=True) + Path(dest).write_bytes(b"fake video data") + + bound_task = _make_bound_task() + + with patch.object(_db_mod, "SessionLocal", MagicMock(return_value=session)): + with patch( + "packages.adapters.sqlalchemy_impl.generated_video_repository.SQLAlchemyGeneratedVideoRepository", + return_value=repo, + ): + with patch("video_processing.oss_helpers.upload_to_oss", return_value="https://oss.example.com/zip"): + with patch( + "apps.worker.worker_app.tasks.batch_download._download_video_to_file", + _noop_download, + ): + _call_task(batch_download_videos, bound_task, ["v1"], "user_1") session.close.assert_called_once() def test_batch_download_closes_session_on_error(): """Session is closed even when get_by_ids raises.""" + import worker_app.db as _db_mod from apps.worker.worker_app.tasks.batch_download import batch_download_videos class _ExplodingRepo: @@ -264,14 +298,13 @@ def test_batch_download_closes_session_on_error(): raise RuntimeError("db down") session = MagicMock() - session_maker = MagicMock(return_value=session) bound_task = _make_bound_task() - with patch( - "packages.adapters.sqlalchemy_impl.generated_video_repository.SQLAlchemyGeneratedVideoRepository", - return_value=_ExplodingRepo(), - ): - with patch("worker_app.db.SessionLocal", session_maker): + with patch.object(_db_mod, "SessionLocal", MagicMock(return_value=session)): + with patch( + "packages.adapters.sqlalchemy_impl.generated_video_repository.SQLAlchemyGeneratedVideoRepository", + return_value=_ExplodingRepo(), + ): with pytest.raises(RuntimeError, match="db down"): _call_task(batch_download_videos, bound_task, ["v1"], "u") -- 2.54.0 From 77c5bb851eb73e8a5c8627cdca487e73e8f1b898 Mon Sep 17 00:00:00 2001 From: CI Bot Date: Mon, 17 Aug 2026 11:48:51 +0000 Subject: [PATCH 2/2] style: auto-format with black + isort + prettier [skip ci-format-check] --- tests/unit/test_batch_download.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/test_batch_download.py b/tests/unit/test_batch_download.py index fd6813f31..b88f9bc59 100755 --- a/tests/unit/test_batch_download.py +++ b/tests/unit/test_batch_download.py @@ -260,6 +260,7 @@ def test_batch_download_session_closed(): to avoid stale PromiseProxy cache from prior tests in the suite. """ import worker_app.db as _db_mod + from apps.worker.worker_app.tasks.batch_download import batch_download_videos videos = [_FakeVideo("v1", "v.mp4")] @@ -291,6 +292,7 @@ def test_batch_download_session_closed(): def test_batch_download_closes_session_on_error(): """Session is closed even when get_by_ids raises.""" import worker_app.db as _db_mod + from apps.worker.worker_app.tasks.batch_download import batch_download_videos class _ExplodingRepo: -- 2.54.0