Skip to content

Stabilize desktop migration tests on Windows#7794

Open
tianmind-studio wants to merge 2 commits into
BasedHardware:mainfrom
tianmind-studio:codex/windows-desktop-migration-test
Open

Stabilize desktop migration tests on Windows#7794
tianmind-studio wants to merge 2 commits into
BasedHardware:mainfrom
tianmind-studio:codex/windows-desktop-migration-test

Conversation

@tianmind-studio

@tianmind-studio tianmind-studio commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Stub try_acquire_user_platform_write_lock on the test database.redis_db module before importing database.users.
  • Read routers/users.py with UTF-8 in AST parity checks so Windows default code pages do not fail on source comments/strings.
  • Patch utils.llm.clients.get_llm with a module-shaped stub in title generation tests, matching the current routers.chat_sessions.generate_session_title import path.
  • Assert get_llm is called with the expected session_titles model key.
  • Reset stale database/models/utils and imported router modules before the test's import sandbox is built, then restore parent package paths so later test modules can import real package children.

Why

test_desktop_migration.py imports several production modules under a lightweight test stub environment. On a minimal Windows backend venv, collection failed when database.users imported a recently-added Redis lock helper that the test stub did not expose. After collection was fixed, the file still failed on Windows because Path.read_text() used the locale codec and because two title tests patched an old llm_mini shape while production imports get_llm().

In broader Windows collect-only runs, earlier tests can also leave stale database.*/utils.* modules in sys.modules, causing this test to reuse old stubs instead of its own import sandbox. After this test imports its target modules, the parent models, utils, and utils.other package paths are restored so later tests are not trapped behind this file's empty stub packages.

Testing

  • python -m pytest tests\unit\test_desktop_migration.py -q -> 91 passed
  • python -m pytest tests\unit --collect-only -q -x --ignore=tests\unit\test_lock_bypass_fixes.py --ignore=tests\unit\test_action_item_date_validation.py --ignore=tests\unit\test_apps_review_reply_validation.py --ignore=tests\unit\test_async_app_integrations.py --ignore=tests\unit\test_async_geocoding.py --ignore=tests\unit\test_async_http_infrastructure.py --ignore=tests\unit\test_async_webhooks.py --ignore=tests\unit\test_auth_redirect_uri.py --ignore=tests\unit\test_available_plans_resilience.py --ignore=tests\unit\test_sync_v2.py --ignore=tests\unit\test_action_item_reminder_cancel_on_complete.py --ignore=tests\unit\test_batch_upload_storage.py --ignore=tests\unit\test_daily_summary_race_condition.py --ignore=tests\unit\test_delete_account_purge_storage.py --ignore=tests\unit\test_delete_account_stripe_cancel.py -> progressed through test_desktop_migration.py and stopped at independent test_desktop_transcribe.py missing deepgram
  • python -m black --line-length 120 --skip-string-normalization tests\unit\test_desktop_migration.py --check
  • python -m py_compile tests\unit\test_desktop_migration.py
  • git diff --check -- backend/tests/unit/test_desktop_migration.py

@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes three Windows-specific test failures in test_desktop_migration.py so the 91-test suite passes on minimal Windows venvs without modifying any production code.

  • Stubs try_acquire_user_platform_write_lock onto the already-registered database.redis_db mock before database.users is imported, preventing an AttributeError on collection.
  • Adds encoding='utf-8' to two Path.read_text() calls that parse routers/users.py via AST, preventing codec errors when Windows uses a non-UTF-8 locale default.
  • Replaces MagicMock(llm_mini=mock_llm) with a proper types.ModuleType stub exposing get_llm, matching the production from utils.llm.clients import get_llm local-import pattern in routers/chat_sessions.py.

Confidence Score: 5/5

Safe to merge — changes are confined to the test file and address three concrete, independently-verified cross-platform failures with minimal risk of unintended side effects.

All three changes are narrow, well-motivated fixes: the Redis lock stub is placed at exactly the right point in the module setup sequence; the UTF-8 encoding calls are standard cross-platform hygiene; and the get_llm module stub now correctly mirrors the function the production code actually imports. No production code is modified.

No files require special attention. The sole changed file is a unit test file.

Important Files Changed

Filename Overview
backend/tests/unit/test_desktop_migration.py Three targeted fixes for Windows test collection/execution failures — Redis lock stub placement, UTF-8 encoding on file reads, and corrected get_llm module stub shape.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[pytest collects test file] --> B[Stub loop registers database.redis_db]
    B --> C[redis_stub.try_acquire_user_platform_write_lock = MagicMock]
    C --> D[sys.path.insert adds backend dir]
    D --> E[import database.users succeeds - finds stub attribute]
    A --> G[TestModelParity tests]
    G --> H[Path.read_text with encoding=utf-8 - no Windows codec error]
    A --> I[TestGenerateTitleEndpoint tests]
    I --> J[ModuleType stub with get_llm = MagicMock]
    J --> K[patch.dict sys.modules utils.llm.clients to stub]
    K --> L[generate_session_title local import resolves to stub]
Loading

Reviews (1): Last reviewed commit: "test(backend): stabilize desktop migrati..." | Re-trigger Greptile

Comment on lines +1700 to +1702
llm_clients_stub = types.ModuleType('utils.llm.clients')
llm_clients_stub.get_llm = MagicMock(return_value=mock_llm)
with patch.dict('sys.modules', {'utils.llm.clients': llm_clients_stub}):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 get_llm argument not asserted

The stub accepts any call (MagicMock(return_value=mock_llm)), so if production code changes from get_llm('session_titles') to get_llm('something_else'), these tests would still pass silently. Adding mock_get_llm.assert_called_once_with('session_titles') after each assertion would make the tests sensitive to the argument the production code actually passes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in the current head: both title-generation tests now assert mock_get_llm.assert_called_once_with('session_titles') after exercising the endpoint.

@tianmind-studio tianmind-studio force-pushed the codex/windows-desktop-migration-test branch from 8771a6a to 9183d1f Compare June 10, 2026 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant