Skip to content

fix(tests): resolve pre-existing test suite failures from missing mocks and env pollution - #2625

Closed
teyrebaz33 wants to merge 1 commit into
NousResearch:mainfrom
teyrebaz33:fix/parallel-client-tests-missing-mock
Closed

teyrebaz33 wants to merge 1 commit into
NousResearch:mainfrom
teyrebaz33:fix/parallel-client-tests-missing-mock

Conversation

@teyrebaz33

Copy link
Copy Markdown
Contributor

Problem

Five test classes were failing on every CI run. None of these failures were caused by the tests themselves being wrong — they were infrastructure issues: missing optional dependency mocks and test environment state pollution from earlier tests.

Root causes and fixes

TestParallelClientConfig (tests/tools/test_web_tools_config.py): The parallel package is an optional dependency not installed in the test environment. from parallel import Parallel raised ModuleNotFoundError. Fixed by injecting a sys.modules mock with a FakeParallel class so isinstance() checks pass correctly.

TestTranscribeLocal / TestTranscribeLocalExtended (tests/tools/test_transcription.py, tests/tools/test_transcription_tools.py): faster_whisper is not installed in the test environment. Fixed by injecting a sys.modules mock in setup_method with cleanup in teardown_method to prevent state leakage between tests.

TestGatewayWebSocket.test_event_received_and_forwarded (tests/integration/test_ha_integration.py): The adapter was created without watch_all=True, so all state_changed events were silently dropped by the domain filter. Fixed by passing watch_all=True to _adapter_for().

TestGetProvider.test_explicit_openai_no_key_returns_none (tests/tools/test_transcription.py): Test environment pollution — earlier tests left OPENAI_API_KEY set in the process environment. monkeypatch.delenv only removed VOICE_TOOLS_OPENAI_KEY but _resolve_openai_api_key() also checks OPENAI_API_KEY. Fixed by patching _resolve_openai_api_key directly so the test is not sensitive to process-level env state.

TestDelegationCredentialResolution.test_direct_endpoint (tests/tools/test_delegate.py): Same env pollution pattern — OPENAI_API_KEY was set by an earlier test. Fixed by including OPENAI_API_KEY: "" in the patch.dict scope so the test controls its own environment.

Verification

All 5 previously-failing test classes now pass. Full test suite: 5808 passed, 0 failed (excluding pre-existing ignores).

…ks and env pollution

Five test classes were failing on every CI run due to missing optional
dependency mocks and test environment state pollution:

- TestParallelClientConfig: 'parallel' module not installed; added
  sys.modules mock with FakeParallel class so isinstance() checks pass

- TestTranscribeLocal / TestTranscribeLocalExtended: 'faster_whisper'
  module not installed; added sys.modules mock in setup_method with
  teardown cleanup to prevent state leakage between tests

- TestGatewayWebSocket.test_event_received_and_forwarded: adapter
  created without watch_all=True so all state_changed events were
  dropped by the domain filter; fixed by passing watch_all=True

- TestGetProvider.test_explicit_openai_no_key_returns_none: test
  pollution from earlier tests leaving OPENAI_API_KEY in the process
  environment; fixed by patching _resolve_openai_api_key directly
  instead of relying on env var deletion

- TestDelegationCredentialResolution.test_direct_endpoint: OPENAI_API_KEY
  left in environment by earlier tests; fixed by including it in the
  patch.dict scope
@alt-glitch alt-glitch added comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have type/test Test coverage or test infrastructure labels May 3, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the contribution @teyrebaz33 — appreciate you taking the time to dig into these.

Closing as a no-op against current main. Verified on today's main (5,739 commits ahead of this PR's base): all 5 test classes pass under scripts/run_tests.sh (the canonical hermetic test wrapper) without any of these changes:

  • TestParallelClientConfig, TestTranscribeLocal, TestTranscribeLocalExtended, TestGetProvider::test_explicit_openai_no_key_returns_none, TestDelegationCredentialResolution::test_direct_endpoint* — 27/27 passed.
  • TestGatewayWebSocket::test_event_received_and_forwarded is marked @pytest.mark.integration and is excluded from CI by addopts = "-m 'not integration'", so it doesn't run on every CI run.

A few specifics:

  1. The faster_whisper mocking issue in test_transcription.py was already addressed in PR fix(ci): unblock shared PR checks #21012 ("test(ci): stabilize shared optional dependency baselines") using patch.dict("sys.modules", ...) — cleaner than mutating sys.modules directly with manual teardown.
  2. The OPENAI_API_KEY env-pollution premise doesn't apply under our test runner: scripts/run_tests.sh and tests/conftest.py (PR test: make test env hermetic; enforce CI parity via scripts/run_tests.sh #11577) hermetically unset all credential-shaped env vars before tests run, matching CI ubuntu-latest. So no test can leak OPENAI_API_KEY to a sibling.
  3. The HA watch_all=True change isn't reachable from default CI (integration marker excluded).

If you hit a real test failure on current main running scripts/run_tests.sh, please open a new PR with the failure output — happy to look at concrete regressions.

@teknium1 teknium1 closed this May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants