Skip to content

TEST Isolate SDK unit tests and move LiteLLM wire checks to integration - #3070

Open
Roman Lutz (romanlutz) wants to merge 8 commits into
microsoft:mainfrom
romanlutz:romanlutz-sdk-test-failures
Open

Roman Lutz (romanlutz) wants to merge 8 commits into
microsoft:mainfrom
romanlutz:romanlutz-sdk-test-failures

Conversation

@romanlutz

Copy link
Copy Markdown
Contributor

Description

Full offline unit runs exposed incomplete image/credential mocks, an environment-sensitive reinitialization test that bypassed memory setup, and real LiteLLM transport tests that downloaded tokenizer data. These are test-isolation problems, not demonstrated new product regressions.

  • Move the six real OpenAI/Anthropic LiteLLM wire checks to gated loopback integration tests. Unit tests use the existing SDK stub and check arguments, trace metadata, response handling, and rejection before SDK invocation. Ordinary unit runs no longer need tokenizer data or a local server.
  • Mock ComicJailbreak's separate template-image dependency and cover missing Goal and Category without reaching rendering.
  • Exercise reinitialization with owned SQLite memory and scoped environment/registry/default state, including configured targets, missing-memory rejection, and cleanup after success/failure.
  • Mock the actual DefaultAzureCredential token boundary and use a deterministic clock. A collection-time timestamp previously caused the supposedly unexpired token test to refresh after a long delay.
  • Give the capability-settings probe explicit capabilities so constructing LiteLLM does not fetch unrelated model metadata.

No production API, dependency, or CI changes. The new integration file requires RUN_ALL_TESTS=true, the optional LiteLLM dependency, and a separately supplied, SHA-256-verified official tokenizer asset. It uses fake keys and loopback only, not live provider credentials; missing/corrupt enabled input fails explicitly. Tokenizer preparation is private to integration tests, with no new build helper or bundled data.

Tests and Documentation

Updated the unit-testing guide and integration guide to document the tier split, integration gate, and tokenizer input contract. JupyText was not run: no notebooks or executable documentation examples changed.

Validation used this worktree's uv environment on Windows/Python 3.13 with all extras, isolated home/memory/cache/output directories, no inherited credentials, and guards rejecting non-loopback DNS/sockets. The saved source hashes still match the committed files. GNU Make was unavailable, so the documented pytest equivalent was used.

Exact pytest invocations, with local inventory instrumentation and retained JUnit reports ($v below expands to the saved artifact directory):

$v = 'C:\Users\romanlutz\.copilot\session-state\c4743559-edf3-4b39-9e19-b040a51e253f\files\sdk-validation'
uv run --frozen --no-sync -m pytest -n 4 --dist=loadfile -p sdk_inventory tests\unit --junitxml=$v\tier-full-unit\junit.xml -o cache_dir=$v\tier-full-unit\pytest-cache
# Passed: 25,078 passed, 26 skipped, zero failures/errors or blocked external attempts.
# PYRIT_TEST_TIKTOKEN_ASSET was absent; each run used a fresh owned cache.

uv run --frozen --no-sync -m pytest -p sdk_inventory tests\integration\targets\test_litellm_trace_integration.py --junitxml=$v\tier-integration-file\junit.xml -o cache_dir=$v\tier-integration-file\pytest-cache
# Passed with RUN_ALL_TESTS=true and the local verified asset: 6 passed, no blocked external attempts.

All four unit workers collected identical inventories of 25,104 nodes. Actual JUnit cases: 25,104; declared count: 25,185. The difference is exactly 81 recorded passing subtest reports, not additional top-level cases. The 26 skips cover external-resource requirements, Windows limitations, unavailable Make/Bicep, and an Azure SQL integration-only path.

Additional controls passed: all 84 LiteLLM unit cases with real LiteLLM/tiktoken imports forbidden; six clean skips with the integration gate disabled and no asset; warm-cache restoration; and expected missing/corrupt integration-input errors with registry/environment restored. No live-provider integration suite was run.

uv run --frozen --no-sync ruff check tests\unit\auth\test_azure_auth.py tests\unit\datasets\test_comic_jailbreak_dataset.py tests\unit\prompt_target\target\test_litellm_chat_target.py tests\unit\prompt_target\test_discover_target_capabilities.py tests\unit\setup\test_reinitialization.py tests\integration\targets\test_litellm_trace_integration.py
# Passed.
uv run --frozen --no-sync ruff format --check tests\unit\auth\test_azure_auth.py tests\unit\datasets\test_comic_jailbreak_dataset.py tests\unit\prompt_target\target\test_litellm_chat_target.py tests\unit\prompt_target\test_discover_target_capabilities.py tests\unit\setup\test_reinitialization.py tests\integration\targets\test_litellm_trace_integration.py
# Passed.
uv run --frozen --no-sync ty check tests\unit\prompt_target\target\test_litellm_chat_target.py tests\integration\targets\test_litellm_trace_integration.py
# Passed.

Earlier broader scoped typing encountered the unchanged tests.unit.mocks import in the discovery test: an installed tests package shadows the checkout namespace outside pytest. No type-rule suppression, package-layout workaround, or unrelated typing change is included.

Mock external image and credential dependencies, make memory and token-clock ownership explicit, and keep real LiteLLM transport assertions in gated loopback integration tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

This branch has not been deployed

No deployments
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.

2 participants