Flaky test: test_load_credentials_without_username under pytest-randomly (keyring state not isolated)

[ryancheley]

Summary

tests/test_auth.py::TestAuthManager::test_load_credentials_without_username flakes under pytest-randomly: it passes in isolation and in most orderings, but fails for certain random seeds / test orderings (observed while running pytest tests/test_auth.py tests/test_main.py). It passes deterministically (-p no:randomly) and on main, so it is order-dependent, not a code bug.

Not urgent (CI is green almost always), but it can cause spurious red CI on an unlucky seed.

The failing test

@patch.dict(os.environ, {"YOUTRACK_BASE_URL": "https://example.youtrack.cloud",
                         "YOUTRACK_TOKEN": "test-token-123"})   # note: no clear=True, no USERNAME
@patch("youtrack_cli.auth.load_dotenv")
def test_load_credentials_without_username(self, mock_load_dotenv):
    config = self.auth_manager.load_credentials()
    assert config.base_url == "https://example.youtrack.cloud"
    assert config.token == "test-token-123"
    assert config.username is None

It uses a default AuthManager, and SecurityConfig.enable_credential_encryption defaults to True, so load_credentials() reads the system keyring first (credential_manager.retrieve_credential(...)) before falling back to env vars. The test does not mock keyring.

Root cause: keyring backend is not isolated between tests

tests/conftest.py has an autouse isolate_environment fixture that clears/restores YOUTRACK_* env vars per test — so env pollution is handled. There is no equivalent isolation for the keyring backend. The keyring backend is global, process-wide, module-level state that pytest does not reset between tests. Any test (or leaked state) that leaves credentials readable in the active keyring backend makes this test read them instead of its intended env values.

Deterministic proof of the mechanism

import keyring
from keyring.backend import KeyringBackend
class Mem(KeyringBackend):
    priority = 1
    def __init__(self): self.s = {}
    def get_password(self, svc, u): return self.s.get((svc, u))
    def set_password(self, svc, u, p): self.s[(svc, u)] = p
    def delete_password(self, svc, u): self.s.pop((svc, u), None)
keyring.set_keyring(Mem())

from youtrack_cli.security import CredentialManager
cm = CredentialManager()
cm.store_credential("youtrack_base_url", "https://polluter.example")
cm.store_credential("youtrack_token", "polluter-token")
cm.store_credential("youtrack_username", "polluter-user")

import os; os.environ.pop("YOUTRACK_USERNAME", None)
from youtrack_cli.auth import AuthManager
cfg = AuthManager().load_credentials()
print(cfg.username, cfg.base_url)   # -> 'polluter-user' 'https://polluter.example'

With keyring seeded, load_credentials() returns the keyring values, so the test's assert config.username is None (and the base_url assert) fail. This is exactly the observed failure.

Reproduction

• Flakes under random ordering: uv run pytest tests/test_auth.py tests/test_main.py (rare seed).
• Always passes: uv run pytest tests/test_auth.py -p no:randomly, or the test in isolation.

The exact polluting test hasn't been pinned (both keyring-writing tests — test_auth.py:~243, test_security.py:~463 — currently mock keyring, so it may be an ambient-backend or MagicMock-leak interaction on specific orderings). The isolation gap below fixes it regardless of which test triggers it.

Recommended fix (resolve later)

Prefer the systemic fix — add keyring isolation mirroring isolate_environment in tests/conftest.py:

@pytest.fixture(scope="function", autouse=True)
def isolate_keyring():
    """Give each test a fresh, empty in-memory keyring backend."""
    import keyring
    from keyring.backend import KeyringBackend

    class _MemKeyring(KeyringBackend):
        priority = 1
        def __init__(self): self._s = {}
        def get_password(self, svc, u): return self._s.get((svc, u))
        def set_password(self, svc, u, p): self._s[(svc, u)] = p
        def delete_password(self, svc, u): self._s.pop((svc, u), None)

    prev = keyring.get_keyring()
    keyring.set_keyring(_MemKeyring())
    try:
        yield
    finally:
        keyring.set_keyring(prev)

This also removes any dependency on the developer's real macOS Keychain during tests. Alternatively (narrower): make the env-based load_credentials tests mock youtrack_cli.security.keyring or construct AuthManager(..., security_config=SecurityConfig(enable_credential_encryption=False)) so they exercise only the env-var fallback path they intend to test.

Acceptance

• Add per-test keyring isolation (or scope the two env-based load_credentials tests off the keyring path).
• test_load_credentials_without_username no longer flakes across random seeds (e.g. loop the suite with many --randomly-seed values in CI to confirm).

[github-actions]

Profile summary:

GitHub user: ryancheley
🟢 No concerns found with user's profile.
🟢 No concerns found with recent PR activity.
🟢 No concerns found with recent issue activity.

For a more detailed report, run `gh-profiler ryancheley`.