Skip to content

fix(ci): keep sample load smoke tests offline-safe#6344

Open
Sehlani042 wants to merge 1 commit into
google:mainfrom
Sehlani042:codex/fix-adk-ci-sample-smoke
Open

fix(ci): keep sample load smoke tests offline-safe#6344
Sehlani042 wants to merge 1 commit into
google:mainfrom
Sehlani042:codex/fix-adk-ci-sample-smoke

Conversation

@Sehlani042

Copy link
Copy Markdown

Summary

  • skip sample load smoke-test entries that require real Google Cloud ADC at import time
  • keep LiteLLM streaming finish-reason handling typed as non-null before setting response error details
  • remove unused imports from tests/unittests/test_samples.py

Why

Recent CI runs on existing PRs hit two unrelated main-branch failures:

  • tests/unittests/test_samples.py::test_sample_loads[...] failed for Cloud samples because offline CI has no ADC.
  • mypy reported new FinishReason | None errors at the LiteLLM streaming finalizers when passing mapped_finish_reason into _finish_reason_to_error_message.

This keeps the new sample load smoke test hermetic while preserving coverage for samples that can construct their root agents offline.

Verification

RED before fix:

  • uv run pytest tests/unittests/test_samples.py::test_sample_loads --tb=short -q -x failed on a2a/a2a_auth/remote_a2a/bigquery_agent with DefaultCredentialsError.
  • uv run mypy src/google/adk/models/lite_llm.py reported the targeted _finish_reason_to_error_message FinishReason | None errors.

GREEN after fix:

  • uv run pytest tests/unittests/test_samples.py::test_sample_loads --tb=short -q -> 149 passed, 30 skipped, 3 xfailed, 1 xpassed
  • uv run pytest tests/unittests/models/test_litellm.py -q -> 317 passed, 1 skipped
  • uv run ruff check src/google/adk/models/lite_llm.py tests/unittests/test_samples.py -> passed
  • uv run pyink --check src/google/adk/models/lite_llm.py tests/unittests/test_samples.py -> passed
  • git diff --check -> passed
  • uv run mypy . still reports the existing baseline errors, but the _finish_reason_to_error_message regression is gone from the full output.

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