Skip to content

Stabilize delete-account stripe test stubs on Windows #7795

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tianmind-studio wants to merge 1 commit into from
+11 −0
Closed

Stabilize delete-account stripe test stubs on Windows #7795

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions backend/tests/unit/test_delete_account_stripe_cancel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def __getattr__(self, name):
'pusher',
'modal',
'ulid',
'pytz',
'twilio',
)

Expand All @@ -57,6 +58,16 @@ def _should_stub(name: str) -> bool:
return any(name == p or name.startswith(p + '.') for p in _STUB_PREFIXES)


def _remove_module_tree(prefix: str) -> None:
for name in list(sys.modules):
if name == prefix or name.startswith(prefix + '.'):
sys.modules.pop(name, None)


for _prefix in _STUB_PREFIXES:
_remove_module_tree(_prefix)
Comment on lines +67 to +68

@greptile-apps greptile-apps Bot Jun 10, 2026

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 Module-level side-effect runs at collection time, not test-execution time

The bare for _prefix in _STUB_PREFIXES: _remove_module_tree(_prefix) block executes when pytest imports this file during collection, not when a test actually runs. In a large test suite collected in a single process, this means the eviction fires once — before any test in this file has begun — which is the desired ordering here, but it also means that if this file is collected after another test has already installed real (non-stub) versions of utils.* or database.* into sys.modules (e.g. through a session-scoped fixture in conftest), those real entries get silently evicted mid-collection. The modules would be cleanly re-imported on next access in environments where they are installable, but in the minimal Windows venv the test targets they are not importable at all — so any other test running after collection that later tries to freshly import from those namespaces would fail. Wrapping the loop in a setup_module() function would scope the eviction to test-execution time and make the intent more explicit.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +67 to +68

@greptile-apps greptile-apps Bot Jun 10, 2026

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 Loop variable _prefix leaks into the module namespace

After the for _prefix in _STUB_PREFIXES loop completes, _prefix remains bound in the module's global namespace with the value of the last element ('twilio'). While harmless in practice (it won't interfere with any test), it is inconsistent with the private-by-convention _ prefix the rest of the module uses, and static analysers will flag it as an unused global.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!



class _StubFinder(importlib.abc.MetaPathFinder, importlib.abc.Loader):
def __init__(self):
self.created = set()
Expand Down
Loading