Skip to content

Stabilize delete-account tests stubs on Windows#7793

Open
tianmind-studio wants to merge 2 commits into
BasedHardware:mainfrom
tianmind-studio:codex/windows-delete-account-purge-test
Open

Stabilize delete-account tests stubs on Windows#7793
tianmind-studio wants to merge 2 commits into
BasedHardware:mainfrom
tianmind-studio:codex/windows-delete-account-purge-test

Conversation

@tianmind-studio

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

Copy link
Copy Markdown
Contributor

Summary

  • Include pytz in the auto-stubbed import prefixes for both delete-account tests that import routers.users.
  • Clear any existing modules under the stubbed prefixes before installing each test meta-path finder, so stale sys.modules entries from earlier tests cannot bypass the local _StubFinder.

Why

test_delete_account_purge_storage.py and test_delete_account_stripe_cancel.py both import routers.users, which has a broad import graph. In a lightweight Windows test environment, collection failed first on missing pytz. In broader collect-only runs, earlier tests can also leave stale database.* or utils.* entries in sys.modules; because importlib returns existing modules before consulting these tests' finders, stale modules can break the router import before the deletion assertions run.

This keeps both delete-account import sandboxes deterministic and scoped to the namespaces they already declare as stubbed.

Testing

  • python -m pytest tests\unit\test_delete_account_purge_storage.py -q -> 5 passed
  • python -m pytest tests\unit\test_delete_account_stripe_cancel.py -q -> 3 passed
  • python -m pytest tests\unit\test_delete_account_purge_storage.py tests\unit\test_delete_account_stripe_cancel.py -q -> 8 passed
  • python -m pytest tests\unit\test_byok_security.py tests\unit\test_delete_account_purge_storage.py tests\unit\test_delete_account_stripe_cancel.py --collect-only -q -x -> 89 tests collected
  • python -m black --line-length 120 --skip-string-normalization tests\unit\test_delete_account_purge_storage.py tests\unit\test_delete_account_stripe_cancel.py --check
  • python -m py_compile tests\unit\test_delete_account_purge_storage.py tests\unit\test_delete_account_stripe_cancel.py
  • git diff --check origin/main...HEAD

@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes two targeted changes to test_delete_account_purge_storage.py to fix flaky collection on Windows minimal venvs: it adds 'pytz' to the auto-stub prefix list so importing routers.users no longer requires the optional package, and it sweeps sys.modules clean of every stubbed namespace before installing the _StubFinder, preventing stale mocks left by earlier test files from bypassing the finder.

  • 'pytz' is appended to _STUB_PREFIXES, eliminating the ModuleNotFoundError that surfaced on Windows when the minimal venv did not include the optional timezone library.
  • A new module-level helper _remove_module_tree is called for every stub prefix before the _StubFinder is inserted into sys.meta_path, making the import sandbox deterministic regardless of test-collection order.
  • The identical sister file test_delete_account_stripe_cancel.py uses the same _StubFinder / _STUB_PREFIXES pattern to import routers.users and is not updated, leaving the same Windows pytz gap and stale-modules vulnerability unfixed there.

Confidence Score: 4/5

Safe to merge for purge_storage tests; however the same Windows pytz failure and stale-mock bypass remain unfixed in the sister file test_delete_account_stripe_cancel.py.

The two changes to test_delete_account_purge_storage.py are correct and well-scoped. The gap is that the identical import-sandbox pattern in test_delete_account_stripe_cancel.py does not receive the same pytz stub entry or the pre-install sys.modules sweep, leaving it vulnerable to the same Windows collection failure and stale-module bypass that motivated this PR.

backend/tests/unit/test_delete_account_stripe_cancel.py — identical stub pattern without the pytz entry and without the _remove_module_tree sweep.

Important Files Changed

Filename Overview
backend/tests/unit/test_delete_account_purge_storage.py Adds pytz to the stub prefix list and introduces a pre-install sys.modules sweep to clear stale mocks left by earlier test files; logic is correct for this file, but the identical sister file test_delete_account_stripe_cancel.py does not receive the same fixes.

Comments Outside Diff (1)

  1. backend/tests/unit/test_delete_account_purge_storage.py, line 38-53 (link)

    P1 Sister file test_delete_account_stripe_cancel.py has the same unfixed gaps

    test_delete_account_stripe_cancel.py is a near-identical copy of this file: it uses the same _StubFinder / _STUB_PREFIXES pattern, it imports routers.users, and that module's import chain pulls in pytz. On a minimal Windows venv where pytz is absent, running stripe_cancel in isolation hits the same collection failure that this PR fixes here. That file also lacks the _remove_module_tree sweep, so any stale database.* or utils.* mock left by test_byok_security.py can bypass its _StubFinder for the same reason described in the PR.

    The two fixes applied here ('pytz' in _STUB_PREFIXES and the pre-install sys.modules clear) should be mirrored in test_delete_account_stripe_cancel.py.

Reviews (1): Last reviewed commit: "test(backend): isolate delete account pu..." | Re-trigger Greptile

@tianmind-studio tianmind-studio changed the title Stabilize delete-account purge test stubs on Windows Stabilize delete-account tests stubs on Windows Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

The Greptile P1 summary was based on the earlier commit that only updated test_delete_account_purge_storage.py.

Addressed in the current head 534ad233: the same pytz stub entry and _remove_module_tree(...) pre-install cleanup are now mirrored in test_delete_account_stripe_cancel.py as well. I rechecked the PR diff and both delete-account test files carry the same import-sandbox fix.

Validation from the PR body:

  • python -m pytest tests\unit\test_delete_account_purge_storage.py -q -> 5 passed
  • python -m pytest tests\unit\test_delete_account_stripe_cancel.py -q -> 3 passed
  • python -m pytest tests\unit\test_delete_account_purge_storage.py tests\unit\test_delete_account_stripe_cancel.py -q -> 8 passed
  • python -m pytest tests\unit\test_byok_security.py tests\unit\test_delete_account_purge_storage.py tests\unit\test_delete_account_stripe_cancel.py --collect-only -q -x -> 89 tests collected

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